IonMonkey: EnterAtBranch defaults to non-constructing

RESOLVED FIXED in mozilla19

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

unspecified
mozilla19
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:p1])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
In the code the function EnterAtBranch set constructing=false, even if the function is constructing. It should also look at fp->isConstructing() (or what it is called exactly).
(Assignee)

Updated

6 years ago
Assignee: general → hv1989
I'm not sure what invariants this could break, but it seems like something we should fix.
Whiteboard: [ion:p1]
(Assignee)

Comment 2

6 years ago
Created attachment 677617 [details] [diff] [review]
Report to the compiler the function is constructing when entering at a branch

I'm also not sure what implications this possible could have. Jit-script ran just fine on 64bit non and non-debug --ion. Do I need to test more? Or should we even try to fuzz test this before landing, because we both don't know the implications?
Attachment #677617 - Flags: review?(dvander)
Attachment #677617 - Flags: review?(dvander) → review+
This has no effect -- Ion functions are intended to never have to specially know whether or not they are called as constructors. We could just stop threading the constructing bits through.

It does occur to me, however, that JM may not gracefully handle calling an Ion function as a constructor -- Ion assumes that the caller handles creating |this| and fixing up any primitive return values. I'm not sure how JM->IM calls work, but it's worth verifying that they're valid in such a case.
(Assignee)

Comment 4

6 years ago
(In reply to Sean Stangl from comment #3)
> This has no effect -- Ion functions are intended to never have to specially
> know whether or not they are called as constructors. We could just stop
> threading the constructing bits through.

I know "mjit::GetPICSingleShape" uses it, to know which JIT info to return. Is possible the only place...

> It does occur to me, however, that JM may not gracefully handle calling an
> Ion function as a constructor -- Ion assumes that the caller handles
> creating |this| and fixing up any primitive return values. I'm not sure how
> JM->IM calls work, but it's worth verifying that they're valid in such a
> case.

I see JM uses 2 different scripts for constructing vs non-constructing. I don't see that for IM ?! IM compiles one and uses this for both constructing and non-constructing. I.e. there is no check on entering ion to make sure we use the constructing script only for constructing script. This probably doesn't bite us, because most of the time function are always called constructing or not constructing. And if we would have the wrong script atm we only have wrong information from MJIT and will bail and recompile (probably with the right information). Could possibly introduce errors later on if we start to relying on constructing bit more often...
Depends on: 808098
https://hg.mozilla.org/mozilla-central/rev/e692328bf6a3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.