[PATCH] D53782: [llvm-objcopy] Don't apply --localize flags to common symbols

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

In https://reviews.llvm.org/D53782#1278636, @jhenderson wrote:

> I can certainly understand your reasoning, but this could also adversely impact a desired use-case of hiding a symbol in an object or library from another object, which is presumably the purpose of --localize-hidden, and is what is discussed in https://chromium.googlesource.com/external/dynamorio/+/master/core/CMakeLists.txt#541:
>
> > We need to do extra work to hide symbols in the static library build.
> >  First we do a partial link with ld -r, which makes a single libdynamorio.o
> >  object file.  Then we use objcopy --localize-hidden to hide all
> >  non-exported symbols.
>
> Let's say that there's a common symbol `foo` which is defined in test.o, and is added to a static library to be shipped by a vendor. They might want this symbol to not appear in their interface. Using --localize-hidden would be one way of doing this, but only if the existing llvm-objcopy behaviour is maintained.
>
> Basically, I'm on the fence on this, but I wanted to clarify the argument against it from my point of view.


I forgot to mention in the patch description (but it is in https://bugs.llvm.org/show_bug.cgi?id=39461): dynamorio is the thing that breaks when switching to llvm-objcopy, so getting it working has been my test case :)

Here's verification w/ a fresh pull of upstream dynamorio:

  $ cmake -DBUILD_TESTS=ON -DCMAKE_OBJCOPY=/path/to/llvm-objcopy ../dynamorio
  $ make -j && ctest -R 'code_api\|api.static_' -E FLAKY

GNU objcopy (just omit -DCMAKE_OBJCOPY) & llvm-objcopy w/ this patch passes. llvm-objcopy from trunk crashes:

  (gdb) r
  Starting program: $HOME/src/dynamorio-build/suite/tests/bin/api.static_noinit
  pre-DR init
  
  Program received signal SIGSEGV, Segmentation fault.
  0x00005555555e2b92 in modules_init () at $HOME/src/dynamorio/core/module_list.c:145
  145         VMVECTOR_ALLOC_VECTOR(loaded_module_areas, GLOBAL_DCONTEXT,
  (gdb) bt
  #0  0x00005555555e2b92 in modules_init () at $HOME/src/dynamorio/core/module_list.c:145
  #1  0x000055555558b5b2 in dynamorio_app_init () at $HOME/src/dynamorio/core/dynamo.c:523
  #2  0x000055555558c0ad in dr_app_setup () at $HOME/src/dynamorio/core/dynamo.c:2693
  #3  0x0000555555584a5f in main (argc=1, argv=0x7fffffffda08) at $HOME/src/dynamorio/suite/tests/api/static_noinit.c:73

I'm not exactly sure where the SIGSEGV is coming from, but `loaded_module_areas` is a common symbol, and diffing the objects w/ llvm-readobj -symbols shows that common symbols are where this differs compared to when building w/ objcopy.


Repository:
  rL LLVM

https://reviews.llvm.org/D53782