Bug 144853

Summary: js/dom/stack-trace.html fails with eager compilation
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, basile_clement, benjamin, commit-queue, ggaren, mark.lam, mhahnenb, mmirman, msaboff, nrotem, oliver, saam, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 144877    
Bug Blocks: 144945    
Attachments:
Description Flags
starting to work
none
the patch benjamin: review+

Description Filip Pizlo 2015-05-10 21:09:07 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2015-05-12 18:06:28 PDT
This looks like an FTL bug. No FTL and the bug goes away.
Comment 2 Filip Pizlo 2015-05-12 18:20:10 PDT
Wow, this is hilarious.  The various escape analyses don't have any clue how to handle:

a: NewObject()
b: Check(Number:@a)

or anything else like that where the check is guaranteed to fail.
Comment 3 Filip Pizlo 2015-05-12 20:36:33 PDT
Created attachment 253012 [details]
starting to work
Comment 4 Filip Pizlo 2015-05-12 21:36:58 PDT
Created attachment 253018 [details]
the patch

One of the newly introduced tests is still crashing.  I'll investigate whether this is related to my patch or not.  If it's unrelated then I'll land the test skipped and file a separate bug.
Comment 5 WebKit Commit Bot 2015-05-12 21:39:22 PDT
Attachment 253018 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:171:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:111:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Filip Pizlo 2015-05-12 21:58:09 PDT
(In reply to comment #4)
> Created attachment 253018 [details]
> the patch
> 
> One of the newly introduced tests is still crashing.  I'll investigate
> whether this is related to my patch or not.  If it's unrelated then I'll
> land the test skipped and file a separate bug.

Yup, it's a preexisting separate bug: https://bugs.webkit.org/show_bug.cgi?id=144945
Comment 7 Benjamin Poulain 2015-05-12 22:03:08 PDT
Comment on attachment 253018 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253018&action=review

> Source/JavaScriptCore/jsc.cpp:624
> +        putDirectNativeFunction(vm, this, Identifier::fromString(&vm, "isInt32"), 0, functionIsInt32, CheckInt32, DontEnum | JSC::Function);

I would prefer CheckInt32Intrinsic to follow the other intrinsics naming.
Comment 8 Filip Pizlo 2015-05-12 22:23:04 PDT
Landed in http://trac.webkit.org/changeset/184260