Closed Bug 777788 Opened 12 years ago Closed 12 years ago

IonMonkey: JM to Ion calls are not monitoring the returned value when required by the Type Inference.

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr10 --- unaffected

People

(Reporter: nbp, Assigned: dvander)

References

Details

(Keywords: sec-moderate, Whiteboard: [ion:t])

Attachments

(4 files, 5 obsolete files)

Attached patch Bug 777537 fix. (obsolete) — Splinter Review
The fix to Bug 777537 which sanitize the current usage of the CompilerOutputs highlight and/or cause invalid reads and errors.

The current implementation has dandling pointers to previously used compilation outputs.  These pointers are not supposed to be used unless checked against the current JSScript content which guarantee their validity.
Attached file Not-so reduced failing test case. (obsolete) —
Test case highlighting the issue.
Attached file Valgrind output. (obsolete) —
Valgrind output created by running --smc-check=all
Whiteboard: [js:t]
Depends on: 778323
Whiteboard: [js:t] → [ion:t]
Attached patch Bug 777537 fix.Splinter Review
Update patch.
Another reduction of the same test case.
Attachment #646196 - Attachment is obsolete: true
Attachment #646197 - Attachment is obsolete: true
Attachment #646198 - Attachment is obsolete: true
Almost readable type error message.
The main function of the « reduced » test case is not compiled with --ion-eager, This cause the main function to be compiled with JM once all smaller functions are compiled with IonMonkey.

Type inference mention that the result of the call should be monitored.  My guess is that we are missing a monitored call and return to the interpreter when executing the « expected[i] » which cause the script to get back into the interpreter which then fails when try to call the function with « a » which does not contain any monitored type.
Summary: IonMonkey: TI: Using a uniq Compiler Output cause unexpected failures. → IonMonkey: JM to Ion calls are not monitoring the returned value when required by the Type Inference.
Provide arguments to back up the previous comment.
Attachment #648241 - Attachment is obsolete: true
Yup, you're absolutely right, thanks for the analysis. Originally the JM->Ion patch bounced back to an earlier rejoin point, but now it uses the native one and natives must monitor their result.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attached patch fix attempt v0 (obsolete) — Splinter Review
Nicolas, could you see if this fixes your problem? It's not 100% working yet (math-jit-tests fails) but it's getting close.

I think the remaining problem is that invalidation now is missing a TypeMonitor, rather than the inline path. If that's the case I may abandon this approach and perform the type monitor inside the IC.
Attached patch fixSplinter Review
This patch makes JM->Ion ICs join right after the normal inline call return site, in between fp->prev being popped and the return type barrier.
Attachment #648930 - Attachment is obsolete: true
Attachment #649802 - Flags: review?(jdemooij)
Comment on attachment 649802 [details] [diff] [review]
fix

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

::: js/src/jit-test/tests/ion/testPos.js
@@ +30,2 @@
>      assertEq(f_double("-2.147483648"), -2.147483648);
> +    // assertEq(f_double("2.147483647"), 2.147483647);

Please revert the changes to this file.
Attachment #649802 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/c1f46fa5a67d
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.