Assertion failure: getSlotRef(EVAL).isUndefined(), at js/src/vm/GlobalObject.h:144

RESOLVED FIXED in Firefox 66

Status

()

--
critical
RESOLVED FIXED
2 months ago
a month ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla67
x86_64
Linux
assertion, jsbugmon, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 unaffected, firefox66 fixed, firefox67 fixed)

Details

(Whiteboard: [jsbugmon:])

Attachments

(2 attachments)

(Reporter)

Description

2 months ago

The following testcase crashes on mozilla-central revision 024bef408a88 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

Object.setPrototypeOf(evalcx("lazy"), {});

Backtrace:

#0 js::GlobalObject::setOriginalEval (this=0x13525a18b060, evalobj=<optimized out>) at js/src/vm/GlobalObject.h:144
#1 FinishObjectClassInit (cx=0x7fe5d2017000, ctor=..., proto=...) at js/src/builtin/Object.cpp:2154
#2 0x000055723c2e5cb4 in js::GlobalObject::resolveConstructor (cx=0x7fe5d2017000, global=..., key=JSProto_Object, mode=<optimized out>) at js/src/vm/GlobalObject.cpp:281
#3 0x000055723c359521 in js::GlobalObject::ensureConstructor (cx=<optimized out>, key=JSProto_Object, global=...) at js/src/vm/GlobalObject.h:169
#4 js::SetPrototype (cx=0x7fe5d2017000, obj=..., proto=..., result=...) at js/src/vm/JSObject.cpp:2803
#5 0x000055723c33eec2 in js::SetPrototype (cx=0x7fe5d2017000, obj=..., proto=...) at js/src/vm/JSObject.cpp:2846
/snip

For detailed crash information, see attachment.

(Reporter)

Comment 2

2 months ago

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/6ec84030fb70
user: Jan de Mooij
date: Tue Jan 15 20:03:43 2019 +0000
summary: Bug 1520093 - Make evalcx work with same-compartment realms. r=jorendorff

Jan, is bug 1520093 a likely regressor?

Flags: needinfo?(jdemooij)
(Assignee)

Updated

2 months ago
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
(Assignee)

Comment 3

2 months ago

In js::SetPrototype we call GlobalObject::ensureConstructor. I think this is
only a problem for evalcx because other globals have an immutable prototype
chain.

Updated

2 months ago
Whiteboard: [jsbugmon:update] → [jsbugmon:]

Comment 4

2 months ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 5

2 months ago
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0eb6b611ee2b
Ensure we're in the global's realm in GlobalObject::resolveConstructor. r=jorendorff

Comment 6

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox67: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Is there a user impact which justifies backport consideration here?

status-firefox65: --- → unaffected
status-firefox66: --- → affected
status-firefox-esr60: --- → unaffected
Flags: needinfo?(jdemooij)
Flags: in-testsuite+
(Assignee)

Comment 8

a month ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)

Is there a user impact which justifies backport consideration here?

Probably not; this particular instance is definitely shell-only. However it's hard to say for sure and the fix is very safe, so I'll request uplift just in case.

Flags: needinfo?(jdemooij)
(Assignee)

Comment 9

a month ago

Comment on attachment 9041184 [details]
Bug 1524707 - Ensure we're in the global's realm in GlobalObject::resolveConstructor. r?jorendorff!

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

same-compartment-realms

User impact if declined

Maybe correctness issues. It's hard to say whether the browser is actually affected.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Very safe/small fix.

String changes made/needed

None

Attachment #9041184 - Flags: approval-mozilla-beta?

Comment on attachment 9041184 [details]
Bug 1524707 - Ensure we're in the global's realm in GlobalObject::resolveConstructor. r?jorendorff!

Fix for potential crash found by fuzzing.
Let's uplift for beta 8.

Attachment #9041184 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 11

a month ago
bugherderuplift
status-firefox66: affected → fixed
You need to log in before you can comment on or make changes to this bug.