search

efficient / libcuckoo

A high-performance, concurrent hash table
Other
1.62k stars 275 forks source link

Fix UBSAN warning about unaligned access #140

Closed milianw closed 3 years ago

milianw commented 3 years ago

Do not create spinlock() default arguments when initializing locks_t. Instead, use the two-arg std::vector constructor which will do the same in-place. This work-arounds the UBSAN alignment warnings such as:

libcuckoo/libcuckoo/cuckoohash_map.hh:805:18: runtime error: member access within misaligned address 0x7f5343f9a6a0 for type 'struct spinlock', which requires 64 byte alignment
0x7f5343f9a6a0: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^
    #0 0x7f5373045890 in libcuckoo::cuckoohash_map<...>::spinlock::spinlock() .../libcuckoo/libcuckoo/cuckoohash_map.hh:805
    #1 0x7f5373043ef2 in libcuckoo::cuckoohash_map<...>::cuckoohash_map(unsigned long, std::hash<QString> const&, std::equal_to<QString> const&, boost::alignment::aligned_allocator<std::pair<QString const, ImageIO::OnDiskStringCache::InternedString>, 64ul> const&) /home/milian/projects/kdab/qitissue/KDAB/io/../../3rdParty/libcuckoo/libcuckoo/cuckoohash_map.hh:116

Fixes: https://github.com/efficient/libcuckoo/issues/53

milianw commented 3 years ago

FTR, the bug seems to have suddenly vanished on my system, see also the upstream report over at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102984

I still think that this patch is fine though, as it simplifies the code to achieve the same

manugoyal commented 3 years ago

Thanks for the fix! Agree the code looks cleaner regardless of whether or not the UBSAN issue occurs. I would have expected the stack-allocated spinlock that's passed into the constructor to be properly aligned, but maybe not?