| Message ID | 4F31EDC9.2060600@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Feb 07, 2012 at 10:36:41PM -0500, Patrick Marlier wrote: > Hi, > > The problem in this PR is that with PIE, getsectdata does not return the > position of tm_clone_table after the relocation. > While _dyld_get_image_vmaddr_slide(0) is enough for PIE, this is not > enough for dylib. > I did not find an easy API function to get position of the > tm_clone_table for a shared library (dylib). So the only way I found is > to get the mach_header address of the current dylib (via > _dyld_get_image_header_containing_address), iterate over loaded binaries > to find the current shared library and use _dyld_get_image_vmaddr_slide > to find the position. > Any other proposal (my knowledge of darwin is really limited)? > > Can someone do a bootstrap and test libitm on darwin (I have a limited > access to a darwin machine, at least libitm tests pass)? Thanks! Done. Native configuration is x86_64-apple-darwin11.3.0 === libitm tests === Running target unix/-m32 FAIL: libitm.c++/eh-1.C execution test === libitm Summary for unix/-m32 === # of expected passes 25 # of unexpected failures 1 # of expected failures 3 # of unsupported tests 1 Running target unix/-m64 FAIL: libitm.c++/eh-1.C execution test === libitm Summary for unix/-m64 === # of expected passes 25 # of unexpected failures 1 # of expected failures 3 # of unsupported tests 1 === libitm Summary === # of expected passes 50 # of unexpected failures 2 # of expected failures 6 # of unsupported tests 2 Compiler version: gcc libitm Platform: x86_64-apple-darwin11.3.0 configure flags: --prefix=/sw --prefix=/sw/lib/gcc4.7 --mandir=/sw/share/man --infodir=/sw/lib/gcc4.7/info --with-build-config=bootstrap-lto --enable-stage1-languages=c,lto --enable-languages=c,c++,fortran,lto,objc,obj-c++,java --with-gmp=/sw --with-libiconv-prefix=/sw --with-ppl=/sw --with-cloog=/sw --with-mpc=/sw --with-system-zlib --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib --program-suffix=-fsf-4.7 --enable-checking=release --enable-cloog-backend=isl I believe the remaining libitm.c++/eh-1.C execution test failures are due to the weakref linker bug currently in Xcode 4.x (radr://10466868) which I hae been told will be fixed in the next Xcode release after Xcode 4.3. Jack > > If tests passed, ok for 4.7? > -- > Patrick Marlier. > libgcc: > > PR libitm/52042 > * config/darwin-crt-tm.c: Changes for PIE and shared library. > > Index: config/darwin-crt-tm.c > =================================================================== > --- config/darwin-crt-tm.c (revision 183968) > +++ config/darwin-crt-tm.c (working copy) > @@ -26,8 +26,20 @@ see the files COPYING3 and COPYING.RUNTIME respect > #include <mach-o/dyld.h> > > /* not listed in mach-o/dyld.h for some reason. */ > -extern char * getsectdata (const char*,const char*,unsigned long*); > +extern char *getsectdatafromheader (struct mach_header*, const char*, > + const char*, unsigned long*); > +extern char *getsectdatafromheader_64 (struct mach_header_64*, const char*, > + const char*, unsigned long*); > > +#ifdef __LP64__ > +#define GET_DATA_TMCT(mh,size) \ > + getsectdatafromheader_64 ((struct mach_header_64*) mh, \ > + "__DATA", "__tm_clone_table", size) > +#else > +#define GET_DATA_TMCT(mh,size) \ > + getsectdatafromheader (mh, "__DATA", "__tm_clone_table", size) > +#endif > + > #define WEAK __attribute__((weak)) > > extern void _ITM_registerTMCloneTable (void *, size_t) WEAK; > @@ -39,17 +51,27 @@ void __doTMRegistrations (void) __attribute__ ((co > > void __doTMRegistrations (void) > { > - char * tm_clone_table_sect_data; > + struct mach_header *mh; > + char *tmct_fixed, *tmct = NULL; > unsigned long tmct_siz; > + unsigned int i, img_count; > > - tm_clone_table_sect_data = getsectdata ("__DATA", > - "__tm_clone_table", > - &tmct_siz); > + mh = _dyld_get_image_header_containing_address ( > + (const void*)&__doTMRegistrations); > + tmct_fixed = GET_DATA_TMCT (mh, &tmct_siz); > tmct_siz /= (sizeof (size_t) * 2); > + > + img_count = _dyld_image_count(); > + for (i = 0; i < img_count && tmct == NULL; i++) > + { > + if (mh == _dyld_get_image_header(i)) > + tmct = tmct_fixed + (unsigned long)_dyld_get_image_vmaddr_slide(i); > + } > + > if (_ITM_registerTMCloneTable != NULL > - && tm_clone_table_sect_data != NULL > + && tmct != NULL > && tmct_siz > 0) > - _ITM_registerTMCloneTable (tm_clone_table_sect_data, (size_t)tmct_siz); > + _ITM_registerTMCloneTable ((void *)tmct, (size_t)tmct_siz); > } > > #endif > @@ -60,18 +82,27 @@ void __doTMdeRegistrations (void) __attribute__ (( > > void __doTMdeRegistrations (void) > { > - char * tm_clone_table_sect_data; > + struct mach_header *mh; > + char *tmct_fixed, *tmct = NULL; > unsigned long tmct_siz; > + unsigned int i, img_count; > > - tm_clone_table_sect_data = getsectdata ("__DATA", > - "__tm_clone_table", > - &tmct_siz); > - > + mh = _dyld_get_image_header_containing_address ( > + (const void *)&__doTMdeRegistrations); > + tmct_fixed = GET_DATA_TMCT (mh, &tmct_siz); > + tmct_siz /= (sizeof (size_t) * 2); > + > + img_count = _dyld_image_count(); > + for (i = 0; i < img_count && tmct == NULL; i++) > + { > + if (mh == _dyld_get_image_header(i)) > + tmct = tmct_fixed + (unsigned long)_dyld_get_image_vmaddr_slide(i); > + } > + > if (_ITM_deregisterTMCloneTable != NULL > - && tm_clone_table_sect_data != NULL > + && tmct != NULL > && tmct_siz > 0) > - _ITM_deregisterTMCloneTable (tm_clone_table_sect_data); > - > + _ITM_deregisterTMCloneTable ((void *)tmct); > } > > #endif
Hi Patrick, thanks for looking at this .. On 8 Feb 2012, at 03:36, Patrick Marlier wrote: > The problem in this PR is that with PIE, getsectdata does not return > the position of tm_clone_table after the relocation. > While _dyld_get_image_vmaddr_slide(0) is enough for PIE, this is not > enough for dylib. > I did not find an easy API function to get position of the > tm_clone_table for a shared library (dylib). So the only way I found > is to get the mach_header address of the current dylib (via > _dyld_get_image_header_containing_address), iterate over loaded > binaries to find the current shared library and use > _dyld_get_image_vmaddr_slide to find the position. > Any other proposal (my knowledge of darwin is really limited)? > > Can someone do a bootstrap and test libitm on darwin (I have a > limited access to a darwin machine, at least libitm tests pass)? > Thanks! > > If tests passed, ok for 4.7? > -- > Patrick Marlier. > libgcc: > > PR libitm/52042 > * config/darwin-crt-tm.c: Changes for PIE and shared library. > > <darwin-pie2.patch> looks good to me .. and DTRT on Darwin 9 too .. (but I can't approve). one nit (also mea culpa in the original...) /* not listed in mach-o/dyld.h for some reason. */ -extern char * getsectdata (const char*,const char*,unsigned long*); +extern char *getsectdatafromheader (struct mach_header*, const char*, + const char*, unsigned long*); +extern char *getsectdatafromheader_64 (struct mach_header_64*, const char*, + const char*, unsigned long*); these are in <mach-o/getsect.h> (dyld documentation glitch ...) === Sometime: I wonder if we should move the definition of the section name to a common place so that we don't need to worry about keeping it in step. Or even better have some compiler-wide mechanism for dealing with the fact that named sections might have different literals/ semantics per target. Iain
On Feb 7, 2012, at 7:36 PM, Patrick Marlier wrote: > The problem in this PR is that with PIE, getsectdata does not return the position of tm_clone_table after the relocation. > If tests passed, ok for 4.7? Ok. Thanks for all your hard work. If you want to change it to include the header and delete the declarations in the source, that is pre-approved as well. > PR libitm/52042 > * config/darwin-crt-tm.c: Changes for PIE and shared library.
On 02/08/2012 12:12 AM, Jack Howarth wrote: > I believe the remaining libitm.c++/eh-1.C execution test failures are due to > the weakref linker bug currently in Xcode 4.x (radr://10466868) which I hae been > told will be fixed in the next Xcode release after Xcode 4.3. > Jack Humm... In fact, not completely. Of course, if the linker was working, the HAVE_ELF_STYLE_WEAKREF will be defined and no problem. Otherwise in the eh-1.c, we can see that _ITM_cxa_allocation_exception is using the dummy function __cxa_allocation_exception defined into eh_cpp.cc but not the one from libstdc++. There is no dynamic symbol resolution since the function is defined in the file. I do not really know why those functions are defined here, I think it may causes more problems than it solves for darwin. When Rainer introduced this, he mentioned "on Tru64 UNIX. Of course it cannot work to provide only a single dummy function, but all weak definitions must be backed by dummy definitions on that platform." http://patchwork.ozlabs.org/patch/126007/ Iain mentioned this when he introduced the dummy def for darwin: *** FWIW, weakref actually work (at runtime) for earlier Darwin - it's just that refs either need to be satisfied by dummies at link time - - or the library namespace has to be flattened (which is generally undesirable). I think refs (from libstdc++) should be satisfied at link time with -lstdc++ but probably I am missing something? Note that removing definitions in eh_cpp.cc make the test passes. Thanks guys! -- Patrick.
Hi Patrick, On 9 Feb 2012, at 05:10, Patrick Marlier wrote: > On 02/08/2012 12:12 AM, Jack Howarth wrote: >> I believe the remaining libitm.c++/eh-1.C execution test failures >> are due to >> the weakref linker bug currently in Xcode 4.x (radr://10466868) >> which I hae been >> told will be fixed in the next Xcode release after Xcode 4.3. >> Jack > > Humm... In fact, not completely. Of course, if the linker was > working, the HAVE_ELF_STYLE_WEAKREF will be defined and no problem. > > Otherwise in the eh-1.c, we can see that > _ITM_cxa_allocation_exception is using the dummy function > __cxa_allocation_exception defined into eh_cpp.cc but not the one > from libstdc++. There is no dynamic symbol resolution since the > function is defined in the file. > > I do not really know why those functions are defined here, I think > it may causes more problems than it solves for darwin. > > When Rainer introduced this, he mentioned "on Tru64 UNIX. Of course > it cannot work to provide only a single dummy function, but all weak > definitions must be backed by dummy definitions on that platform." > http://patchwork.ozlabs.org/patch/126007/ > > Iain mentioned this when he introduced the dummy def for darwin: > *** FWIW, weakref actually work (at runtime) for earlier Darwin Weak refs have been working a (long) time on Darwin @ runtime - don't remember what version they were introduced ... but maybe from almost/ actually the start. > - it's just that refs either need to be satisfied by dummies at link > time - We are dealing with tool bugs (or, at least, changes in behavior) - the Linker would not [intentionally, see below*] accept unsatisfied linkage for weak refs @ Darwin <= 9 (XCode 3.1.4) ... and then for the XCode 3.2.x tool-chain on Darwin 10 it does accept it (i.e. ELF- like semantics work). Then (an acknowledged bug) with the (current) XCode 4.x tool-chain on either D10 or D11 the linker is again not accepting. * The (original, Darwin version, weak ref) idea, as documented, is that weak refs are for optional features - so the developer @ link- time supplies the library reference (place where the reference would be found, if present - this tells dyld what the proper two-level name of the lib is, when present). Then, at runtime dyld should not barf if the reference is not present. This is a different approach from the ELF weak-ref where the reference does not need to be supplied @ link-time - although it seems that Darwin is shifting towards supporting both from D10 onwards. > - or the library namespace has to be flattened (which is generally > undesirable). this would be a (IMO very) Bad Idea because we'd be doing it for every exe that included libitm... bleah. The two-level lib namespace is one of Darwin's nicer features ;) > I think refs (from libstdc++) should be satisfied at link time with - > lstdc++ but probably I am missing something? when you link c++ code, that is what is required .. but obv. we don't want lstdc++ on the link line for c (because the library *will* be found @ runtime - and thus the routines would be called). I have an idea ... perhaps we supply (yet another) crt that is linked last on the line and provides the dummies. That way the linkage will be satisfied from libstd++ for c++ and the dummies for c. will try and see if I can fit a trial of that idea in on D9 & D10. > Note that removing definitions in eh_cpp.cc make the test passes. but it will make the c tests fail for D9 and any version of Darwin on which elf-style weak refs is not currently working ... cheers Iain > > Thanks guys! > -- > Patrick.
Jack, Can I ask you to close this PR? http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52042 Indeed, my bugzilla account is a simple one and I cannot to change the bug status... -- Patrick On 02/07/2012 10:36 PM, Patrick Marlier wrote: > Hi, > > The problem in this PR is that with PIE, getsectdata does not return the > position of tm_clone_table after the relocation. > While _dyld_get_image_vmaddr_slide(0) is enough for PIE, this is not > enough for dylib. > I did not find an easy API function to get position of the > tm_clone_table for a shared library (dylib). So the only way I found is > to get the mach_header address of the current dylib (via > _dyld_get_image_header_containing_address), iterate over loaded binaries > to find the current shared library and use _dyld_get_image_vmaddr_slide > to find the position. > Any other proposal (my knowledge of darwin is really limited)? > > Can someone do a bootstrap and test libitm on darwin (I have a limited > access to a darwin machine, at least libitm tests pass)? Thanks! > > If tests passed, ok for 4.7? > -- > Patrick Marlier. > libgcc: > > PR libitm/52042 > * config/darwin-crt-tm.c: Changes for PIE and shared library. >
On Feb 28, 2012, at 12:20 PM, Patrick Marlier wrote:
> Can I ask you to close this PR?
Done.
Index: config/darwin-crt-tm.c =================================================================== --- config/darwin-crt-tm.c (revision 183968) +++ config/darwin-crt-tm.c (working copy) @@ -26,8 +26,20 @@ see the files COPYING3 and COPYING.RUNTIME respect #include <mach-o/dyld.h> /* not listed in mach-o/dyld.h for some reason. */ -extern char * getsectdata (const char*,const char*,unsigned long*); +extern char *getsectdatafromheader (struct mach_header*, const char*, + const char*, unsigned long*); +extern char *getsectdatafromheader_64 (struct mach_header_64*, const char*, + const char*, unsigned long*); +#ifdef __LP64__ +#define GET_DATA_TMCT(mh,size) \ + getsectdatafromheader_64 ((struct mach_header_64*) mh, \ + "__DATA", "__tm_clone_table", size) +#else +#define GET_DATA_TMCT(mh,size) \ + getsectdatafromheader (mh, "__DATA", "__tm_clone_table", size) +#endif + #define WEAK __attribute__((weak)) extern void _ITM_registerTMCloneTable (void *, size_t) WEAK; @@ -39,17 +51,27 @@ void __doTMRegistrations (void) __attribute__ ((co void __doTMRegistrations (void) { - char * tm_clone_table_sect_data; + struct mach_header *mh; + char *tmct_fixed, *tmct = NULL; unsigned long tmct_siz; + unsigned int i, img_count; - tm_clone_table_sect_data = getsectdata ("__DATA", - "__tm_clone_table", - &tmct_siz); + mh = _dyld_get_image_header_containing_address ( + (const void*)&__doTMRegistrations); + tmct_fixed = GET_DATA_TMCT (mh, &tmct_siz); tmct_siz /= (sizeof (size_t) * 2); + + img_count = _dyld_image_count(); + for (i = 0; i < img_count && tmct == NULL; i++) + { + if (mh == _dyld_get_image_header(i)) + tmct = tmct_fixed + (unsigned long)_dyld_get_image_vmaddr_slide(i); + } + if (_ITM_registerTMCloneTable != NULL - && tm_clone_table_sect_data != NULL + && tmct != NULL && tmct_siz > 0) - _ITM_registerTMCloneTable (tm_clone_table_sect_data, (size_t)tmct_siz); + _ITM_registerTMCloneTable ((void *)tmct, (size_t)tmct_siz); } #endif @@ -60,18 +82,27 @@ void __doTMdeRegistrations (void) __attribute__ (( void __doTMdeRegistrations (void) { - char * tm_clone_table_sect_data; + struct mach_header *mh; + char *tmct_fixed, *tmct = NULL; unsigned long tmct_siz; + unsigned int i, img_count; - tm_clone_table_sect_data = getsectdata ("__DATA", - "__tm_clone_table", - &tmct_siz); - + mh = _dyld_get_image_header_containing_address ( + (const void *)&__doTMdeRegistrations); + tmct_fixed = GET_DATA_TMCT (mh, &tmct_siz); + tmct_siz /= (sizeof (size_t) * 2); + + img_count = _dyld_image_count(); + for (i = 0; i < img_count && tmct == NULL; i++) + { + if (mh == _dyld_get_image_header(i)) + tmct = tmct_fixed + (unsigned long)_dyld_get_image_vmaddr_slide(i); + } + if (_ITM_deregisterTMCloneTable != NULL - && tm_clone_table_sect_data != NULL + && tmct != NULL && tmct_siz > 0) - _ITM_deregisterTMCloneTable (tm_clone_table_sect_data); - + _ITM_deregisterTMCloneTable ((void *)tmct); } #endif
Hi, The problem in this PR is that with PIE, getsectdata does not return the position of tm_clone_table after the relocation. While _dyld_get_image_vmaddr_slide(0) is enough for PIE, this is not enough for dylib. I did not find an easy API function to get position of the tm_clone_table for a shared library (dylib). So the only way I found is to get the mach_header address of the current dylib (via _dyld_get_image_header_containing_address), iterate over loaded binaries to find the current shared library and use _dyld_get_image_vmaddr_slide to find the position. Any other proposal (my knowledge of darwin is really limited)? Can someone do a bootstrap and test libitm on darwin (I have a limited access to a darwin machine, at least libitm tests pass)? Thanks! If tests passed, ok for 4.7? -- Patrick Marlier. libgcc: PR libitm/52042 * config/darwin-crt-tm.c: Changes for PIE and shared library.