Closed Bug 683714 Opened 9 years ago Closed 7 years ago

"Assertion failure: fun->getProto() == proto" in CloneFunctionObject

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase, Whiteboard: fixed-in-jaegermonkey)

Attachments

(3 files)

Assertion failure: fun->getProto() == proto, at js/src/jsfun.h:474
Attached file stack trace
Whiteboard: js-triage-needed
I claim this is the same bug we hit on Linux M1: <https://tbpl.mozilla.org/php/getParsedLog.php?id=6263993>
Attached patch patchSplinter Review
Kind of bizarre.

We put a method barrier on a function created in a compileAndGo script, after the script executed XPConnect reparented certain objects so that for the fun in the method barrier, fun->getGlobal() != fun->getProto()->getGlobal() (the parent window of the function changed, but not that of its prototype).  The method barrier broke, and when cloning the object we tried to make a function whose prototype was the Function.prototype of the *new* window.  TI assertions caught this (changing the type/proto of a function when cloning it for a method barrier breaks TI assumptions).  The fix adds a variant of CloneFunctionObject for use in barriers, which always preserves parent/proto.
Attachment #558523 - Flags: review?(jorendorff)
Whiteboard: js-triage-needed → js-triage-done
Pushed to JM for further fuzz testing.

http://hg.mozilla.org/projects/jaegermonkey/rev/50d4f6fa00ce
Whiteboard: js-triage-done → fixed-in-jaegermonkey
Comment on attachment 558523 [details] [diff] [review]
patch

This worries me for a couple reasons.

Fundamentally, I still don't see exactly how this situation arises.

But also, if XPConnect can reparent an object that is on the scope chain of a compileAndGo function *before* it is cloned, I don't see why it couldn't reparent it *after* it is cloned. That would change the global of a compileAndGo function, which would break engine invariants, right? Maybe not, if only script->global() matters now?

Also, the patch needs a test. I think a jsapi-test would be better than a full-browser test because the asserting test case Jesse found doesn't exactly make it clear what's going on.

Also, I'm worried that SetParent has rules (related to method barriers) that it doesn't assert, even if those rules don't come into play in this testcase. (The patch looks like an OK way to fix this if XPConnect really isn't breaking any engine invariants, but I don't know if that's true.)

I'm a little worried that compile-and-go might have issues in combination with scope chains containing multiple objects.
Attachment #558523 - Flags: review?(jorendorff)
Did this ever land, or, more likely, was it obsoleted by changes in SpiderMonkey?
Flags: needinfo?(bhackett1024)
Yes, in rev 50d4f6fa00ce.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(bhackett1024)
Resolution: --- → FIXED
Can we land the test?
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.