| Summary: | ld.so is miscompiled by GCC 11 | ||
|---|---|---|---|
| Product: | glibc | Reporter: | H.J. Lu <hjl.tools> |
| Component: | dynamic-link | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | carlos, fweimer, hubicka, jakub, slyich |
| Priority: | P2 | Flags: | fweimer:
security-
|
| Version: | 2.33 | ||
| Target Milestone: | 2.33 | ||
| See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98110 | ||
| Host: | Target: | x86-64 | |
| Build: | Last reconfirmed: | ||
|
Description
H.J. Lu
2020-12-02 23:06:25 UTC
commit 520d5ad337eaa15860a5a964daf7ca46cf31c029 Author: Jan Hubicka <jh@suse.cz> Date: Sat Nov 14 13:52:36 2020 +0100 Detect EAF flags in ipa-modref A minimal patch for the EAF flags discovery. It works only in local ipa-modref and gives up on cyclic SSA graphs. It improves pt_solution_includes disambiguations twice. is the first bad commit. Only lightly tested fix, i?86 will need similar change (with __seg_gs):
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index a08bf972de..ccb5f24d92 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -180,11 +180,16 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
-# define THREAD_SELF \
+# if __GNUC_PREREQ (6, 0)
+# define THREAD_SELF \
+ (*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
+# else
+# define THREAD_SELF \
({ struct pthread *__self; \
asm ("mov %%fs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
+# endif
/* Magic for libthread_db to know how to do THREAD_SELF. */
# define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */
How about this
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index a08bf972de..4eeab0e7a9 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -180,11 +180,19 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
-# define THREAD_SELF \
+# if __GNUC_PREREQ (11, 0)
+# define THREAD_SELF \
+ ({ struct pthread *__self; \
+ __self = (struct pthread *) (__builtin_thread_pointer () \
+ + offsetof (struct pthread, header.self)); \
+ __self;})
+# else
+# define THREAD_SELF \
({ struct pthread *__self; \
asm ("mov %%fs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
+# endif
/* Magic for libthread_db to know how to do THREAD_SELF. */
# define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */
__builtin_thread_pointer is potentially trapping (which is the root of the problem). Does GCC know about that? It also results in a load followed by an add, instead of a single load, I think. Using the __segfs or __seggs namespaces looks preferable to me. (In reply to Florian Weimer from comment #4) > __builtin_thread_pointer is potentially trapping (which is the root of the > problem). Does GCC know about that? It also results in a load followed by an > add, instead of a single load, I think. Using the __segfs or __seggs > namespaces looks preferable to me. GCC makes it const __attribute__((nothrow)), and that is I think ok except in the dynamic linker. I don't think + __self = (struct pthread *) (__builtin_thread_pointer () \ + + offsetof (struct pthread, header.self)); \ does what the old code did, which was movq %fs:16, __self but your version is movq %fs:0, %reg; leaq 16(%reg), __self That would be *(struct pthread **) (__builtin_thread_pointer () + offsetof (struct pthread, header.self)) if we optimize that back to just movq %fs:16, __self But, as it is for x86 GCC 11+ only, isn't it better to use __seg_{f,g}s that should work already since GCC 6? (In reply to Florian Weimer from comment #4) > __builtin_thread_pointer is potentially trapping (which is the root of the > problem). Does GCC know about that? It also results in a load followed by an > add, instead of a single load, I think. Using the __segfs or __seggs > namespaces looks preferable to me. OK. Shouldn't all %fs references be replaced by __seg_fs? Fixed for glibc 2.33 by: commit 1d9cbb96082e646de7515a1667efa041ffb79958 Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Dec 3 13:33:44 2020 +0100 x86: Fix THREAD_SELF definition to avoid ld.so crash (bug 27004) The previous definition of THREAD_SELF did not tell the compiler that %fs (or %gs) usage is invalid for the !DL_LOOKUP_GSCOPE_LOCK case in _dl_lookup_symbol_x. As a result, ld.so could try to use the TCB before it was initialized. As the comment in tls.h explains, asm volatile is undesirable here. Using the __seg_fs (or __seg_gs) namespace does not interfere with optimization, and expresses that THREAD_SELF is potentially trapping. (In reply to H.J. Lu from comment #6) > OK. Shouldn't all %fs references be replaced by __seg_fs? Yes, I see we require GCC 6.2 as a minimum. So the __GLIBC_PREREQ is actually unnecessary. |