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
:
:
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 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 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 Marty Rosenberg [:mjrosenb] 2012-03-27 20:57:39 PDT
landed: http://hg.mozilla.org/projects/ionmonkey/rev/01dfaa84975b
Comment 3 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.