Bug 186196 – [JSC] Correct values and members of JSBigInt appropriately

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Correct values and members of JSBigInt appropriately

[Yusuke Suzuki]

Created attachment 341763 [details]
Patch

[Darin Adler]

Comment on attachment 341763 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:11
> +        2. JSBigInt::allocationSize should be annotated with `inline`.

Does this have any effect? Recently I’ve discovered that for the latest versions of clang, at least, "inline" really means "permission to define this in multiple translation units" or in other words "this function is in a header", and isn’t having an effect on what actually gets inlined.

> Source/JavaScriptCore/runtime/JSBigInt.cpp:1309
> -    ASSERT(msdTopBit == (xBitLength - 1) % digitBits);
> +    ASSERT(msdTopBit == static_cast<int>((xBitLength - 1) % digitBits));

Why this change?

[Yusuke Suzuki]

Comment on attachment 341763 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/ChangeLog:11
>> +        2. JSBigInt::allocationSize should be annotated with `inline`.
> 
> Does this have any effect? Recently I’ve discovered that for the latest versions of clang, at least, "inline" really means "permission to define this in multiple translation units" or in other words "this function is in a header", and isn’t having an effect on what actually gets inlined.

Yeah, in GCC, this is effective. Maybe, this is because this `allocationSize()` function is static class member function, so it can be called outside of JSBigInt.cpp.
This `inline` prevents `allocationSize()` function from being called outside of this JSBigInt.cpp.

  c6400e:       48 8b 87 d0 ff 00 00    mov    0xffd0(%rdi),%rax
  c64015:       48 89 fd                mov    %rdi,%rbp
  c64018:       48 85 c0                test   %rax,%rax
  c6401b:       74 03                   je     c64020 <JSC::JSBigInt::createWithLength(JSC::VM&, unsigned int)+0x20>
  c6401d:       4c 8b 28                mov    (%rax),%r13
  c64020:       44 89 e7                mov    %r12d,%edi
  c64023:       e8 68 69 70 ff          callq  36a990 <JSC::JSBigInt::allocationSize(unsigned int)@plt>
  c64028:       48 3d 60 1f 00 00       cmp    $0x1f60,%rax
  c6402e:       76 50                   jbe    c64080 <JSC::JSBigInt::createWithLength(JSC::VM&, unsigned int)+0x80>
  c64030:       48 8d bd 40 36 00 00    lea    0x3640(%rbp),%rdi
  c64037:       45 31 c0                xor    %r8d,%r8d
  c6403a:       31 c9                   xor    %ecx,%ecx
  c6403c:       48 89 c2                mov    %rax,%rdx

>> Source/JavaScriptCore/runtime/JSBigInt.cpp:1309
>> +    ASSERT(msdTopBit == static_cast<int>((xBitLength - 1) % digitBits));
> 
> Why this change?

It emits warnings with GCC since this comparison is signed v.s. unsigned.

[Yusuke Suzuki]

Committed r232401: <https://trac.webkit.org/changeset/232401>

[Radar WebKit Bug Importer]

<rdar://problem/40729485>