rocksdb icon indicating copy to clipboard operation
rocksdb copied to clipboard
verified publisher icon facebook

Hide private symbols in shared library

Open rhubner opened this issue 1 year ago • 9 comments

Add new build flag HIDE_PRIVATE_SYMBOLS to allow compile shared library without exported private symbols.

Work only in Release mode.

Fix #12929

rhubner avatar Aug 19 '24 16:08 rhubner

I tested this in https://salsa.debian.org/debian/rocksdb/-/merge_requests/3 and it didn't work. I wonder did I enable the flag and 'Release mode' correctly?

...
+ _ZZNSt19_Sp_make_shared_tag5_S_tiEvE5__tag@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb12FileMetaDataESaIS1_EE17_M_realloc_appendIJRKS1_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb12FileMetaDataESaIS1_EE17_M_realloc_appendIJRKS1_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb13LevelMetaDataESaIS1_EE17_M_realloc_appendIJRiRmS_INS0_15SstFileMetaDataESaIS7_EEEEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb13LevelMetaDataESaIS1_EE17_M_realloc_appendIJRiRmS_INS0_15SstFileMetaDataESaIS7_EEEEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb16IngestedFileInfoESaIS1_EE17_M_realloc_appendIJRKS1_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb16IngestedFileInfoESaIS1_EE17_M_realloc_appendIJRKS1_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb16IngestedFileInfoESaIS1_EE17_M_realloc_appendIJS1_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb16IngestedFileInfoESaIS1_EE17_M_realloc_appendIJS1_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb17CompactionOutputs6OutputESaIS2_EE17_M_realloc_appendIJNS0_12FileMetaDataERKNS0_21InternalKeyComparatorERbSA_RmEEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorIN7rocksdb17CompactionOutputs6OutputESaIS2_EE17_M_realloc_appendIJNS0_12FileMetaDataERKNS0_21InternalKeyComparatorERbSA_RmEEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJRiRKS2_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJRiRKS2_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJRiS2_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJRiS2_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJS3_EEEvDpOT_EN11_Guard_eltsD1Ev@Base 9.3.1-1
+ _ZZNSt6vectorISt4pairIiN7rocksdb12FileMetaDataEESaIS3_EE17_M_realloc_appendIJS3_EEEvDpOT_EN11_Guard_eltsD2Ev@Base 9.3.1-1
+ _ZZNSt9once_flag18_Prepare_executionC4IZSt9call_onceIMNSt13__future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS3_12_Result_baseENS7_8_DeleterEEvEEPbEJPS4_SC_SD_EEvRS_OT_DpOT0_EUlvE_EERSI_ENUlvE_4_FUNEv@Base 9.3.1-1
...

ottok avatar Aug 22 '24 04:08 ottok

More info on how Debian scans the ABI:

  • https://manpages.debian.org/unstable/dpkg-dev/dpkg-gensymbols.1.en.html
  • https://manpages.debian.org/unstable/dpkg-dev/deb-symbols.5.en.html

ottok avatar Aug 27 '24 05:08 ottok

Hello @ottok,

thanks for quick reply. I think we need to update CMake parameters. This is what I found in your build system :

cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=None -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_INSTALL_LOCALSTATEDIR=/var -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON -DFETCHCONTENT_FULLY_DISCONNECTED=ON -DCMAKE_INSTALL_RUNSTATEDIR=/run -DCMAKE_SKIP_INSTALL_ALL_DEPENDENCY=ON "-GUnix Makefiles" -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_LIBDIR=lib/x86_64-linux-gnu -DWITH_BZ2=ON -DWITH_LZ4=ON -DWITH_ZLIB=ON -DWITH_ZSTD=ON -DWITH_SNAPPY=ON -DROCKSDB_BUILD_SHARED=ON -DWITH_BENCHMARK_TOOLS=ON -DWITH_CORE_TOOLS=ON -DWITH_TOOLS=ON -DWITH_TESTS=ON -DPORTABLE=1 -DHIDE_PRIVATE_SYMBOLS=1 -DFAIL_ON_WARNINGS=OFF ..

I think the problem is -DCMAKE_BUILD_TYPE=None. Try to change to -DCMAKE_BUILD_TYPE=Release.

Radek

rhubner avatar Aug 27 '24 09:08 rhubner

I applied -DCMAKE_BUILD_TYPE=Release in https://salsa.debian.org/debian/rocksdb/-/merge_requests/3 and now build fails with:

/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x28): undefined reference to `rocksdb::Customizable::AreEquivalent(rocksdb::ConfigOptions const&, rocksdb::Configurable const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) const'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x38): undefined reference to `rocksdb::Configurable::PrepareOptions(rocksdb::ConfigOptions const&)'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x40): undefined reference to `rocksdb::Configurable::ValidateOptions(rocksdb::DBOptions const&, rocksdb::ColumnFamilyOptions const&) const'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x50): undefined reference to `rocksdb::Configurable::ParseStringOptions(rocksdb::ConfigOptions const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x58): undefined reference to `rocksdb::Configurable::ConfigureOptions(rocksdb::ConfigOptions const&, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >*)'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x60): undefined reference to `rocksdb::Configurable::ParseOption(rocksdb::ConfigOptions const&, rocksdb::OptionTypeInfo const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void*)'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x68): undefined reference to `rocksdb::Configurable::OptionsAreEqual(rocksdb::ConfigOptions const&, rocksdb::OptionTypeInfo const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void const*, void const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) const'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x70): undefined reference to `rocksdb::Customizable::SerializeOptions(rocksdb::ConfigOptions const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro._ZTVN7rocksdb9Benchmark10KeepFilterE[_ZTVN7rocksdb9Benchmark10KeepFilterE]+0x78): undefined reference to `rocksdb::Customizable::GetOptionName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const'
/usr/bin/ld: CMakeFiles/db_bench.dir/tools/db_bench_tool.cc.o:(.data.rel.ro+0x0): undefined reference to `vtable for rocksdb::BlockCacheTierMetadata'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
collect2: error: ld returned 1 exit status
make[3]: *** [CMakeFiles/db_bench.dir/build.make:139: db_bench] Error 1
make[3]: Leaving directory '/tmp/build/source/obj-x86_64-linux-gnu'
make[2]: *** [CMakeFiles/Makefile2:254: CMakeFiles/db_bench.dir/all] Error 2
make[2]: Leaving directory '/tmp/build/source/obj-x86_64-linux-gnu'
make[1]: *** [Makefile:139: all] Error 2
make[1]: Leaving directory '/tmp/build/source/obj-x86_64-linux-gnu'
dh_auto_build: error: cd obj-x86_64-linux-gnu && make -j4 "INSTALL=install --strip-program=true" VERBOSE=1 returned exit code 2
make: *** [debian/rules:60: binary] Error 2

Seems now that the linker still sees the private symbols and fails to compute as they have become hidden..?

ottok avatar Aug 29 '24 05:08 ottok

Hello @ottok,

thanks for quick reply. It looks like this will be bigger problem. As you suggested I hide all symbols except C API. But as you can see, other program are using also C++ API. I don't know how to efficiently export only the public symbols and hide others.

I'm able to compile librocksdb.so with thees flags : -DWITH_TESTS=OFF -DWITH_BENCHMARK_TOOLS=OFF -DWITH_TOOLS=OFF and then make rocksdb. But then it may not have some C++ API. 🤔

I think this will need more refactoring across RocksDB codebase to define which classes/structs/methods are public API and what is private.

cc: @adamretter: What are your thoughts?

Radek

rhubner avatar Aug 29 '24 12:08 rhubner

Thanks for looking into this and considering refactoring!

RocksDB does not strictly require this, but it would be good engineering practice to have clear separation of private and public symbols as it decreases the chance of bugs and makes long-term maintenance and upgrades of the library easier and less likely to have unexpected breakage with consuming software. Both Debian and Fedora care about ABI symbols and tracking them.

ottok avatar Aug 30 '24 22:08 ottok

@rhubner Any chance you might pick this up again some day? Having a clean ABI would be nice for stability and maintainability.

ottok avatar Nov 24 '24 04:11 ottok

I rebased the patches in https://salsa.debian.org/debian/rocksdb/-/merge_requests/3 on latest 9.8.4 just to check if what happens, and I learned at the situation is still the same (same test build outcome, and adjacent code upstream is unchanged).

ottok avatar Dec 21 '24 22:12 ottok

Thanks @rhubner for looking into this. I applied the latest version of this on top of latest RocksDB version in Debian and build is failing due to undefined references. Build log at https://salsa.debian.org/otto/rocksdb/-/jobs/7087000. Links in the right sidebar will show you the exact code that went into the build.

...
[ 93%] Linking CXX executable cache_bench
/usr/bin/cmake -E cmake_link_script CMakeFiles/cache_bench.dir/link.txt --verbose=1
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: warning: relocation against `_ZTVN7rocksdb13HistogramImplE' in read-only section `.text'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::CacheBench::Run()':
./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:479:(.text+0xb15): undefined reference to `rocksdb::SystemClock::Default()'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::CacheBench::Run() [clone .isra.0]':
./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:195:(.text+0xb35): undefined reference to `rocksdb::kDefaultToAdaptiveMutex'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `SharedState':
./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:195:(.text+0xb47): undefined reference to `rocksdb::port::Mutex::Mutex(bool)'
/usr/bin/ld: ./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:194:(.text+0xb52): undefined reference to `rocksdb::port::CondVar::CondVar(rocksdb::port::Mutex*)'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::CacheBench::Run()':
./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:485:(.text+0xbec): undefined reference to `vtable for rocksdb::HistogramImpl'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::HistogramImpl::HistogramImpl()':
./obj-x86_64-linux-gnu/./monitoring/histogram.h:112:(.text+0xc8c): undefined reference to `rocksdb::HistogramStat::HistogramStat()'
/usr/bin/ld: ./obj-x86_64-linux-gnu/./monitoring/histogram.h:112:(.text+0xcb5): undefined reference to `rocksdb::HistogramImpl::Clear()'
/usr/bin/ld: ./obj-x86_64-linux-gnu/./monitoring/histogram.h:112:(.text+0xd83): undefined reference to `rocksdb::HistogramStat::HistogramStat()'
/usr/bin/ld: ./obj-x86_64-linux-gnu/./monitoring/histogram.h:112:(.text+0xdb6): undefined reference to `rocksdb::HistogramImpl::Clear()'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::MutexLock::MutexLock(rocksdb::port::Mutex*)':
./obj-x86_64-linux-gnu/./util/mutexlock.h:37:(.text+0xe7d): undefined reference to `rocksdb::port::Mutex::Lock()'
/usr/bin/ld: CMakeFiles/cache_bench.dir/cache/cache_bench_tool.cc.o: in function `rocksdb::CacheBench::Run()':
./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:497:(.text+0xe87): undefined reference to `rocksdb::port::CondVar::Wait()'
/usr/bin/ld: ./obj-x86_64-linux-gnu/./cache/cache_bench_tool.cc:504:(.text+0xf7d): undefined reference to `rocksdb::port::CondVar::SignalAll()'
...

ottok avatar Feb 14 '25 04:02 ottok