Closed Bug 692291 Opened 13 years ago Closed 13 years ago

IonMonkey: Correctly extract JSFunction from JSObject for calls.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sstangl, Unassigned)

References

Details

Attachments

(2 files)

Attached patch Fix.Splinter Review
Calls are incorrect: they assume that testing a JSObject to be a JSFunction means that the JSObject can be used as a JSFunction, when the interpreter JSOP_CALL implementation actually fetches the JSFunction* from JSObject.privateData. So now we do that.

(Incidentally, privateData is pretty shady. Could we better express it using a union?)

This fixes crashes in 14 jit-tests on x64, --ion-eager. With this patch applied, we're down to 27.
Attachment #565029 - Flags: review?(dvander)
privateData is an API thing, it is private to the JS class and different classes use it in different ways.  There are some well known uses of privateData like for FunctionClass, but in general it can hold any pointer-sized value.
Comment on attachment 565029 [details] [diff] [review]
Fix.

Review of attachment 565029 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/shared/CodeGenerator-x86-shared.cpp
@@ +707,5 @@
>          return false;
>  
> +    // Extract the function object.
> +    // TODO: Apparently this is not always required. Can TI help?
> +    masm.movePtr(Operand(objreg, offsetof(JSObject, privateData)), objreg);

For a generic call, we can suck this up - for a direct, monomorphic call, we shouldn't have to do it. So I'd lose the TODO so vim doesn't hang Christmas lights around it :)
Attachment #565029 - Flags: review?(dvander) → review+
Second small patch to calls handles ION_DISABLED_SCRIPT -- the JSScript.ion is occasionally 0x1, designating a former compilation failure. Thus the NULL check is incorrect.

With this patch applied, we now have 2 jit-test failures, one probably from StackIter, one from SunSpider.
Attachment #565068 - Flags: review?(dvander)
http://hg.mozilla.org/projects/ionmonkey/rev/63c64b0c437e

With the patch still up for review applied, we pass all jit-tests on x64 --ion-eager.
Attachment #565068 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/801d2c532aa5
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.