Bug 165728
The problem is that libstdc++-devel headers use pthread_* functions
conditionally. If the program is linked with -lpthread, then STL headers
are thread-safe, if not, then they are not and nothing forces -lpthread
being linked in. This is done using #pragma weak pthread_create etc.
But as they are marked weak, which is needed for the STL inlines, they end up
being WEAK undefined symbols in the resulting object files, therefore linking
with -lpthread is not required and the symbols will be just NULL if libpthread
is not linked in.
To keep the libstdc++ behaviour of using pthread_* only when -lpthread is
linked in, but make user's code always use strong undefined symbols for these,
one solution would be to never inline those in the STL headers and only keep
them inside of libstdc++.{so,a}. This would slow things down, but more
importantly, break ABI compatibility, so is IMHO not an option for RHEL3 nor
RHEL4.
Unfortunately, GCC emits .weak even if the symbol is only used in an never used
inline function, so I'm afraid there is no easy solution.
I was trying:
extern "C" {
extern void f1 (void) __asm ("F1") __attribute__((weak));
extern void F1 (void);
extern void f2 (void) __asm ("F2") __attribute__((weak));
extern void F2 (void);
extern void f3 (void) __asm ("F3") __attribute__((weak));
extern void F3 (void);
}
extern inline void foo (void) { if (f1) f1 (); }
extern inline void bar (void) { if (f2) f2 (); }
void test (void) { bar (); F1 (); F2 (); F3 (); }
and
extern void F1 (void);
extern void F2 (void);
extern void F3 (void);
extern inline void foo (void) { extern __typeof (F1) F1 __attribute__((weak)); if
(F1) F1 (); }
extern inline void bar (void) { extern __typeof (F2) F2 __attribute__((weak)); if
(F2) F2 (); }
void test (void) { bar (); F1 (); F2 (); F3 (); }
and in both cases F1 and F2 were WEAK UNDEF symbols and only F3 was strong. If
also F1 was strong, then libstdc++ could use this, so that pthread_* would be
only weak if actually inlined into some code (which still would happen pretty
often).
Perhaps with the exception of pthread_create, which (unlike ObjC) C++ never
needs, so if getting an error if -lpthread is not linked in, but pthread_create
is used in the user code would be enough, then gthr*.h headers could have a
special magic macro that would disable declaring pthread_create weak and defining
__gthread_create function. libstdc++ headers then could define that macro before
including the gthr*.h header.
I don't think it's reasonable for an implicitly-included header file to make user-visible symbols weak like this. A better solution would probably involve moving the static variable in gthread_active_p to global scope, define it in gthr-posix.c, where pthread_create and others would then be made weak and referenced through wrapper functions or global pointers to functions that the header would use. Wrapper functions would enable the use of lazy relocations, but would probably make the code slightly slower. Another possibility that crosses my mind is to introduce means in gas to have both weak and strong references to the same symbol, such that, if there are any strong references, the symbol is not marked as weakundef in the symbol table. We'd then have to introduce syntax in GCC to support the introduction of alternate weak names for a symbol, such that uses thereof use the new gas syntax. Note that I'm not talking about a weak symbol that is an alias to another symbol, I'm talking about an alias to an external symbol that references it weakly. Note this is related to the issues experienced with libstdc++/24071 in gcc bugzilla. This has been in the FSF GCC as PR 4372 for a long time now. Created attachment 120314 [details]
Patch for binutils that introduces .weakref
This patch is a backport of the patch just installed in upstream binutils.
I've omitted the COFF bits, since they were not exactly trivial to backport and
we only use ELF anyway. The GCC patch might still take a bit to add here,
since it still hasn't been approved upstream, although the backport patch is
much simpler than what I'm proposing upstream.
Now, since this is a binutils patch, and this bug is about gcc, should we
create a new bug for the binutils feature that the gcc fix requires?
Created attachment 120342 [details]
Introduce support in GCC for attribute weakref, and use it in libraries. Requires assembler .weakref support.
This is a limited version of the patch I'm proposing for GCC mainline,
backported to RHEL 3's gcc. It is much simpler because it assumes the
assembler supports .weakref, and it's limited to posix threads.
My test run revealed GCC 3.2 lacks infrastructure in the test harness to support the new test. I'm tempted to leave the test out, since the lib build will have tested that first, but I'll poke at it a little bit to try to get something that works. If it is just some dg-require-* etc. and is something RHEL3 toolchain is supposed to support on all architectures, I guess replacing that with target i?86-*-linux* x86_64-*-linux* s390*-linux* ppc*-linux* powerpc*-linux* ia64-*-linux* would do the job very well. Or are you talking about dump scanning? It's dg-additional-sources. Completely ignored on GCC 3.2, and searching the wrong dir for additional sources on 3.4, even on a full test run (unlike GCC mainline, that fails with RUNTESTFLAGS=dg.exp but works in a full run) Well, if you can test in C++, then g++.old-deja's // Additional sources: foo.cc should work just fine in 3.2. Created attachment 120378 [details]
Introduce support in GCC for attribute weakref, and use it in libraries. Requires assembler .weakref support.
Yep, testing in C++ is fine (that's where the thing gets actually used!), and
I'd forgotten I'd added `Additional sources:' support myself to old-deja.exp
:-) Thanks for the suggestion. This revised patch passes C++ testing on 3.2
(and passed C testing except for the test now moved to C++; retesting now)
Created attachment 120399 [details]
Updated GCC patch, with better test, removal of unneeded .weak and fixed NULL-testing of weakrefs
Additional testing in preparation to submit the code to the net revealed one
critical and one cosmetical bug. The critical one is that comparing the
address of a weakref with NULL would produce incorrect results, since GCC would
incorrectly assume it to be non-zero, since weakrefs were not marked as
external as they should. The cosmetical one is that we emitted .weak
directives for used weakrefs; this was harmless, but totally unnecessary after
a .weakref directive. This patch fixes both. The test was enhanced to catch
the critical error.
Created attachment 120886 [details]
Updated GCC patch, that won't use weakref on other compilers
Jakub mentioned that we should be careful to not use weakref on other compilers
that don't support it, even though they pretend to be GCC in many other
regards. We've decided to use __GNUC_RH_RELEASE__ for this. The gthr-posix.h
patch becomes much uglier this way, unfortunately, but it's well worth it.
This patch depends on another patch that introduces __GNUC_RH_RELEASE__ in GCC
3.2, that I'll attach momentarily.
Created attachment 120892 [details]
Patch that introduces -D__GNUC_RH_RELEASE__ in gcc32
This patch is a requirement for the weakref patch to have the desired effect on
libstdc++. It will define __GNUC_RH_RELEASE__ to the RPM %release of the GCC
package. If `Red Hat' or `Red Hat Linux' are not in the beginning of the
parenthesized version name, or if the version number that follows does not
match compiler_version, then the macro is not defined. This is slightly
different from the gcc34 semantics, in which the built-in preprocessor always
knows its own version number so it can define the macro correctly even in the
presence of -V. I hope this is close enough, because we really can't do
better.
It seems there is a lot of crappy packages in the wild, e.g. nmap, that
#define __attribute__(x) /* Nothing */
In the nmap case it is because of a broken configure test.
I think for RHEL{3,4} we should add && !defined (__attribute__) to the
conditional for using weakref attribute (as unlike most __attribute__'s in the
headers here it is not informational and/or optimization hint, but without
the attribute it will not work at all) and if __attribute__ is defined,
fall back to the #pragma weak stuff. For FC5, we should keep it as is,
so that broken packages are eventually fixed.
Adding `!defined(__attribute__)' sounds like a good idea, indeed. Created attachment 121137 [details]
Don't use weakref ifdef __attribute__, and fix ppc64 -mcall-aixdesc weakrefs
Jakub pointed out that some projects #define away __attribute__, which would
break our weakrefs, so fallback to the old machinery in this case.
Also, Jakub reported .weakref was broken on ppc -m64 -mcall-aixdesc, because we
issued a weakref for the base symbol, but referenced the dot symbol. Fixed in
this updated patch.
While at that, I added a tab before .weakref, such that the output assembly
looks nicer.
Created attachment 121158 [details]
Updated binutils patch that fixes .weakref .foo, .bar on ppc
There were two problems affecting ppc64 in the last patch: when testing whether
the .sym corresponding to a sym was used, to force the sym used, we used a
function that implicitly referenced .sym, thus causing a strong reference to
the .sym to make to the output. Fixing that caused the unused weakref target
for sym to be removed even though .sym remained, so I narrowed the condition
for removal of a weakrefd in write.c. No testsuite regressions, and now
.weakref foo, bar
.weakref .foo, .bar
assembles to nothing, and adding
bl .foo
causes both bar and .bar to be output as weak undef symbols. This should fix
bootstrap on ppc64.
gcc-3.2.3-54 and binutils-2.14.90.0.4-42 built in dist-3.0E-qu-candidate. An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2006-0147.html |
g++ should fail to link the program if a pthread function is used, but no pthread libraries are being linked in. It is possible that the problem lies with libstdc++ which should link against the pthread libraries. This problem also occurs on RHEL4. $ cat pthread-linking-test.C #include <pthread.h> #include <stdlib.h> #include <unistd.h> #include <stdio.h> #include <iostream> static void *foo (void* data) { return 0; } int main(int argc, char **argv) { pthread_t id; if ( pthread_create(&id, 0, foo, 0) != 0 ) { perror("failed to create thread"); } } $ g++ -g -o pthread-linking-test pthread-linking-test.C $ ./pthread-linking-test Segmentation fault (core dumped) $ ldd pthread-linking-test libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x00101000) libm.so.6 => /lib/tls/libm.so.6 (0x00430000) libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00b19000) libc.so.6 => /lib/tls/libc.so.6 (0x00305000) /lib/ld-linux.so.2 (0x002ec000) $ diff -u pthread-linking-test.C pthread-linking-test2.C --- pthread-linking-test.C 2005-08-11 17:10:48.371915945 +0100 +++ pthread-linking-test2.C 2005-08-11 17:15:20.991874341 +0100 @@ -2,7 +2,6 @@ #include <stdlib.h> #include <unistd.h> #include <stdio.h> -#include <iostream> static void *foo (void* data) { $ g++ -g -o pthread-linking-test2 pthread-linking-test2.C /tmp/ccG5Jxra.o(.text+0x34): In function `main': /tmp/rubbihs/pthread-linking-test2.C:15: undefined reference to `pthread_create'collect2: ld returned 1 exit status