Bug 104247 – Strange results calculating a square root in a loop

• Component: JavaScriptCore
• Status: RESOLVED FIXED

I'm computing the square root of an arbitrary number about a 1000 times and printing the result only if it's less than 0. At some point it starts printing 2.236.... (for sqrt of 5) which is the expected result, but shouldn't be printed since it's not a negative number. Furthermore, some iterations later it starts printing some large (negative for some inputs to sqrt) number.

I noticed it breaks for some numbers inside sqrt (5,7,11,15) but not for others (6, 13)

[Alexey Proskuryakov]

Created attachment 178028 [details]
same test as an attachment

[Alexey Proskuryakov]

Hits an assertion failure in debug ToT:

ASSERTION FAILED: originalNode->shouldGenerate()
/Users/ap/Safari/OpenSource/Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp(309) : void JSC::DFG::CFGSimplificationPhase::fixPossibleGetLocal(JSC::DFG::BasicBlock *, JSC::DFG::Edge &, bool)
1   0x10d872563 JSC::DFG::CFGSimplificationPhase::fixPossibleGetLocal(JSC::DFG::BasicBlock*, JSC::DFG::Edge&, bool)
2   0x10d870fb0 JSC::DFG::CFGSimplificationPhase::mergeBlocks(unsigned int, unsigned int, unsigned int)
3   0x10d86fb9e JSC::DFG::CFGSimplificationPhase::run()
4   0x10d86f8c5 bool JSC::DFG::runAndLog<JSC::DFG::CFGSimplificationPhase>(JSC::DFG::CFGSimplificationPhase&)
5   0x10d86f855 bool JSC::DFG::runPhase<JSC::DFG::CFGSimplificationPhase>(JSC::DFG::Graph&)
6   0x10d86f768 JSC::DFG::performCFGSimplification(JSC::DFG::Graph&)

[Alexey Proskuryakov]

<rdar://problem/12826880>

[Filip Pizlo]

Fascinating! It looks like this is somehow exposing a bug in our CFG simplifier. That's why the irrelevant branch is so important - without it we don't do CFG simplification, and presumably, don't end up breaking the IR leading to bad codegen.

[Filip Pizlo]

Created attachment 178118 [details]
fix the assertion

[Filip Pizlo]

(In reply to comment #5)
> Created an attachment (id=178118) [details]
> fix the assertion

I'm still running layout tests.  I will land once I have the -expected.txt file.

[Oliver Hunt]

Comment on attachment 178118 [details]
fix the assertion

r+, but you need to add the expected  results when you land

[Build Bot]

Comment on attachment 178118 [details]
fix the assertion

Attachment 178118 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15202006

New failing tests:
fast/js/dfg-cfg-simplify-redundant-dead-get-local.html

[Filip Pizlo]

(In reply to comment #7)
> (From update of attachment 178118 [details])
> r+, but you need to add the expected  results when you land

Yup, will do!

[WebKit Review Bot]

Comment on attachment 178118 [details]
fix the assertion

Attachment 178118 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15183315

New failing tests:
fast/js/dfg-cfg-simplify-redundant-dead-get-local.html

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/136989