search

boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
199 stars 161 forks source link

UBSan false positive in thread_specific_ptr #236

Closed Kojoley closed 5 years ago

Kojoley commented 5 years ago

./b2 -j2 toolset=clang-7 cxxflags="-fsanitize=undefined -fno-sanitize-recover=undefined" linkflags="-fsanitize=undefined" libs/thread/test//ex_tss -a

testing.capture-output bin.v2/libs/thread/test/ex_tss.test/clang-linux-7/debug/threadapi-pthread/threading-multi/visibility-hidden/ex_tss.run
====== BEGIN OUTPUT ======
libs/thread/src/pthread/thread.cpp:114:37: runtime error: member call on address 0x000001938d10 which does not point to an object of type 'boost::detail::tss_cleanup_function'
0x000001938d10: note: object is of type 'boost::thread_specific_ptr<int>::delete_data'
 00 00 00 00  90 f8 67 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  31 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'boost::thread_specific_ptr<int>::delete_data'

EXIT STATUS: 1
====== END OUTPUT ======

Update:

viboes commented 5 years ago

Hi, thanks for the report.

We know that thread_specific_ptr is broken since a long time, but we don't know how to fix it.

Any help is welcome, a PR, any aditional analysis, ...

Kojoley commented 5 years ago

Sigh, I thought it is something new because previously I did not see the problem (I found it via Spirit), it should be a new feature in UBSAN or the problem did show up without adding a -fno-sanitize-recover=undefined flag.

Kojoley commented 5 years ago

I feel like it is a bug in UBSAN, but I may be wrong because I did not succeeded in making a MWE. With #237 UBSAN is happy, but I am not sure why it works.

pdimov commented 5 years ago

This is likely a false positive caused by hidden visibility. Use visibility=global on UBsan runs.

Kojoley commented 5 years ago

@pdimov You are right, the problem is gone if I add visibility=global. Is it a known limitation or it is needed to be reported?

pdimov commented 5 years ago

According to

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80963

it's not clear if UBsan is correct here or not; in principle, with hidden visibility, two base classes of the same name exist in the program, but in C++ this can't be true due to ODR.

pdimov commented 5 years ago

In either case, this is a known issue - here's @mclow encountering it in 2013: https://cplusplusmusings.wordpress.com/2013/03/26/testing-libc-with-fsanitizeundefined/

pdimov commented 5 years ago

I just add visibility=global to the UBsan configurations. :-)

Kojoley commented 5 years ago

In either case, this is a known issue - here's @mclow encountering it in 2013: cplusplusmusings.wordpress.com/2013/03/26/testing-libc-with-fsanitizeundefined

If https://www.boost.org/development/tests/develop/Marshall-UBSAN.html is his worker, then he himself forgot about it.

Kojoley commented 5 years ago

@pdimov The GCC guys think it is not a false positive, but ODR rules violation https://www.mail-archive.com/gcc-bugs@gcc.gnu.org/msg534862.html

pdimov commented 5 years ago

That's the same bug I linked above. :-)

Kojoley commented 5 years ago

Hah, I am blind :-)

What do you think about placing __attribute__((visibility("default"))) on interface classes?

pdimov commented 5 years ago

I'm not going to do that. visibility=global on UBsan runs is good enough for me. Having two identical definitions doesn't violate ODR, by the definition of ODR.

viboes commented 5 years ago

Do we have some conclusion for this issue?

Kojoley commented 5 years ago

I have opened a ticket on LLVM bugtracker but by this time it has no response https://bugs.llvm.org/show_bug.cgi?id=39191