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.

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

(Unintentional) dependency in C++ static constructor order; causes SIGFPE when source files are reordered #1730

Closed
blue42u opened this issue Apr 21, 2024 · 1 comment · Fixed by #1731

Comments

@blue42u
Copy link
Contributor

blue42u commented Apr 21, 2024

Intention
I was reading through the build scripts for Dyninst 13.0.0 (porting to make a Meson wrap), and ran across a case in common/CMakeLists.txt where a source file is listed in the _private_headers array. This seemed like a typo, so I tried moving it to the _sources array:

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 669659e44..ff49d1cb4 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -77,7 +77,6 @@ set(_private_headers
     src/parseauxv.h
     src/pathName.h
     src/pool_allocators.h
-    src/registers/MachRegister.C
     src/sha1.h
     src/singleton_object_pool.h
     src/stats.h
@@ -111,7 +110,8 @@ set(_sources
     src/debug_common.C
     src/VariableLocation.C
     src/Buffer.C
-    src/MachSyscall.C)
+    src/MachSyscall.C
+    src/registers/MachRegister.C)
 
 if(DYNINST_OS_UNIX)
   list(APPEND _sources src/addrtranslate-sysv.C src/symbolDemangleWithCache.C

Describe the bug
Despite the seemingly innocuous nature of the above patch, the resulting libraries crash when used.

To Reproduce
After applying the above patch, build Dyninst and attempt to load libcommon.so. The result is an FPE (divide by zero):

LD_PRELOAD=./build/common/libcommon.so nm
[1]    185631 floating point exception (core dumped)  LD_PRELOAD=./common/libcommon.so nmgdb ...
...snip...
Program terminated with signal SIGFPE, Arithmetic exception.
#0  0x00007f51a507cc67 in std::__detail::_Mod_range_hashing::operator() (__den=0, __num=0, this=<optimized out>) at /usr/include/c++/13/bits/hashtable_policy.h:528
528	    { return __num % __den; }
(gdb) bt
#0  0x00007f51a507cc67 in std::__detail::_Mod_range_hashing::operator() (__den=0, __num=0, this=<optimized out>) at /usr/include/c++/13/bits/hashtable_policy.h:528
#1  std::__detail::_Hash_code_base<int, std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Select1st, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, false>::_M_bucket_index (__bkt_count=0, __c=0, 
    this=0x7f51a50f6aa0 <(anonymous namespace)::names>) at /usr/include/c++/13/bits/hashtable_policy.h:1332
#2  std::_Hashtable<int, std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_bucket_index (
    __c=0, this=0x7f51a50f6aa0 <(anonymous namespace)::names>) at /usr/include/c++/13/bits/hashtable.h:797
#3  std::_Hashtable<int, std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_emplace<int&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7f51a50f6aa0 <(anonymous namespace)::names>)
    at /usr/include/c++/13/bits/hashtable.h:2098
#4  std::_Hashtable<int, std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::emplace<int&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7f51a50f6aa0 <(anonymous namespace)::names>)
    at /usr/include/c++/13/bits/hashtable.h:961
#5  std::unordered_map<int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::emplace<int&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x7f51a50f6aa0 <(anonymous namespace)::names>) at /usr/include/c++/13/bits/unordered_map.h:396
#6  Dyninst::MachRegister::MachRegister (this=0x7f51a50f311c <Dyninst::InvalidReg>, r=0, n=...)
    at /home/deepthought/Projects/dyninst/common/src/registers/MachRegister.C:27
#7  0x00007f51a4ffd9c9 in __static_initialization_and_destruction_0 () at /home/deepthought/Projects/dyninst/common/h/registers/abstract_regs.h:55
#8  0x00007f51a50fde3e in ?? () from /lib64/ld-linux-x86-64.so.2
#9  0x00007f51a50fdf24 in ?? () from /lib64/ld-linux-x86-64.so.2
#10 0x00007f51a5113500 in ?? () from /lib64/ld-linux-x86-64.so.2

Expected behavior
No crash should happen.

System (please complete the following information):

  • Linux x86_64
  • Confirmed present in 6ef6687
  • Glibc 2.37
@hainest
Copy link
Contributor

hainest commented Apr 21, 2024

ran across a case in common/CMakeLists.txt where a source file is listed in the _private_headers array

I sometimes impress myself with the stupid mistakes I make.

After applying the above patch, build Dyninst and attempt to load libcommon.so. The result is an FPE (divide by zero):

Oh, SIOF, you haunt my dreams.

For additional fun, there is an extra copy of MachRegister.h in the _public_headers list. I really made a mess here.


@kupsch I fixed this by moving the definitions of names and all_regs into dyn_regs.C so that they are in the same TU as the definitions of the registers and guaranteed to be initialized in order. It's a little ugly because it would make them global symbols in libcommon, but we can leave them unexported to keep them from escaping further. Thoughts?

hainest added a commit that referenced this issue Apr 21, 2024
This fixes the static initialization ordering issue reported in
#1730.
hainest added a commit that referenced this issue Apr 22, 2024
This fixes the static initialization ordering issue reported in #1730.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants