Segmentation fault in fixupNode from DFGFixupPhase.cpp

[Roman Zhuykov]

In fixupNode function after creating a new node (for example by blessArrayOperation), we must not use the "node" reference, since the vector of nodes may be reallocated. Everywhere the pointer "Node* nodePtr = &m_graph[m_compileIndex];" is created, but not in the last DEBUG_PROPAGATION_VERBOSE block.

[Roman Zhuykov]

Created attachment 178726 [details]
Proposed patch

[Alexey Proskuryakov]

Can a regression test be made for this?

[Roman Zhuykov]

(In reply to comment #2)
> Can a regression test be made for this?
The are two problems with regression test.
1) I have only a test which fails when I use JSC with my patch from https://bugs.webkit.org/show_bug.cgi?id=104638
That patch sometimes creates DFG with a lot of comparison nodes and that's why I catch this problem. I can't create any manual test to catch this on trunk JSC version.
2) Wrong code works only when 
DFG_ENABLE(DEBUG_PROPAGATION_VERBOSE) is enabled. As I know, no testing is done with such debug options. For my testing I usually compile JSC (only console version, without the whole webkit) for two platforms - x86-64 and ARMv7 Thumb2, and I always create jsc-release version and debug one with the following debug options enabled:
DFG_ENABLE_DEBUG_VERBOSE
DFG_ENABLE_DEBUG_PROPAGATION_VERBOSE
DFG_ENABLE_VERBOSE_SPECULATION_FAILURE
ENABLE_JIT_VERBOSE
ENABLE_JIT_VERBOSE_OSR
WTF_USE_UDIS86
ENABLE_SAMPLING_COUNTERS
ENABLE_SAMPLING_FLAGS
ENABLE_SAMPLING_REGIONS
ENABLE_SAMPLING_THREAD
ENABLE_OPCODE_SAMPLING
ENABLE_CODEBLOCK_SAMPLING
Every couple of days some changes in JSC add some problems to this extra-debug version. I can give three examples. One unresolved problem with failing asserts is here https://bugs.webkit.org/show_bug.cgi?id=100111
The other example is typo intoduced as soon as October 23 (rev.132182) and it fails compilation only when  ENABLE_OPCODE_SAMPLING enabled, so nobody cares:
--- a/Source/JavaScriptCore/bytecode/SamplingTool.h 
+++ b/Source/JavaScriptCore/bytecode/SamplingTool.h 
@@ -240,7 +241,7 @@ namespace JSC { 
                 , m_savedSample(samplingTool->m_sample) 
                 , m_savedCodeBlock(samplingTool->m_codeBlock) 
             { 
-                if (isHostcall) 
+                if (isHostCall) 
                     samplingTool->m_sample |= 0x1; 
             } 
And the last, simply running Scripts\run-javascriptcore-tests for today rev137407 on x86-64 I got a lot of new failures, more than 300 failures for debug build. This problem starts since revision 137179.
Most of such problems are solved pretty fast, but maybe it would be better to find them immediately when someone creates the patch. Certainly I can create the bugs for all such situations, but it's strange to have no feedback: https://bugs.webkit.org/show_bug.cgi?id=98758 was just silently fixed inside very big patch containing a lot of stuff, irrelevant to ARM platform.

[Roman Zhuykov]

Fixed in SVN rev. 140227

[Brent Fulgham]

Closing. Landed in http://trac.webkit.org/changeset/140227

[Brent Fulgham]

Comment on attachment 178726 [details]
Proposed patch

Clearing flag, since this patch is landed.

[Filip Pizlo]

We will remove these DFG_ENABLE verbose things. You should use the various runtime verbosity options instead.