Bug 251112

Summary: Compiling C++ with asan fails by default because libclang_rt.asan-x86_64.so uses symbol pthread_attr_get_np but doesn't link to libpthread.so
Product: Base System Reporter: Andrew Stitcher <astitcher>
Component: binAssignee: Konstantin Belousov <kib>
Status: New ---    
Severity: Affects Some People CC: dim, emaste, kib
Priority: ---    
Version: 12.2-RELEASE   
Hardware: Any   
OS: Any   

Description Andrew Stitcher 2020-11-13 20:33:19 UTC
Compiler error:
/usr/bin/c++ -g -fno-omit-frame-pointer -shared-libasan -fsanitize=address,undefined -fsanitize-recover=vptr -O2 -g -DNDEBUG -flto=thin c/tests/CMakeFiles/test_main.dir/test_main.cpp.o c/tests/CMakeFiles/c-proactor-test.dir/pn_test.cpp.o c/tests/CMakeFiles/c-proactor-test.dir/pn_test_proactor.cpp.o c/tests/CMakeFiles/c-proactor-test.dir/proactor_test.cpp.o -o c/tests/c-proactor-test  -Wl,-rpath,/home/vagrant/qpid-proton/BLD/c  c/libqpid-proton-proactor.so.1.7.1  c/libqpid-proton-core.so.10.11.0 && :
ld: error: /usr/lib/clang/10.0.1/lib/freebsd/libclang_rt.asan-x86_64.so: undefined reference to pthread_attr_get_np

And indeed the library only links in portable pthreads via libc:
$ ldd /usr/lib/clang/10.0.1/lib/freebsd/libclang_rt.asan-x86_64.so
	libc++.so.1 => /usr/lib/libc++.so.1 (0x800675000)
	libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x800742000)
	libm.so.5 => /lib/libm.so.5 (0x800764000)
	libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x800796000)
	libc.so.7 => /lib/libc.so.7 (0x80024e000)

Whereas the needed symbol is in libpthreads:
$ objdump -T /usr/lib/libpthread.so | grep pthread_attr_get_np
000000000001b960  w   DF .text	0000000000000264  FBSD_1.0    pthread_attr_get_np
000000000001b960  w   DF .text	0000000000000264  FBSDprivate_1.0 _pthread_attr_get_np

You can work around this by manually linking -lpthreads, but our build files which work elsewhere don't do this as it's not necessary elsewhere.
Comment 1 Konstantin Belousov freebsd_committer 2020-11-17 19:05:15 UTC
SO what do you want ? A stub for pthread_attr_get_np() in libc ?  Or should clang
c++ driver changed to link with libpthread when asan is enabled ?
Comment 2 Andrew Stitcher 2020-11-17 20:53:39 UTC
(In reply to Konstantin Belousov from comment #1)

I'd say neither.

To me it looks like a build error in the clang++ asan support shared library: If libclang_rt.asan-x86_64.so uses the symbol pthread_attr_get_np then it should be linked against the shared library that provides that symbol.

But if this is not possible for some reason then changing the driver to link against libpthreads when using the shared library version of asan seems better to me.
Comment 3 Andrew Stitcher 2020-11-17 20:54:44 UTC
Also note that this bug only occurs when using -shared-libasan not when using the static library version of asan.
Comment 4 Konstantin Belousov freebsd_committer 2020-11-18 03:26:30 UTC
(In reply to Andrew Stitcher from comment #3)
It really depends on what this symbol is used for.

Linking asan lib with libpthread would change the mode of operation of all
binaries linked with it.  Presence of libpthread modifies libc, libc++, and
perhaps unlisted number of other non-system libs: they start using proper
pthread locks like mutexes.
Comment 5 Andrew Stitcher 2020-11-18 15:40:06 UTC
(In reply to Konstantin Belousov from comment #4)

I'm not sure I really understand what you mean about linking with libpthread.so changing the behaviour of libc.so. Most of the FreeBSD pthread calls are already in libc (or at least don't need to explicitly link with -lpthread according to the manpage).

So I expected that libc is already multithreaded and linking in pthread is only used for the _np (non portable/posix) pthread calls.

Looking at the clang source code pthread_attr_get_np is used to get the stack parameters (pthread_getattr_np is #defined to pthread_attr_get_np):

  pthread_attr_t attr;
  CHECK_EQ(pthread_getattr_np(pthread_self(), &attr), 0);
  my_pthread_attr_getstack(&attr, &stackaddr, &stacksize);

I'm not sure how relevant this is to what you're interested in, but the symbol is used to call a function.
Comment 6 Konstantin Belousov freebsd_committer 2020-11-19 08:03:29 UTC
(In reply to Andrew Stitcher from comment #5)
Depending on whether libpthread linked into the process or not (as well it
might be not linked but loaded later), libc changes its algorithms.  For
instance, stdio FILEs are locked if libpthread is present, and not locked otherwise.  Similarly, rtld uses real (but custom) rw locks to protect it
internal state if libpthread is there, otherwise it just masks signals using
cheap sigfastblock(2) backdoor.

This is why I asked whether it acceptable to change the C runtime behavior
by sneaky linking with libpthread if ASAN is used.
Comment 7 Andrew Stitcher 2020-11-19 21:05:28 UTC
(In reply to Konstantin Belousov from comment #6)

So how do the pthreads calls that are in libc work? Are they what you call 'stubs'? In which case does the stub load libpthread to do the actual work or what else could happen?

If you were to add a stub to libc for pthread_att_get_np what would it do? the asan runtime requires it to actually return real info about the running thread so it would have to load the actual libpthread in some way if it was a stub.

It does seems to me that if you never spawn any threads then it looks like the asan runtime might never actually ever call pthread_attr_get_np. So maybe it would be good not to change the libc behaviour unless threads were in use.
Comment 8 Konstantin Belousov freebsd_committer 2020-11-20 12:01:55 UTC
(In reply to Andrew Stitcher from comment #7)
Stubs do nothing in different ways.  They never load libpthread.  Mostly they
do nothing and return success, like pthread_mutex_lock() stub.  Some very
specific stubs return failure since callers are prepared for it, like
pthread_once() or pthread_key_create().

Basically, stubs allow to compile a library with knowledge of threads, but only
utilize synchronization and other thread-related services if process actually
loaded libpthread.

If we add pthread_attr_get_np() stub to libc, most likely it would do nothing.
It should fit with you description of asan runtime not calling
pthread_attr_get_np() if no threads were spawned.
Comment 9 Andrew Stitcher 2020-11-24 04:57:31 UTC
(In reply to Konstantin Belousov from comment #8)
I think I understand the libpthread/libc interaction a bit better now: It seems that the only way to get pthread_create() is to actually link libpthread. So any executable that actually has multiple threads will necessarily have libpthread linked in. This is what I was missing before when you were talking about pthread stubs - that is no stub for pthread_create() in libc.

It looks like there are stubs (weak symbols) in libc for the other relevant pthread symbols used in libclang_rt.asan (even the ones that are documented to require linking with -lpthread) so I think it would make sense to have a stub for pthread_attr_get_np() which returns the ESRCH error (this is the documented return if it cannot find the thread id which I think is necessarily the case if there are no threads!). In the case of libclang_rt.asan I think if it did get called somehow - which looks to be impossible - the return value is checked and should abort the run.
Comment 10 Konstantin Belousov freebsd_committer 2020-11-26 20:32:26 UTC
Comment 11 commit-hook freebsd_committer 2020-11-28 12:19:37 UTC
A commit references this bug:

Author: kib
Date: Sat Nov 28 12:19:21 UTC 2020
New revision: 368125
URL: https://svnweb.freebsd.org/changeset/base/368125

  libc: Add pthread_attr_get_np(3) stub, reporting ESRCH.

  This seems to be required by recent clang asan.
  I do not see other way than put the symbol under FBSD_1.0 version.

  PR:	251112
  Reported by:	Andrew Stitcher <astitcher@apache.org>
  Reviewed by:	emaste
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week
  Differential revision:	https://reviews.freebsd.org/D27389

Comment 12 commit-hook freebsd_committer 2020-12-05 09:08:31 UTC
A commit references this bug:

Author: kib
Date: Sat Dec  5 09:08:28 UTC 2020
New revision: 368361
URL: https://svnweb.freebsd.org/changeset/base/368361

  MFC r368125:
  libc: Add pthread_attr_get_np(3) stub, reporting ESRCH.

  PR:	251112

_U  stable/12/