[webkit-reviews] review denied: [Bug 122306] Add callOperation to Baseline JIT : [Attachment 213334] Patch with Updates from reviews
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 4 00:03:46 PDT 2013
Geoffrey Garen <ggaren at apple.com> has denied Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 122306: Add callOperation to Baseline JIT
https://bugs.webkit.org/show_bug.cgi?id=122306
Attachment 213334: Patch with Updates from reviews
https://bugs.webkit.org/attachment.cgi?id=213334&action=review
------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213334&action=review
> Source/JavaScriptCore/jit/JITInlines.h:232
> +ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(J_JITOperation_E
operation, int dst)
> +{
> + setupArgumentsExecState();
> + return appendCallWithExceptionCheckSetCellResult(operation, dst);
> +}
> +
> +ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(J_JITOperation_EP
operation, int dst, void* pointer)
> +{
> + setupArgumentsWithExecState(TrustedImmPtr(pointer));
> + return appendCallWithExceptionCheckSetCellResult(operation, dst);
> +}
I don't think you understood my meaning. The function types here are
J_JITOperation_E and J_JITOperation_EP:
typedef EncodedJSValue JIT_OPERATION (*J_JITOperation_E)(ExecState*);
typedef EncodedJSValue JIT_OPERATION (*J_JITOperation_EP)(ExecState*, void*);
It's inappropriate for a utility function that calls a function returning
EncodedJSValue to assume that the return value is a cell. That's the assembly
equivalent of an invalid cast.
You should either change the return type of operationNewRegexp to be JSCell*,
or you should change these call sites to store a full EncodedJSValue.
More information about the webkit-reviews
mailing list