1782562 - Assertion failure: newElts <= (2147483647) && newElts * EltSize <= (2147483647) (invalid Vector size (see bug 510319)), at dist/include/mozilla/Vector.h:137

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

var z = "1";
try {
    f = function (x) {
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
        (function(){});
    };
    for (let i = 0; i < 99; i++)
        z += z
} catch (e) {}
uneval(this);

Thread 1 "js-dbg-32-linux" received signal SIGSEGV, Segmentation fault.
0x57afc501 in mozilla::Vector<char16_t, 32u, js::StringBufferAllocPolicy>::growStorageBy (this=0xffff9e48, aIncr=536891304) at /home/skygentoo/shell-cache/js-dbg-32-linux-x86_64-bc1d41e88ae3/objdir-js/dist/include/mozilla/Vector.h:136
136	  MOZ_ASSERT(newElts <= PTRDIFF_MAX && newElts * EltSize <= PTRDIFF_MAX,
(gdb) bt
#0  0x57afc501 in mozilla::Vector<char16_t, 32u, js::StringBufferAllocPolicy>::growStorageBy (this=0xffff9e48, aIncr=536891304) at /home/skygentoo/shell-cache/js-dbg-32-linux-x86_64-bc1d41e88ae3/objdir-js/dist/include/mozilla/Vector.h:136
#1  0x57ba30f0 in mozilla::Vector<char16_t, 32u, js::StringBufferAllocPolicy>::reserve (this=0xffff9e48, aRequest=536891304) at /home/skygentoo/shell-cache/js-dbg-32-linux-x86_64-bc1d41e88ae3/objdir-js/dist/include/mozilla/Vector.h:1115
#2  0x58b69135 in js::StringBuffer::inflateChars (this=0xffffa110) at /home/skygentoo/trees/mozilla-central/js/src/util/StringBuffer.cpp:67
#3  0x57afd15e in js::StringBuffer::append (this=0xffffa110, str=0xf6900010) at /home/skygentoo/trees/mozilla-central/js/src/util/StringBuffer.h:405
#4  0x57c55d72 in js::ObjectToSource(JSContext*, JS::Handle<JSObject*>)::$_3::operator()(JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, PropertyKind) const (this=0xffffa090, id=id@entry=..., val=..., kind=PropertyKind::Normal) at /home/skygentoo/trees/mozilla-central/js/src/builtin/Object.cpp:468
#5  0x57c54448 in js::ObjectToSource (cx=<optimized out>, obj=...) at /home/skygentoo/trees/mozilla-central/js/src/builtin/Object.cpp:514
#6  0x57c6948d in obj_toSource (cx=0xf6a15100, argc=0, vp=0xffffa3d8) at /home/skygentoo/trees/mozilla-central/js/src/builtin/Object.cpp:169
#7  0x57bcd800 in CallJSNative (cx=0xf6a15100, native=0x57c69360 <obj_toSource(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:417
#8  0x57bc0430 in js::InternalCallOrConstruct (cx=0xf6a15100, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:505
#9  0x57bc0e9c in InternalCall (cx=0xf6a15100, args=..., reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:572
#10 0x57bc106e in js::Call (cx=0xf6a15100, fval=..., thisv=..., args=..., rval=..., reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:604
#11 0x57d687ec in js::Call (cx=0xf6a15100, fval=..., thisObj=0xf6438040, rval=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.h:109
#12 0x57fbc9a3 in js::ValueToSource (cx=0xf6a15100, v=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/ToSource.cpp:191
#13 0x57e9303b in str_uneval (cx=0xf6a15100, argc=1, vp=0xffffa948) at /home/skygentoo/trees/mozilla-central/js/src/builtin/String.cpp:374
#14 0x57bcd800 in CallJSNative (cx=0xf6a15100, native=0x57e92fe0 <str_uneval(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:417
#15 0x57bc0430 in js::InternalCallOrConstruct (cx=0xf6a15100, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:505
#16 0x57bc0e9c in InternalCall (cx=0xf6a15100, args=..., reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:572
#17 0x57bc0e14 in js::CallFromStack (cx=0xf6a15100, args=..., reason=js::CallReason::Call) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:577
#18 0x585bb893 in js::jit::DoCallFallback (cx=0x567e2484, frame=0xffffa990, stub=0xf68faf7c, argc=1, vp=0xffffa948, res=...) at /home/skygentoo/trees/mozilla-central/js/src/jit/BaselineIC.cpp:1583
#19 0xe81f038b in ?? ()
#20 0xe81fe117 in ?? ()
#21 0xe81ea7f3 in ?? ()
#22 0x585c3dc6 in EnterBaseline (cx=<optimized out>, data=...) at /home/skygentoo/trees/mozilla-central/js/src/jit/BaselineJIT.cpp:142
#23 js::jit::EnterBaselineInterpreterAtBranch (cx=0xf6a15100, fp=0xf6aeb010, pc=0xf6afc213 "\223\004") at /home/skygentoo/trees/mozilla-central/js/src/jit/BaselineJIT.cpp:198
#24 0x57badf14 in Interpret (cx=0x567e2484, state=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:2190
#25 0x57bad162 in js::RunScript (cx=<optimized out>, state=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:389
#26 0x57bc268f in js::ExecuteKernel (cx=0xf6a15100, script=..., envChainArg=..., evalInFrame=..., result=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:783
#27 0x57bc2a56 in js::Execute (cx=0xf6a15100, script=..., envChain=..., rval=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/Interpreter.cpp:815
#28 0x57d21743 in ExecuteScript (cx=0xf6a15100, envChain=..., script=..., rval=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/CompilationAndEvaluation.cpp:516
#29 0x57d21915 in JS_ExecuteScript (cx=0xf6a15100, scriptArg=...) at /home/skygentoo/trees/mozilla-central/js/src/vm/CompilationAndEvaluation.cpp:540
#30 0x57af25b1 in RunFile (cx=0xf6a15100, filename=<optimized out>, file=0xf781c120, compileMethod=CompileUtf8::DontInflate, compileOnly=<optimized out>) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:1067
#31 0x57af1dcf in Process (cx=0xf6a15100, filename=<optimized out>, forceTTY=<optimized out>, kind=FileScript) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:1655
#32 0x57ac0025 in ProcessArgs (cx=0xf6a15100, op=0xffffca00) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:11038
#33 Shell (cx=0xf6a15100, op=0xffffca00) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:11739
#34 0x57ab8ef0 in main (argc=6, argv=0xffffcba4) at /home/skygentoo/trees/mozilla-central/js/src/shell/js.cpp:12846
(gdb)

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b014f84dacd0
user:        Steve Fink
date:        Wed Jul 27 22:59:51 2022 +0000
summary:     Bug 1774733 - Allow AllocPolicy to determine Vector growth policy, and be aggressive about StringBuilder allocation strategy to reduce memcpy'ing. r=jandem

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with 'CXX="clang++ -msse2 -mfpmath=sse"' PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig AR=ar 'CC="clang -msse2 -mfpmath=sse"' sh ./configure --host=x86_64-pc-linux-gnu --target=i686-pc-linux --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-bootstrap --disable-tests, tested on m-c rev bc1d41e88ae3.

Note that this may be slightly intermittent. Not sure if this is s-s. Steve, is bug 1774733 a likely regressor?

Comment 1

[Andrew McCreight [:mccr8]]

Same assertion as bug 1782468, so maybe a dupe.

Comment 2

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

Set release status flags based on info from the regressing bug 1774733

Comment 4

[Steve Fink [:sfink] [:s:]]

Actually, I'm going to un-dupe these for now. I assumed too quickly that the obvious fix would fix all of these, and it doesn't. They may end up being duplicates after all, but I'm not confident anymore. (One may have been entirely due to an added assertion, but it's behaving weirdly so even there I'm not sure atm.)

Comment 5

[Daniel Veditz [:dveditz]]

Steve, since you thought you had a fix: what would be the security impact if it was the problem you think it was? Looks like an invalid pointer?

Comment 6

[Daniel Veditz [:dveditz]]

For now guessing based on the severity of our old bug (note: we've since redefined "critical" somewhat. That old bug would now be considered sec-high).

Comment 8

[Steve Fink [:sfink] [:s:]]

(In reply to Daniel Veditz [:dveditz] from comment #5)

Steve, since you thought you had a fix: what would be the security impact if it was the problem you think it was? Looks like an invalid pointer?

Inheriting severity from bug 510319 makes sense.

The issue predates bug 1774733. I added an equivalent assert to the original code and it fires with both the testcase here and from bug 1782468. (Or at least, with the testcase here it does 50% of the time. The other 50% it throws an out of memory exception. It probably depends on the placement of heap allocations within the 32-bit virtual address space.)

The problem was introduced in bug 685783, which replaced a (now removed) tl::UnsafeRangeSizeMask<T>::result to check for the problem. The new code instead checked whether it was safe to multiply the size by two (a conservative test to see if RoundUpPow2 might cause problems), but the definition of "safe" is that it will fit into a size_t, which is less restrictive than whether it fits within PTRDIFF_MAX. The other similar check tests whether multiplying the size by four will overflow, which is safe because anything that passes that test will satisfy both restrictions.

The one and only value that this code will ever return that would fail the assert is 0x80000000, because it's the only value that (1) fits into 32 bits, (2) is bigger than PTRDIFF_MAX, and (3) can be the result of RoundUpPow2. Still, unless I'm missing something that is still problematic because it means a ptrdiff_t representing the difference between the beginning and end of the Vector will produce undefined behavior.

Comment 9

[Andrew McCreight [:mccr8]]

Did you mean to mark this fixed? If so, what was it fixed by?

Comment 10

[Steve Fink [:sfink] [:s:]]

Er, I'm not sure how I resolved this bug by posting that last change. Reopening.

I am not at all sure this can be a problem in practice. It would return 0x80000000, which would be used as a size parameter to malloc or realloc. I'm not sure you can ever allocate half the 32-bit address space in one contiguous allocation. I would expect malloc/realloc to fail with oom. On my system, I tried it and that is exactly what happens.

On 32-bit Windows, I'm pretty sure this is guaranteed, since you really only get a 2GB address space. For a 32-bit application running on a 64-bit system, you get more than that, but people seem to say that you'll never be able to allocate more than about 1.5GB.

I will revise my opinion: this is sec-medium at the most, and probably not security sensitive (or even a bug) at all.

Comment 11

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1782562 - be more conservative in max Vector size

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed: https://hg.mozilla.org/integration/autoland/rev/7519215902c940727ed9fba043e8820b6133fa14

Backed out for causing spidermonkey bustages on bug1782468-ptrdiff-veclen.js:
https://hg.mozilla.org/integration/autoland/rev/a3d51a8dab4ea1215e1f56394fda873eeb20f585

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=JboMVYOlQsGrmjR2WwiyLg.0&revision=7519215902c940727ed9fba043e8820b6133fa14&searchStr=spidermonkey
Failure log: https://treeherder.mozilla.org/logviewer?job_id=386687150&repo=autoland
Failure line:

js/src/jit-test/tests/bug1782468-ptrdiff-veclen.js | /builds/worker/checkouts/gecko/js/src/jit-test/tests/bug1782468-ptrdiff-veclen.js:1:27 SyntaxError: missing ] after element list: (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [5.1 s]

Comment 13

[Cristian Tuns]

Backed out for causing SM bustages.

• Backout link
• Push with failures
• Failure Log
• Failure lines: /builds/worker/checkouts/gecko/js/src/jit-test/tests/bug1782562-toSource-veclen.js:28:13 Error: Assertion failed: got false, expected true
  TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/bug1782562-toSource-veclen.js | /builds/worker/checkouts/gecko/js/src/jit-test/tests/bug1782562-toSource-veclen.js:28:13 Error: Assertion failed: got false, expected true (code 3, args "--blinterp-eager") [7.1 s]

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

be more conservative in max Vector size r=jandem
https://hg.mozilla.org/integration/autoland/rev/e767b44e5c74df78aec8d07452351c6c94a00f59
https://hg.mozilla.org/mozilla-central/rev/e767b44e5c74

Comment 15

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

The patch landed in nightly and beta is affected.
:sfink, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox104 to wontfix.

For more information, please visit auto_nag documentation.

Comment 16

[Steve Fink [:sfink] [:s:]]

I don't think it's necessary. It's sort of like an overzealous assertion, but it's not really overzealous because the logic needed to prove that it's ok is pretty gnarly and could become invalid on a future 32-bit architecture. It's safer to avoid the problem, but unlikely to make a difference in practice today.