1902581 - MOZ_CRASH([unhandlable oom] SavedStacksMetadataBuilder)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[anbu]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:125.0) Gecko/20100101 Firefox/125.0

Steps to reproduce:

Test case:

function foo() {
    return foo;
}
Object.defineProperty(foo, "sameZoneAs", { get: foo });
class Bar extends foo {
}
const t1 = this.newGlobal(Bar).Debugger;
const apple = t1(Uint8Array).memory;
apple.trackingAllocationSites = apple;

const obj = {
    valueOf() {
        return this >> t1;
    },
};
obj.valueOf();

The test engine version:
4582d9

Build commands:

cd debug_xx

/bin/sh ../../gecko-dev/js/src/configure --enable-debug --disable-optimize --disable-shared-js --disable-tests

make -j 12

Actual results:

Exec result:

#01: js::AutoEnterOOMUnsafeRegion::crash_impl(char const*)[./js +0x2904380]
#02: ???[./js +0x2445d69]
#03: ???[./js +0x2aeea12]
#04: JS::Realm::setNewObjectMetadata(JSContext*, JS::Handle<JSObject*>)[./js +0x2ab7cc5]
#05: ???[./js +0x25bd2d0]
#06: ???[./js +0x25c49c3]
#07: ???[./js +0x2913497]
#08: ???[./js +0x29130b0]
#09: ???[./js +0x24a1885]
#10: ???[./js +0x2879f9a]
#11: ???[./js +0x286c26d]
#12: ???[./js +0x2e79f23]
#13: ???[./js +0x2870399]
#14: ???[./js +0x28700b0]
#15: JS_ReportErrorNumberASCIIVA(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, __va_list_tag*)[./js +0x2e49525]
#16: JS_ReportErrorNumberASCII(JSContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, ...)[./js +0x2e2d76b]
#17: JSContext::onOverRecursed()[./js +0x28feff8]
#18: js::ReportOverRecursed(JSContext*)[./js +0x28ff17a]
#19: ???[./js +0x2603279]
#20: ???[./js +0x25d90d6]
#21: ???[./js +0x25da063]
#22: ???[./js +0x25da5d9]
#23: ???[./js +0x25da789]
#24: ???[./js +0x287af8a]
#25: ???[./js +0x291e12b]
#26: JS::OrdinaryToPrimitive(JSContext*, JS::Handle<JSObject*>, JSType, JS::MutableHandle<JS::Value>)[./js +0x291dd51]
#27: ???[./js +0x291e80d]
#28: ???[./js +0x26188f7]
#29: ???[./js +0x2e86cea]
#30: ???[./js +0x260b761]
#31: ???[./js +0x2e8755c]
#32: ???[./js +0x2618861]
#33: ???[./js +0x25f660d]
#34: ???[./js +0x25fc672]
#35: ???[./js +0x35b783d]
#36: ??? (???:???)
Segmentation fault (core dumped)

Comment 1

[Iain Ireland [:iain]]

Thanks for the report!

Broadly speaking, MOZ_CRASH([unhandlable oom]) bugs are usually not interesting. They happen when we run out of memory in a situation where we can't / won't recover. We already recover from OOM much more than any other engine, so we're not worried about webcompat issues.

This particular issue, though, is potentially worth fixing. Here's a simplified testcase:

let g = newGlobal({sameZoneAs: this});
let dbg = g.Debugger(this);
dbg.memory.trackingAllocationSites = true;

function foo() {
  foo()
}
try {
  foo();
} catch {}

What's happening here is that we're triggering a stack overflow while being watched by a debugger that is tracking allocation sites. When we call DebugAPI::onLogAllocationSite, we eventually end up here in getNonWrapperObjectForCurrentCompartment. This code calls a preWrap callback if one exists. We were apparently worried about infinite recursion here, so we check the stack limit before calling the callback. However, we do so before we check whether the callback even exists. Because we already triggered stack overflow, we immediately return failure. The caller assumes we ran out of memory and triggers the crash.

We can fix this in at least some cases by moving the stack check inside the conditional.

Searching through crashstats for stacks containing both setNewObjectMetadata and ReportOverRecursed, it doesn't look like this happens in the wild.

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1902581: Don't check for over-recursion unless there's a preWrap hook r=jandem

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:iain, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f60815ad4df6
Don't check for over-recursion unless there's a preWrap hook r=jandem

Comment 5

[tszentpeteri]

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