IonMonkey: Forbid JITing annotated frames

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:p1:fx18])

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
IonMonkey cannot annotate frames, so js_setframeannotation may not work. Current plan is to rename the API to not take an fp, and to only operate on the topmost frame. From there, we can make the API invalidate and forbid ever JITing the calling function.
(Assignee)

Comment 1

5 years ago
Err, this isn't totally good enough since invalidation does not give us an fp. But maybe as part of the bailout we can move the annotation from the entry fp (which could no have had one at entry) to the bailed fp.
(Assignee)

Comment 2

5 years ago
Created attachment 634198 [details] [diff] [review]
patch for m-i

On try, SetFrameAnnotation's |fp| was always |cx->fp()|, so this should work.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #634198 - Flags: review?(mrbkap)
(Assignee)

Comment 3

5 years ago
Created attachment 634200 [details] [diff] [review]
patch for ionmonkey

This patch disables IonMonkey for annotated frames.
Attachment #634200 - Flags: review?(jdemooij)
Comment on attachment 634198 [details] [diff] [review]
patch for m-i

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

::: caps/src/nsScriptSecurityManager.cpp
@@ +2742,5 @@
>          return NS_ERROR_FAILURE; // XXX better error code?
>      }
>      if (NS_FAILED(principal->EnableCapability(capability, &annotation)))
>          return NS_ERROR_FAILURE;
> +    JS_SetTopFrameAnnotation(cx, annotation);

Is there a way we can assert here or in JS_SetTopFrameAnnotation that fp is the top frame?

::: js/src/jsdbgapi.cpp
@@ +524,2 @@
>  {
> +    cx->fp()->setAnnotation(annotation);

Is it worth asserting that we have a frame?
Attachment #634198 - Flags: review?(mrbkap) → review+
Comment on attachment 634200 [details] [diff] [review]
patch for ionmonkey

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

Looks good, I'm glad this didn't get too complicated.
Attachment #634200 - Flags: review?(jdemooij) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [ion:p1:fx18]
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/f6c3f006b57a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.