Last Comment Bug 738124 - IonMonkey: OSI breaks if a pool lands in the wrong place
: IonMonkey: OSI breaks if a pool lands in the wrong place
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: ARM Linux
-- normal (vote)
: ---
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-21 19:53 PDT by Marty Rosenberg [:mjrosenb]
Modified: 2012-07-27 17:57 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Record the call site rather than the return site in OSI (12.14 KB, patch)
2012-03-21 19:53 PDT, Marty Rosenberg [:mjrosenb]
dvander: review+
Details | Diff | Splinter Review

Description User image Marty Rosenberg [:mjrosenb] 2012-03-21 19:53:06 PDT
Created attachment 608205 [details] [diff] [review]
Record the call site rather than the return site in OSI

if we have the sequence

1) blx func
2) add sp, sp, 8
3) b pool_end:
.data pool
pool_end:
4) more code

OSI correctly attempts to choose (3) to be the instruction that will later be replaced with a call, however, we record one higher(4), since it is going to be the return address.  With this recorded, the displacement of (4) is correctly updated with the length of the pool, then we attempt to place a call with (4) as its return address, resulting in a call that can never be executed.

By recording (3), then incrementing to get the return address, we patch the call correctly.
Comment 1 User image David Anderson [:dvander] 2012-03-22 15:38:17 PDT
Comment on attachment 608205 [details] [diff] [review]
Record the call site rather than the return site in OSI

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

::: js/src/ion/shared/CodeGenerator-shared.cpp
@@ +343,5 @@
>      JS_ASSERT(masm.currentOffset() - lastOsiPointOffset_ >= Assembler::patchWrite_NearCallSize());
>      lastOsiPointOffset_ = masm.currentOffset();
>  
> +    *callPointOffset = masm.currentOffset();
> +    static int osiCount = 0;

^- Remember to take this out before checking in.
Comment 2 User image Marty Rosenberg [:mjrosenb] 2012-03-27 20:57:39 PDT
landed: http://hg.mozilla.org/projects/ionmonkey/rev/01dfaa84975b
Comment 3 User image Marty Rosenberg [:mjrosenb] 2012-07-27 17:57:19 PDT
whoops.

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