Bug 217362

Created attachment 410608 [details]
Patch

Created attachment 410612 [details]
Patch

Comment on attachment 410612 [details]
Patch

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

r=me with fixes.

> Source/JavaScriptCore/llint/LLIntExceptions.cpp:76
> +MacroAssemblerCodeRef<ExceptionHandlerPtrTag> catcher(OpcodeSize size)

Can we call this handleCatch (verb) instead of catcher (noun)?

> Source/JavaScriptCore/llint/LLIntExceptions.cpp:80
> +        return catcherThunk(size);

Ditto: handleCatchThunk?

> Source/JavaScriptCore/llint/LLIntThunks.cpp:152
> +            codeRef.construct(generateThunkWithJumpTo<JITThunkPtrTag>(wasm_function_prologue, "function for call"));

Is there any reason this shouldn't also be using JSEntryPtrTag (and obsoleting JITThunkPtrTag)?  Looks like this is the last use of it.  We used to use JITThunkPtrTag for certain pointers based on where they point to.  Now, you've changed this to based on where the pointers will be used.  Based on where the pointer is used, it sounds like wasm_function_prologue should be using JSEntryPtrTag too.

You don't have to do this in this patch, but maybe it warrants a look later.

Comment on attachment 410612 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/llint/LLIntExceptions.cpp:76
>> +MacroAssemblerCodeRef<ExceptionHandlerPtrTag> catcher(OpcodeSize size)
> 
> Can we call this handleCatch (verb) instead of catcher (noun)?

Changed.

>> Source/JavaScriptCore/llint/LLIntExceptions.cpp:80
>> +        return catcherThunk(size);
> 
> Ditto: handleCatchThunk?

Changed.

>> Source/JavaScriptCore/llint/LLIntThunks.cpp:152
>> +            codeRef.construct(generateThunkWithJumpTo<JITThunkPtrTag>(wasm_function_prologue, "function for call"));
> 
> Is there any reason this shouldn't also be using JSEntryPtrTag (and obsoleting JITThunkPtrTag)?  Looks like this is the last use of it.  We used to use JITThunkPtrTag for certain pointers based on where they point to.  Now, you've changed this to based on where the pointers will be used.  Based on where the pointer is used, it sounds like wasm_function_prologue should be using JSEntryPtrTag too.
> 
> You don't have to do this in this patch, but maybe it warrants a look later.

wasmFunctionEntryThunk's code is used for jump target. So this I not JSEntry. I think JITThunkPtrTag is the right tag for this thunk.

Committed r268077: <https://trac.webkit.org/changeset/268077>

<rdar://problem/70019125>