Last Comment Bug 726220 - IonMonkey: ContainsCodeAddress has an off by 1 error
: IonMonkey: ContainsCodeAddress has an off by 1 error
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-02-10 16:15 PST by Marty Rosenberg [:mjrosenb]
Modified: 2012-02-17 19:44 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

/home/mrosenberg/patches/fixFunctionCheck-r0.patch (1.42 KB, patch)
2012-02-10 16:15 PST, Marty Rosenberg [:mjrosenb]
no flags Details | Diff | Splinter Review
/home/mrosenberg/patches/fixFunctionCheck-r1.patch (2.43 KB, patch)
2012-02-15 11:56 PST, Marty Rosenberg [:mjrosenb]
sstangl: review+
Details | Diff | Splinter Review

Description User image Marty Rosenberg [:mjrosenb] 2012-02-10 16:15:22 PST
Created attachment 596214 [details] [diff] [review]

we call containsCodeAddress with a pointer that is the return address from a function.
For the most part, this works, but in some cases, when we know a call isn't going to return, no code is placed after the call, so the return address is not technically part of the function, and this makes walking up the stack *quite* sad.
Comment 1 User image Sean Stangl [:sstangl] 2012-02-13 16:07:40 PST
Comment on attachment 596214 [details] [diff] [review]

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

::: js/src/ion/IonCode.h
@@ +286,5 @@
> +        // however, when the code for exceptions is generated, there is no code
> +        // after the call.  If that was the last instruction in the function,
> +        // then the return address would be exactly at the upper bound of the
> +        // function, so it has been changed to <=
> +        return method()->raw() <= addr && addr <= method()->raw() + method()->instructionsSize();

This change causes containsCodeAddress() to lie for the purpose of appeasing exceptions. Instead of having this function lie, could we solve the problem locally in the exception generators by inserting NOPs into the code stream, with commentary?
Comment 2 User image Marty Rosenberg [:mjrosenb] 2012-02-15 11:56:09 PST
Created attachment 597509 [details] [diff] [review]

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