Bug 190033 – [BigInt] BigInt.proptotype.toString is broken when radix is power of 2

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Caio Lima]

When we have a BigInt with length >= 2, the call to BigInt.prototype.toString with a radix that is power of 2 causes the following crash:

ASSERTION FAILED: chunkDivisor
Source/JavaScriptCore/runtime/JSBigInt.cpp(1254) : static WTF::String JSC::JSBigInt::toStringGeneric(JSC::ExecState *, JSC::JSBigInt *, unsigned int)
1   0x1018a83e9 WTFCrash
2   0x100005b5b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x10140c670 JSC::JSBigInt::toStringGeneric(JSC::ExecState*, JSC::JSBigInt*, unsigned int)
4   0x10140c33c JSC::JSBigInt::toString(JSC::ExecState*, unsigned int)
5   0x10133acf9 JSC::bigIntProtoFuncToString(JSC::ExecState*)
6   0x3576d0ed177
7   0x10118c9d4 llint_entry
8   0x101184300 vmEntryToJavaScript
9   0x101099f4a JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
10  0x101099519 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
11  0x101371c2f JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
12  0x10002238d runInteractive(GlobalObject*)
13  0x100007a77 int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
14  0x10000650f jscmain(int, char**)
15  0x10000646e main
16  0x7fff70624015 start
17  0x2
Process 91544 stopped

[Caio Lima]

Created attachment 350960 [details]
Patch

[Caio Lima]

Created attachment 351012 [details]
Patch

[Caio Lima]

Created attachment 351068 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 351068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351068&action=review

r=me with nits.

> Source/WTF/wtf/MathExtras.h:580
> +#if COMPILER(GCC_OR_CLANG)

Use `COMPILER(GCC_COMPATIBLE)`.

[Caio Lima]

Created attachment 351216 [details]
Patch

[Caio Lima]

Comment on attachment 351216 [details]
Patch

Thank you very much for the review!

[WebKit Commit Bot]

Comment on attachment 351216 [details]
Patch

Clearing flags on attachment: 351216

Committed r236647: <https://trac.webkit.org/changeset/236647>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44895609>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 190124

[Caio Lima]

Created attachment 351242 [details]
Patch

[Yusuke Suzuki]

(In reply to Caio Lima from comment #11)
> Created attachment 351242 [details]
> Patch

What is the changed part from the previous one?

[Caio Lima]

(In reply to Yusuke Suzuki from comment #12)
> (In reply to Caio Lima from comment #11)
> > Created attachment 351242 [details]
> > Patch
> 
> What is the changed part from the previous one?

ctz32 was wrong implemented. It was 
```
if (!number)
   return __builtin_ctz(number);

return 32;
```

Which return 32 for every number but 0.

I changed to:
```
if (number) // No not "!"
   return __builtin_ctz(number);

return 32;
```

[Caio Lima]

Comment on attachment 351242 [details]
Patch

From previous patch, I changed ctz32 (Source/WTF/wtf/MathExtras.h:581) because previous version was wrong. Asking for review again before upstreaming the patch.

[Dawei Fenton (:realdawei)]

(In reply to WebKit Commit Bot from comment #7)
> Comment on attachment 351216 [details]
> Patch
> 
> Clearing flags on attachment: 351216
> 
> Committed r236647: <https://trac.webkit.org/changeset/236647>

32bit JSC crashing after this revision.

Sample output:
https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/2647/steps/webkit-32bit-jsc-test/logs/stdio

stress/big-int-to-string.js.big-int-enabled: 1   0x27dd2b WTFCrash
stress/big-int-to-string.js.big-int-enabled: 2   0x280b3b WTF::CrashOnOverflow::crash()
stress/big-int-to-string.js.big-int-enabled: 3   0x280b2b WTF::CrashOnOverflow::overflowed()
stress/big-int-to-string.js.big-int-enabled: 4   0x2bf7f7 WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>::at(unsigned long)
stress/big-int-to-string.js.big-int-enabled: 5   0x2bbcf4 WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>::operator[](unsigned long)
stress/big-int-to-string.js.big-int-enabled: 6   0x12de7b1 JSC::JSBigInt::toStringBasePowerOfTwo(JSC::ExecState*, JSC::JSBigInt*, unsigned int)
stress/big-int-to-string.js.big-int-enabled: 7   0x12de1a4 JSC::JSBigInt::toString(JSC::ExecState*, unsigned int)
stress/big-int-to-string.js.big-int-enabled: 8   0x11ee681 JSC::bigIntProtoFuncToString(JSC::ExecState*)
stress/big-int-to-string.js.big-int-enabled: 9   0x54e0f0e1
stress/big-int-to-string.js.big-int-enabled: 10  0x536057 llint_entry
stress/big-int-to-string.js.big-int-enabled: 11  0x52febf vmEntryToJavaScript
stress/big-int-to-string.js.big-int-enabled: 12  0xef1c49 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
stress/big-int-to-string.js.big-int-enabled: 13  0xef10a2 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
stress/big-int-to-string.js.big-int-enabled: 14  0x122b2eb JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
stress/big-int-to-string.js.big-int-enabled: 15  0x10889f runWithOptions(GlobalObject*, CommandLine&, bool&)
stress/big-int-to-string.js.big-int-enabled: 16  0xd8c7a jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
stress/big-int-to-string.js.big-int-enabled: 17  0xba3aa int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
stress/big-int-to-string.js.big-int-enabled: 18  0xb8b40 jscmain(int, char**)
stress/big-int-to-string.js.big-int-enabled: 19  0xb8a67 main
stress/big-int-to-string.js.big-int-enabled: 20  0xa73f4611 start
stress/big-int-to-string.js.big-int-enabled: test_script_2250: line 2: 98930 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useBigInt\=true --useFTLJIT\=true big-int-to-string.js )
stress/big-int-to-string.js.big-int-enabled: ERROR: Unexpected exit code: 139

[Yusuke Suzuki]

Comment on attachment 351242 [details]
Patch

r=me

[Caio Lima]

Comment on attachment 351242 [details]
Patch

Thx for the review

[WebKit Commit Bot]

Comment on attachment 351242 [details]
Patch

Clearing flags on attachment: 351242

Committed r236737: <https://trac.webkit.org/changeset/236737>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.