Bug 149531

Summary: We should only expect a RareCaseProfile to exist if the rare case actually exists
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the fix. sbarati: review+

Description Mark Lam 2015-09-24 10:27:31 PDT
The current code that calls rareCaseProfileForBytecodeOffset() always assumes that it will always return a non-null RareCaseProfile.  As a result, op_add in the baseline JIT is forced to add a dummy slow case that will never be taken, only to ensure that the RareCaseProfile for that bytecode is created, and it will always produce a counter value of 0 (since that path will never be taken).  Instead, we'll make the callers of rareCaseProfileForBytecodeOffset() check if the profile actually exist before dereferencing it.
Comment 1 Mark Lam 2015-09-24 10:44:03 PDT
Created attachment 261875 [details]
the fix.
Comment 2 Saam Barati 2015-09-24 11:25:43 PDT
Comment on attachment 261875 [details]
the fix.

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

r=me with comments.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:3974
> +    auto profile = rareCaseProfileForBytecodeOffset(bytecodeOffset);

nit: Can we write the actual type here?

> Source/JavaScriptCore/jit/JITArithmetic.cpp:797
>      if (!types.first().mightBeNumber() || !types.second().mightBeNumber()) {

We need 32-bit code too.

> Source/JavaScriptCore/jit/JITArithmetic.cpp:826
> +    RELEASE_ASSERT(!(!types.first().mightBeNumber() || !types.second().mightBeNumber()));

I think this reads better as: 
RELEASE_ASSERT(types.first().mightBeNumber() && types.second().mightBeNumber()))
Comment 3 Mark Lam 2015-09-24 11:40:19 PDT
Thanks.  I've fixed the issues before landing.

Landed in r190213: <http://trac.webkit.org/r190213>.