1459624 - error: duplicate symbol: nsXPTCStubBase::

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Ritter [:tjr]]

Besides Bug 1459314, one of the other errors I'm getting is

> 0:08.57 /builds/worker/my-workspace/build/src/clang/bin/ld.lld: error: duplicate symbol: nsXPTCStubBase::Stub57()
> 0:08.57 >>> defined in ../../xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.o
> 0:08.57 >>> defined in ../../xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.o
> 0:08.57 /builds/worker/my-workspace/build/src/clang/bin/ld.lld: error: duplicate symbol: nsXPTCStubBase::Stub162()
> 0:08.57 >>> defined in ../../xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.o
> 0:08.57 >>> defined in ../../xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.o

I think this is because we need to whitelist nsXPTCStubBase, as done in Bug 1303644.  

AFAICT, clang doesn't support this custom 'vtablelink' whitelist option, but it does have support for other whitelist options: https://clang.llvm.org/docs/SanitizerSpecialCaseList.html so I'm going to see what can be done with either existing options, or adding a new option.

Comment 1

[Tom Ritter [:tjr]]

After implementing the whitelist option (correctly; maybe?) I still see the errors. It does seem like the library has duplicate symbols:

> xpcom/reflect/xptcall/xptcall.o
>                  U _ZN14nsXPTCStubBase6Stub57Ev
>                  U _ZN14nsXPTCStubBase6Stub57Ev
> xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.o
> ---------------- T _ZN14nsXPTCStubBase6Stub57Ev
> ---------------- T _ZN14nsXPTCStubBase6Stub57Ev

Comment 2

[Tom Ritter [:tjr]]

> /builds/worker/my-workspace/build/src/clang/bin/clang++ -B /builds/worker/my-workspace/build/src/gcc/bin -std=gnu++14 -o xptcstubs_x86_64_linux.o -c -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/dist/stl_wrappers -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/dist/system_wrappers -include /builds/worker/my-workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/my-workspace/build/src/xpcom/reflect/xptcall/md/unix -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/xpcom/reflect/xptcall/md/unix -I/builds/worker/my-workspace/build/src/xpcom/reflect/xptcall -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/dist/include -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/dist/include/nspr -I/builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/my-workspace/build/src/obj-x86_64-pc-linux-gnu/mozilla-config.h -Qunused-arguments -fuse-ld=lld -flto=thin -fsanitize=cfi-icall -fsanitize-blacklist=/builds/worker/my-workspace/build/src/new-cfi-whitelist.txt -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -U_FORTIFY_SOURCE -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fuse-ld=lld -flto=thin -fsanitize=cfi-icall -fsanitize-blacklist=/builds/worker/my-workspace/build/src/new-cfi-whitelist.txt -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -U_FORTIFY_SOURCE -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -O1 -fno-omit-frame-pointer -MD -MP -MF .deps/xptcstubs_x86_64_linux.o.pp /builds/worker/my-workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp


produces 

> llvm-nm xptcstubs_x86_64_linux.o | grep 58
> ---------------- t .Ltmp158
> ---------------- t .Ltmp158
> ---------------- t .Ltmp58
> ---------------- t .Ltmp58
> ---------------- T _ZN14nsXPTCStubBase6Stub58Ev
> ---------------- T _ZN14nsXPTCStubBase6Stub58Ev
> ---------------- T _ZN14nsXPTCStubBase7Stub158Ev
> ---------------- T _ZN14nsXPTCStubBase7Stub158Ev


Whereas the corresponding non-cfi command produces

> ---------------- t .Ltmp158
> ---------------- t .Ltmp58
> ---------------- T _ZN14nsXPTCStubBase6Stub58Ev
> ---------------- T _ZN14nsXPTCStubBase7Stub158Ev

Compiling with -E yields (in both cases):

> class nsIXPTCStubBase : public nsISupports
> {
> public:
> ...
> virtual nsresult Stub57() = 0;
> ...
> };
> 
> class nsXPTCStubBase final : public nsIXPTCStubBase
> {
> public:
> ...
> virtual nsresult Stub57() override;
> ...
> };

Comment 3

[Tom Ritter [:tjr]]

I mixed up (57 and 58) in some of the output, but same pattern applies to both.

Comment 4

[Tom Ritter [:tjr]]

After a lot of minimization; I determined that Stub_Entry produced one entry (https://searchfox.org/mozilla-central/source/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp#130) and is required to be present to compile

And Sentinal Entry https://searchfox.org/mozilla-central/source/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp#208 produces one entry and is not required to be present to compile.

Comment 5

[Tom Ritter [:tjr]]

Okay, here is a minimized reproducer.

>/builds/worker/my-workspace/build/src/clang/bin/clang++ -std=gnu++14 -c -o xptcstubs_x86_64_linux.o -fuse-ld=lld -flto=thin -fsanitize=cfi-icall xptcstubs_x86_64_linux.i && llvm-nm xptcstubs_x86_64_linux.o | grep Stub20 && rm xptcstubs_x86_64_linux.o
>clang-6.0: warning: treating 'cpp-output' input as 'c++-cpp-output' when in C++ mode, this behavior is deprecated [-Wdeprecated]
>clang-6.0: warning: argument unused during compilation: '-fuse-ld=lld' [-Wunused-command-line-argument]
>---------------- T _ZN14nsXPTCStubBase6Stub20Ev
>---------------- T _ZN14nsXPTCStubBase6Stub20Ev

I'm going to have to dig into this to understand what we're doing here, and why it works with LTO but not CFI...

>extern int printf (__const char *__restrict __format, ...);
>
>class nsIXPTCStubBase
>{
>public:
>virtual int Stub20() = 0;
>virtual int Sentinel0() = 0;
>};
>class nsXPTCStubBase final : public nsIXPTCStubBase
>{
>public:
>virtual int Stub20() override;
>virtual int Sentinel0() override;
>};
>
>
>asm(".section \".text\"\n\t" \
>    ".align 2\n\t" \
>        ".globl _ZN14nsXPTCStubBase6Stub20Ev\n\t"       \
>    ".hidden  _ZN14nsXPTCStubBase6Stub20Ev\n\t" \
>    ".type  _ZN14nsXPTCStubBase6Stub20Ev,@function\n" \
>    "_ZN14nsXPTCStubBase6Stub20Ev:\n\t" \
>
>    "movl $20, %eax\n\t" \
>    "jmp  SharedStub\n\t" \
>
>    ".size  _ZN14nsXPTCStubBase6Stub20Ev,.-_ZN14nsXPTCStubBase6Stub20Ev\n\t" \
>    );
>
>int nsXPTCStubBase::Sentinel0() { printf("hi"); return 0; }

Comment 6

[Tom Ritter [:tjr]]

Okay, here is a minimized reproducer.

> /builds/worker/my-workspace/build/src/clang/bin/clang++ -std=gnu++14 -c -o xptcstubs_x86_64_linux.o -fuse-ld=lld -flto=thin -fsanitize=cfi-icall xptcstubs_x86_64_linux.i && llvm-nm xptcstubs_x86_64_linux.o | grep Stub20 && rm xptcstubs_x86_64_linux.o
> clang-6.0: warning: treating 'cpp-output' input as 'c++-cpp-output' when in C++ mode, this behavior is deprecated [-Wdeprecated]
> clang-6.0: warning: argument unused during compilation: '-fuse-ld=lld' [-Wunused-command-line-argument]
> ---------------- T _ZN14nsXPTCStubBase6Stub20Ev
> ---------------- T _ZN14nsXPTCStubBase6Stub20Ev

I'm going to have to dig into this to understand what we're doing here, and why it works with LTO but not CFI...

> extern int printf (__const char *__restrict __format, ...);
> 
> class nsIXPTCStubBase
> {
> public:
> virtual int Stub20() = 0;
> virtual int Sentinel0() = 0;
> };
> class nsXPTCStubBase final : public nsIXPTCStubBase
> {
> public:
> virtual int Stub20() override;
> virtual int Sentinel0() override;
> };
> 
> 
> asm(".section \".text\"\n\t" \
>     ".align 2\n\t" \
>         ".globl _ZN14nsXPTCStubBase6Stub20Ev\n\t"       \
>     ".hidden  _ZN14nsXPTCStubBase6Stub20Ev\n\t" \
>     ".type  _ZN14nsXPTCStubBase6Stub20Ev,@function\n" \
>     "_ZN14nsXPTCStubBase6Stub20Ev:\n\t" \
> 
>     "movl $20, %eax\n\t" \
>     "jmp  SharedStub\n\t" \
> 
>     ".size  _ZN14nsXPTCStubBase6Stub20Ev,.-_ZN14nsXPTCStubBase6Stub20Ev\n\t" \
>     );
> 
> int nsXPTCStubBase::Sentinel0() { printf("hi"); return 0; }

Comment 7

[Tom Ritter [:tjr]]

So clang fixed whatever this bug was at some point. As of revision 332262 it works.