Closed Bug 1319952 Opened 3 years ago Closed 3 years ago

Assertion failure: isNurseryAllocAllowed(), at js/src/gc/Allocator.cpp:79

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- verified

People

(Reporter: gkw, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, jsbugmon, testcase, Whiteboard: [jsbugmon:update,ignore])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 47f42f21541b (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --ion-offthread-compile=off --no-baseline --no-ion):

compile.__proto__.toString = (function() {
    try {
        compile + function(){};
    } catch (e) {}
    offThreadCompileScript("");
});
Math.round(function(){});


Backtrace:

0   js-dbg-64-dm-clang-darwin-47f42f21541b	0x0000000105e5908b JSObject* js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1>(JSContext*, unsigned long, unsigned long, js::Class const*) + 267 (Allocator.cpp:79)
1   js-dbg-64-dm-clang-darwin-47f42f21541b	0x0000000105e58c76 JSObject* js::Allocate<JSObject, (js::AllowGC)1>(js::ExclusiveContext*, js::gc::AllocKind, unsigned long, js::gc::InitialHeap, js::Class const*) + 198 (Allocator.cpp:51)
2   js-dbg-64-dm-clang-darwin-47f42f21541b	0x00000001059f154b JSObject::create(js::ExclusiveContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::ObjectGroup*>) + 731 (jsobjinlines.h:378)
3   js-dbg-64-dm-clang-darwin-47f42f21541b	0x00000001059b7190 NewObject(js::ExclusiveContext*, JS::Handle<js::ObjectGroup*>, js::gc::AllocKind, js::NewObjectKind, unsigned int) + 432 (jsobj.cpp:651)
4   js-dbg-64-dm-clang-darwin-47f42f21541b	0x00000001059b6922 js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::gc::AllocKind, js::NewObjectKind, unsigned int) + 354 (RootingAPI.h:786)
/snip

For detailed crash information, see attachment.
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/4e6fc50106d3
user:        Tom Schuster
date:        Tue Nov 22 20:53:38 2016 +0100
summary:     Bug 1213341 - Use ordinary objects for Error prototypes. r=arai

Tom, is bug 1213341 a likely regressor?
Blocks: 1213341
Flags: needinfo?(evilpies)
GC is on the stack, so locking s-s just-in-case as a start.
Group: javascript-core-security
I think we can fix this by calling NewFunctionWithProto in ErrorObject::createConstructor with SingletonObject, that should match what GenericCreateConstructor does internally and will prevent us from allocating in the nursery. No idea why this happens though, definitely not something that is obvious.
Flags: needinfo?(evilpies)
Attachment #8813902 - Flags: review?(jcoppeard)
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 34fce7c12173).
Comment on attachment 8813902 [details] [diff] [review]
Make the Error constructors singletons

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

I agree we should make this do the same thing as GenericCreateConstructor and happily that fixes this.

I'm going to file a separate bug to address the reason this happens: we don't want nursery allocations for a compartment that is going to be used off-main-thread, but we're still using the original JSContext* which allows that do happen.  At least we have assertions to show us when it goes wrong.
Attachment #8813902 - Flags: review?(jcoppeard) → review+
See Also: → 1320046
Assignee: nobody → evilpies
Considering that this is only in Nightly I can just land this?
thanks you for fixing!

(In reply to Tom Schuster [:evilpie] from comment #8)
> Considering that this is only in Nightly I can just land this?

https://wiki.mozilla.org/Security/Bug_Approval_Process

> Core-security bug fixes should just be landed by a developer without any explicit approval if: 
> ...
> B) The bug is a recent regression on mozilla-central. This means
>   * A specific regressing check-in has been identified
>   * The developer can (and has) marked the status flags for ESR, Beta, and Aurora as "unaffected"
>   * We have not shipped this vulnerability in anything other than a nightly build

yes, after setting status flags.
https://hg.mozilla.org/mozilla-central/rev/ba097fb49461
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.