[Webkit-unassigned] [Bug 109836] [JIT] Memory overwrite by Math object functions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=109836


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #190067|review?                     |review-
               Flag|                            |




--- Comment #4 from Filip Pizlo <fpizlo at apple.com>  2013-03-22 08:34:42 PST ---
(From update of attachment 190067)
View in context: https://bugs.webkit.org/attachment.cgi?id=190067&action=review

> Source/JavaScriptCore/jit/SpecializedThunkJIT.h:109
> -            storeDouble(src, Address(stackPointerRegister, -(int)sizeof(double)));
> -            loadPtr(Address(stackPointerRegister, OBJECT_OFFSETOF(JSValue, u.asBits.tag) - sizeof(double)), regT1);
> -            loadPtr(Address(stackPointerRegister, OBJECT_OFFSETOF(JSValue, u.asBits.payload) - sizeof(double)), regT0);
> +            push(regT2);
> +            push(regT2);
> +            storeDouble(src, Address(stackPointerRegister, +(int)sizeof(double)));
> +            loadPtr(Address(stackPointerRegister, OBJECT_OFFSETOF(JSValue, u.asBits.tag) + sizeof(double)), regT1);
> +            loadPtr(Address(stackPointerRegister, OBJECT_OFFSETOF(JSValue, u.asBits.payload) + sizeof(double)), regT0);
>              Jump lowNonZero = branchTestPtr(NonZero, regT1);
>              Jump highNonZero = branchTestPtr(NonZero, regT0);
> +            pop(regT2);
> +            pop(regT2);

This looks wrong; you seem to be jumping over the pops.  This will leave the stack in a weird state.

I'm not sure if this is a real bug, either.  It ought to be safe to store at a negative stack offset, in most platforms.

Plus, I don't like the implications for performance of the thunk: you're doing some extra pushes and pops.

But if it's a real bug, you could have implemented this by changing the offset at which we store the double.  The JITStackFrame::args should be reusable here, since we're not in the middle of a JITStub call.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.