apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.44k stars 3.52k forks source link

[C++] Gandiva exposes LLVM symbols #23536

Open asfimport opened 4 years ago

asfimport commented 4 years ago

Despite the fact the cmake seems to indicate the opposite, I see that gandiva on macOS is compiled with LLVM symbols exported. E.g.:


nm ../sw/osx_x86-64/arrow/latest/lib/libgandiva.dylib | grep _LLVMEnablePrettyStackTrace
000000000120b5a0 T _LLVMEnablePrettyStackTrace

is that expected?

Reporter: Giulio Eulisse

Note: This issue was originally created as ARROW-7241. Please see the migration documentation for further details.

asfimport commented 4 years ago

Wes McKinney / @wesm: It is not our intent to expose any symbols from statically-linked dependencies

asfimport commented 4 years ago

Giulio Eulisse: Update on this.

On linux it seems to be sufficient to build LLVM with:


-DCMAKE_CXX_FLAGS=-fvisibility=hidden -DCLANG_TOOL_LIBCLANG_BUILD=OFF

on macOS this is not the case. So far we managed to get the wanted result by hacking:


set_target_properties(${LIB_NAME}_shared PROPERTIES LINK_FLAGS -Wl,-unexported_symbol,_LLVMEnablePrettyStackTrace)

into cmake_modules/BuildUtils.cmake. This is of course not an acceptable solution because it hides only the symbol we check for, not all of them. Any better ideas?

asfimport commented 4 years ago

Giulio Eulisse: In the end we settled to:


cat >no-llvm-symbols.txt << EOF
_LLVM*
__ZN4llvm*
__ZNK4llvm*
EOF
cmake ... -DCMAKE_SHARED_LINKER_FLAGS="-Wl,-unexported_symbols_list,$PWD/no-llvm-symbols.txt"

which hides everything from LLVM which does not come from Gandiva itself. In our limited tests this works fine. Is this of common interest? How do we proceed from here?

asfimport commented 4 years ago

Antoine Pitrou / @pitrou: [~ktf] Sorry  for the delay. Yes, this would be of interest. I've tried this patch: https://gist.github.com/pitrou/0e88d3d829175a6179b70671ed49a865

But unfortunately it crashes at the end of one of the Gandiva tests (in the llvm::LLVMContext destructor). Do you want to take it up and investigate?

asfimport commented 4 years ago

Giulio Eulisse: Sorry, by now we have fixed the dependency which was more under our control, so this is not particularly needed anymore on our side and I've other priorities.