807380 - IonMonkey: EnterAtBranch defaults to non-constructing

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

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).

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

I'm not sure what invariants this could break, but it seems like something we should fix.

Comment 2

[Hannes Verschore [:h4writer]]

Attachment: 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?

Comment 3

[Sean Stangl [:sstangl]]

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.

Comment 4

[Hannes Verschore [:h4writer]]

(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...

Comment 5

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e692328bf6a3

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e692328bf6a3