This is an archive of the discontinued LLVM Phabricator instance.

Allow TSAN to work with libc-internal locking on FreeBSD
Changes PlannedPublic

Authored by arichardson on Feb 8 2021, 2:18 AM.

Details

Reviewers
dim
emaste
dvyukov
kib
krytarowski
vitalybuka
Group Reviewers
Restricted Project
Summary

On FreeBSD locking inside libc (e.g. for stdio) calls the libthr private
API (using _pthread_* functions)rather than the standard pthread_*.
This can lead to many false-positive race reports unless we also intercept
the version with the leading underscore.
Unfortunately this is a rather large diff since all pthread interceptors
had to be changed to use a new macro.

I don't have access to a NetBSD system for testing, but it seems like this
approach could also be used to handle the NetBSD __libc_pthread_* functions.

Diff Detail

Event Timeline

arichardson requested review of this revision.Feb 8 2021, 2:18 AM
arichardson created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2021, 2:18 AM

Can you follow the NetBSD approach and go for something like TSAN_MAYBE_INTERCEPT_NETBSD_ALIAS instead of patching shared code between all platforms?

Can you follow the NetBSD approach and go for something like TSAN_MAYBE_INTERCEPT_NETBSD_ALIAS instead of patching shared code between all platforms?

I can definitely do that and intercept only the locking ones. I chose this approach because intercepting all aliases makes it less likely to miss one required function that only rarely results in false-positives.

Would it be possible for NetBSD to also use this approach for the __libc_pthread_ functions? Or is only a subset exported as __libc_pthread_?

#elif SANITIZER_NETBSD
#define INTERCEPTOR_PTHREAD(ret, func, ...)      \
  INTERCEPTOR(ret, __libc_pthread_##func, __VA_ARGS__) \
  ALIAS(WRAPPER_NAME(pthread_##func));           \
  INTERCEPTOR(ret, pthread_##func, __VA_ARGS__)
#define INTERCEPT_PTHREAD_FUNCTION(name) \
  INTERCEPT_FUNCTION(pthread_##name);    \
  INTERCEPT_FUNCTION(__libc_pthread_##name)
krytarowski added a comment.EditedFeb 8 2021, 4:26 AM

Would it be possible for NetBSD to also use this approach for the libc_pthread_ functions? Or is only a subset exported as libc_pthread_?

No. It's a subset that might go on libc major version bump away. It's a legacy indirection subset, not used in new pthread calls.

Would it be possible for NetBSD to also use this approach for the libc_pthread_ functions? Or is only a subset exported as libc_pthread_?

No. It's a subset that might go on libc bump away. It's a legacy indirection subset, not used in new pthread calls.

Thanks for the explanation. In that case it might make more sense to restrict this to the FreeBSD functions defined in lib/libc/gen/_pthread_stubs.c

vitalybuka requested changes to this revision.Aug 10 2021, 12:09 PM
vitalybuka added a subscriber: vitalybuka.

I assume it's stale?

This revision now requires changes to proceed.Aug 10 2021, 12:09 PM
arichardson planned changes to this revision.Aug 10 2021, 3:56 PM