Bug 183732 - Drop s_exceptionInstructions static initializer
Summary: Drop s_exceptionInstructions static initializer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-18 19:25 PDT by Joseph Pecoraro
Modified: 2018-03-19 22:35 PDT (History)
13 users (show)

See Also:


Attachments
Patch (2.11 KB, patch)
2018-03-18 22:10 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2018-03-18 19:25:00 PDT
bmalloc has a static initializer. Can we eliminate it?

Test:

    $ cat jsc-test.m
    #include <JavaScriptCore/JavaScriptCore.h>
    int main() {
        [[[JSContext alloc] init] release];
        return 0;
    }

    $ clang -framework JavaScriptCore jsc-test.m -o jsc-test
    $ DYLD_PRINT_INITIALIZERS=1 ./jsc-test
    ...
    dyld: calling initializer function 0x7fff4faaf260 in /System/Library/Frameworks/JavaScriptCore.framework/Versions/A/JavaScriptCore
    ...

I don't know exactly what the initializer is... but here is how far I got tracking it:

    $ otool -l /System/Library/Frameworks/JavaScriptCore.framework/JavaScriptCore | grep __mod_init_func -B1 -A10
    Section
      sectname __mod_init_func
       segname __DATA
          addr 0x0000000000d6d268
          size 0x0000000000000008
        offset 14078568
         align 2^3 (8)
        reloff 0
        nreloc 0
         flags 0x00000009
     reserved1 0
     reserved2 0

    $ lldb /System/Library/Frameworks/JavaScriptCore.framework/JavaScriptCore
    (lldb) memory read 0x0000000000e0a2d0 --format address
    0x00e0a2d0: 0x0000000000d5a8a0

    (lldb) dis -s 0x0000000000d5a8a0 -c 15
    JavaScriptCore`__cpu_indicator_init:
    JavaScriptCore[0xd5a8a0] <+0>:  pushq  %rbp
    JavaScriptCore[0xd5a8a1] <+1>:  pushq  %r15
    JavaScriptCore[0xd5a8a3] <+3>:  pushq  %r14
    JavaScriptCore[0xd5a8a5] <+5>:  pushq  %r13
    JavaScriptCore[0xd5a8a7] <+7>:  pushq  %r12
    JavaScriptCore[0xd5a8a9] <+9>:  pushq  %rbx
    JavaScriptCore[0xd5a8aa] <+10>: pushq  %rax
    JavaScriptCore[0xd5a8ab] <+11>: xorl   %eax, %eax
    JavaScriptCore[0xd5a8ad] <+13>: cmpl   $0x0, 0x103750(%rip)      ; Gigacage::g_wasEnabled + 3
    JavaScriptCore[0xd5a8b4] <+20>: jne    0xd5aed7                  ; <+1591>
    JavaScriptCore[0xd5a8ba] <+26>: xorl   %eax, %eax
    JavaScriptCore[0xd5a8bc] <+28>: movq   %rbx, %rsi
    JavaScriptCore[0xd5a8bf] <+31>: cpuid  
    JavaScriptCore[0xd5a8c1] <+33>: xchgq  %rbx, %rsi
    JavaScriptCore[0xd5a8c4] <+36>: movl   %esi, %r9d

So it appears to be related to Gigacage::g_wasEnabled.
Comment 1 mitz 2018-03-18 20:36:47 PDT
This doesn’t look to be related to Gigacage. __cpu_indicator_init supports __builtin_cpu_supports() and comes from the compiler (see e.g. <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/cpu_model.c>). It may have ended up in JavaScriptCore because something used the builtin or the compiler thought it was needed. It doesn’t seem to be the case in TOT (although TOT has a static initializer for JSC::LLInt::Data::s_exceptionInstructions AFAICT).
Comment 2 Yusuke Suzuki 2018-03-18 22:06:37 PDT
(In reply to mitz from comment #1)
> This doesn’t look to be related to Gigacage. __cpu_indicator_init supports
> __builtin_cpu_supports() and comes from the compiler (see e.g.
> <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/
> cpu_model.c>). It may have ended up in JavaScriptCore because something used
> the builtin or the compiler thought it was needed. It doesn’t seem to be the
> case in TOT (although TOT has a static initializer for
> JSC::LLInt::Data::s_exceptionInstructions AFAICT).

Nice catch.
Comment 3 Yusuke Suzuki 2018-03-18 22:10:29 PDT
Created attachment 336032 [details]
Patch
Comment 4 mitz 2018-03-18 22:10:52 PDT
(In reply to Yusuke Suzuki from comment #2)
> (In reply to mitz from comment #1)
> > This doesn’t look to be related to Gigacage. __cpu_indicator_init supports
> > __builtin_cpu_supports() and comes from the compiler (see e.g.
> > <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/
> > cpu_model.c>). It may have ended up in JavaScriptCore because something used
> > the builtin or the compiler thought it was needed. It doesn’t seem to be the
> > case in TOT (although TOT has a static initializer for
> > JSC::LLInt::Data::s_exceptionInstructions AFAICT).
> 
> Nice catch.

Actually, in production builds I still see the __cpu_indicator_init, so it would be nice to understand where it’s coming from (I don’t see any code actually using __builtin_cpu_supports()) and whether it can be eliminated.
Comment 5 Yusuke Suzuki 2018-03-18 22:14:37 PDT
(In reply to mitz from comment #4)
> (In reply to Yusuke Suzuki from comment #2)
> > (In reply to mitz from comment #1)
> > > This doesn’t look to be related to Gigacage. __cpu_indicator_init supports
> > > __builtin_cpu_supports() and comes from the compiler (see e.g.
> > > <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/
> > > cpu_model.c>). It may have ended up in JavaScriptCore because something used
> > > the builtin or the compiler thought it was needed. It doesn’t seem to be the
> > > case in TOT (although TOT has a static initializer for
> > > JSC::LLInt::Data::s_exceptionInstructions AFAICT).
> > 
> > Nice catch.
> 
> Actually, in production builds I still see the __cpu_indicator_init, so it
> would be nice to understand where it’s coming from (I don’t see any code
> actually using __builtin_cpu_supports()) and whether it can be eliminated.

Oh, it still exists. OK, I've opened the bug for that.
https://bugs.webkit.org/show_bug.cgi?id=183736
Comment 6 Yusuke Suzuki 2018-03-18 22:17:08 PDT
(In reply to mitz from comment #4)
> Actually, in production builds I still see the __cpu_indicator_init, so it
> would be nice to understand where it’s coming from (I don’t see any code
> actually using __builtin_cpu_supports()) and whether it can be eliminated.

__builtin_popcount? I remember that popcount requires relatively newer CPUs.
Comment 7 Yusuke Suzuki 2018-03-19 22:08:38 PDT
Comment on attachment 336032 [details]
Patch

Thanks!
Comment 8 WebKit Commit Bot 2018-03-19 22:34:52 PDT
Comment on attachment 336032 [details]
Patch

Clearing flags on attachment: 336032

Committed r229740: <https://trac.webkit.org/changeset/229740>
Comment 9 WebKit Commit Bot 2018-03-19 22:34:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-03-19 22:35:17 PDT
<rdar://problem/38651518>