Last Comment Bug 717077 - OSI doesn't actually work on ARM
: OSI doesn't actually work on ARM
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: ARM Linux
-- normal (vote)
: ---
Assigned To: general
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2012-01-10 15:28 PST by Marty Rosenberg [:mjrosenb]
Modified: 2012-02-01 14:06 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix OSI on arm (6.29 KB, patch)
2012-01-10 15:28 PST, Marty Rosenberg [:mjrosenb]
cdleary: review+
Details | Diff | Splinter Review

Description User image Marty Rosenberg [:mjrosenb] 2012-01-10 15:28:02 PST
Created attachment 587508 [details] [diff] [review]
fix OSI on arm

This was my bad. I mis-identified all of the OSI failures as other known failures, and didn't even bother looking into them.

Problems with my implementation of OSI: the stack may be misaligned when we enter the invalidator, since on a return from JS code, the (aligned) return value is popped, and on a return from C code, the (aligned) return value is ignored.  Then, the calculation of the previous frame was wrong, since it only accounted for the padding that exists on x86.
Finally, I didn't distinguish the return slot from the return value of a function.

This patch also fixes a typo in all three implementations that ask for three arguments, but only provide two to InvalidationBailout.
Comment 1 User image Chris Leary [:cdleary] (not checking bugmail) 2012-01-10 20:41:00 PST
Comment on attachment 587508 [details] [diff] [review]
fix OSI on arm

Review of attachment 587508 [details] [diff] [review]:

::: js/src/ion/arm/Bailouts-arm.cpp
@@ +170,2 @@
>      }
> +  public:

Useless label?

Note You need to log in before you can comment on or make changes to this bug.