Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide private symbols in shared library #12944

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Hide private symbols in shared library #12944

wants to merge 1 commit into from

Conversation

rhubner
Copy link
Contributor

@rhubner rhubner commented Aug 19, 2024

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

Work only in Release mode.

Fix #12929

@rhubner
Hide private symbols in shared library
8c9409e 
Add new build flag HIDE_PRIVATE_SYMBOLS to allow compile
shared library without exported private symbols.

Work only in Release mode.
@rhubner rhubner mentioned this pull request Aug 20, 2024
Open
@ottok
Copy link
Contributor

ottok commented Aug 22, 2024

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
Copy link
Contributor

ottok commented Aug 27, 2024

More info on how Debian scans the ABI:

@rhubner
Copy link
Contributor Author

rhubner commented Aug 27, 2024

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

@ottok
Copy link
Contributor

ottok commented Aug 29, 2024

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..?

@rhubner
Copy link
Contributor Author

rhubner commented Aug 29, 2024

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

@ottok
Copy link
Contributor

ottok commented Aug 30, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RocksDB library leaks private symbols
3 participants
@rhubner @ottok @facebook-github-bot