Bug 239475
| Summary: | Linking libthr with -nodefaultlibs statically can cause infinite recursion | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Alex Richardson <arichardson> |
| Component: | threads | Assignee: | freebsd-threads (Nobody) <threads> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | CC: | kib |
| Priority: | --- | ||
| Version: | 11.2-RELEASE | ||
| Hardware: | Any | ||
| OS: | Any | ||
For me it was reproduced as infinite loop, perhaps due to the the tail call elimination. The issue is that when -lc is passed first, __pthread_cleanup_push_imp is found in libc, and then the libthr jump table, which references the symbol, is satisfied with the same libc definition.
Try this, I did not even compiled with the patch. It might require some additional tweaking to get the stuff actually working.
diff --git a/lib/libthr/thread/thr_clean.c b/lib/libthr/thread/thr_clean.c
index 5a93d94a7e5..7bc7d62b617 100644
--- a/lib/libthr/thread/thr_clean.c
+++ b/lib/libthr/thread/thr_clean.c
@@ -49,6 +49,10 @@ __FBSDID("$FreeBSD$");
__weak_reference(_pthread_cleanup_push, pthread_cleanup_push);
__weak_reference(_pthread_cleanup_pop, pthread_cleanup_pop);
+/* help static linking when libc symbols have preference */
+__weak_reference(__pthread_cleanup_push_imp, __pthread_cleanup_push_imp1);
+__weak_reference(__pthread_cleanup_pop_imp, pthread_cleanup_pop_imp1);
+
void
__pthread_cleanup_push_imp(void (*routine)(void *), void *arg,
struct _pthread_cleanup_info *info)
diff --git a/lib/libthr/thread/thr_init.c b/lib/libthr/thread/thr_init.c
index 7b043a38b1f..22802c0ae1a 100644
--- a/lib/libthr/thread/thr_init.c
+++ b/lib/libthr/thread/thr_init.c
@@ -202,6 +202,10 @@ STATIC_LIB_REQUIRE(_thread_state_running);
#define DUAL_ENTRY(entry) \
(pthread_func_t)entry, (pthread_func_t)entry
+void __pthread_cleanup_push_imp1(void (*)(void *), void *,
+ struct _pthread_cleanup_info *);
+void __pthread_cleanup_pop_imp1(int);
+
static pthread_func_t jmp_table[][2] = {
{DUAL_ENTRY(_pthread_atfork)}, /* PJT_ATFORK */
{DUAL_ENTRY(_pthread_attr_destroy)}, /* PJT_ATTR_DESTROY */
@@ -265,8 +269,8 @@ static pthread_func_t jmp_table[][2] = {
{DUAL_ENTRY(_pthread_setspecific)}, /* PJT_SETSPECIFIC */
{DUAL_ENTRY(_pthread_sigmask)}, /* PJT_SIGMASK */
{DUAL_ENTRY(_pthread_testcancel)}, /* PJT_TESTCANCEL */
- {DUAL_ENTRY(__pthread_cleanup_pop_imp)},/* PJT_CLEANUP_POP_IMP */
- {DUAL_ENTRY(__pthread_cleanup_push_imp)},/* PJT_CLEANUP_PUSH_IMP */
+ {DUAL_ENTRY(__pthread_cleanup_pop_imp1)},/* PJT_CLEANUP_POP_IMP */
+ {DUAL_ENTRY(__pthread_cleanup_push_imp1)},/* PJT_CLEANUP_PUSH_IMP */
{DUAL_ENTRY(_pthread_cancel_enter)}, /* PJT_CANCEL_ENTER */
{DUAL_ENTRY(_pthread_cancel_leave)}, /* PJT_CANCEL_LEAVE */
{DUAL_ENTRY(_pthread_mutex_consistent)},/* PJT_MUTEX_CONSISTENT */
Thanks, I guess that should work. However, we probably have to do it for all the functions in the table? Or maybe only the ones referenced in libc? This seems to be: void _pthread_cancel_enter(int); void _pthread_cancel_leave(int); void ___pthread_cleanup_push_imp(void (*)(void *), void *, struct _pthread_cleanup_info *); void ___pthread_cleanup_pop_imp(int); A commit references this bug: Author: kib Date: Wed Jul 31 19:27:24 UTC 2019 New revision: 350481 URL: https://svnweb.freebsd.org/changeset/base/350481 Log: Avoid conflicts with libc symbols in libthr jump table. In some corner cases of static linking and unexpected libraries order on the linker command line, libc symbol might preempt the same libthr symbol, in which case libthr jump table points back to libc causing either infinite recursion or loop. Handle all of such symbols by using private libthr names for them, ensuring that the right pointers are installed into the table. In collaboration with: arichardson PR: 239475 Tested by: pho MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential revision: https://reviews.freebsd.org/D21088 Changes: head/lib/libthr/thread/thr_attr.c head/lib/libthr/thread/thr_cancel.c head/lib/libthr/thread/thr_clean.c head/lib/libthr/thread/thr_cond.c head/lib/libthr/thread/thr_detach.c head/lib/libthr/thread/thr_equal.c head/lib/libthr/thread/thr_exit.c head/lib/libthr/thread/thr_fork.c head/lib/libthr/thread/thr_getthreadid_np.c head/lib/libthr/thread/thr_init.c head/lib/libthr/thread/thr_join.c head/lib/libthr/thread/thr_kill.c head/lib/libthr/thread/thr_main_np.c head/lib/libthr/thread/thr_mutex.c head/lib/libthr/thread/thr_mutexattr.c head/lib/libthr/thread/thr_once.c head/lib/libthr/thread/thr_private.h head/lib/libthr/thread/thr_rwlock.c head/lib/libthr/thread/thr_self.c head/lib/libthr/thread/thr_sig.c head/lib/libthr/thread/thr_spec.c A commit references this bug: Author: kib Date: Wed Jul 31 20:04:40 UTC 2019 New revision: 350483 URL: https://svnweb.freebsd.org/changeset/base/350483 Log: Avoid conflicts with libc symbols in libthr jump table. In some corner cases of static linking and unexpected libraries order on the linker command line, libc symbol might preempt the same libthr symbol, in which case libthr jump table points back to libc causing either infinite recursion or loop. Handle all of such symbols by using private libthr names for them, ensuring that the right pointers are installed into the table. In collaboration with: arichardson PR: 239475 Tested by: pho MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential revision: https://reviews.freebsd.org/D21088 Changes: head/lib/libthr/thread/thr_private.h |
For the following test program: ``` #include <stdio.h> #include <pthread.h> int main() { printf("pthread_create = %p\n", (void*)&pthread_create); return 0; } ``` If I link -lthr after -lc, I get infinite recursion in the pthread interposing table since the value in libthr.a is resolved to the one from libc which then just calls itself. #0 0x000000000026c0bc in __pthread_cleanup_push_imp () #1 0x000000000026c30a in printf () #2 0x000000000026c25d in main () The following fails: $ clang -static -o test-c-first -nodefaultlibs -lc -lthr test.c -fuse-ld=lld && ./test-c-first Whereas this works: clang -static -o test-thr-first -nodefaultlibs -lthr -lc test.c -fuse-ld=lld && ./test-thr-first pthread_create = 0x24dbc0 I am hitting this issue while writing some tests that want to avoid pulling in all default libs, but the order if libraries on the link command line is difficult to change. I can probably work around it, but it seems like this infinite recursion should not happen even if you link in the wrong order. I'm not sure why the functions in the libthr table resolve to the libc ones instead of the ones in libthr.