Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Wed, May 4, 2011 at 00:52, Chris Demetriou wrote: > Ian, per your previous comment (which I read as pre-approval for this > change 8-), I'm planning to commit the attached as soon as it's done > testing. > > > chris > - > [libgcc/ChangeLog] > 2011-05-04 Chris Demetriou > > * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... > (__x86.get_pc_thunk.bx): To this. > (__morestack): Adjust for rename, remove undef of __i686. > Passed my tests (bootstrap C + test C on ubuntu lucid x86-64, with and w/o --with-pic), so submitted @ r173391. Slightly different changelog message: 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename to... (__x86.get_pc_thunk.bx): ...this. (__morestack): Adjust for rename, remove undef of __i686. (more consistent with other renames in the same file. 8-) sorry for the churn. chris
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Tue, May 3, 2011 at 16:39, Chris Demetriou wrote: > > It is also OK if you s/__i686/__x86/ to correspond to your earlier > > change. Either way is OK. > > Well, that change is not in trunk. > Should that change move to trunk, yes, it's appropriate to change this > as well (though strictly not *necessary*). *sigh* As it turns out, adding __i686.get_pc_thunk.bx to this assembly code -- *cough* this _preprocessed_ assembly code -- chokes in certain configurations. For instance, if you configure with: --with-arch-32=pentium3 then libgcc will build with -march=pentium3 The way the symbol-visibility files are built and processed, morestack.vis is generated from morestack.S, and contains: .hidden __i686.get_pc_thunk.bx morestack.vis is included via -include when compiling morestack.S. ./morestack.vis:4: Error: junk at end of line, first unrecognized character is `1' As someone smarter than me might have guessed... in this configuration, __i686 is defined to be '1'. So, even if the trunk get_thunk code continues to use __i686.get_pc_thunk., it's necessary to use something different here. Kinda feels like I got smacked in the face by walking into my own rake... *sigh* Ian, per your previous comment (which I read as pre-approval for this change 8-), I'm planning to commit the attached as soon as it's done testing. chris - [libgcc/ChangeLog] 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... (__x86.get_pc_thunk.bx): To this. (__morestack): Adjust for rename, remove undef of __i686. [libgcc/ChangeLog] 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... (__x86.get_pc_thunk.bx): To this. (__morestack): Adjust for rename, remove undef of __i686. Index: libgcc/config/i386/morestack.S === --- libgcc/config/i386/morestack.S (revision 173355) +++ libgcc/config/i386/morestack.S (working copy) @@ -278,8 +278,7 @@ movl 4(%esp),%eax # Function argument. movl %eax,(%esp) #ifdef __PIC__ -#undef __i686 - call __i686.get_pc_thunk.bx # %ebx may not be set up for us. + call __x86.get_pc_thunk.bx # %ebx may not be set up for us. addl $_GLOBAL_OFFSET_TABLE_, %ebx call _Unwind_Resume@PLT # Resume unwinding. #else @@ -451,20 +450,19 @@ #if !defined(__x86_64__) && defined(__PIC__) # Output the thunk to get PC into bx, since we use it above. -# (__i686 was already undef'd above; don't need to worry about it here.) - .section .text.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat - .globl __i686.get_pc_thunk.bx - .hidden __i686.get_pc_thunk.bx + .section .text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat + .globl __x86.get_pc_thunk.bx + .hidden __x86.get_pc_thunk.bx #ifdef __ELF__ - .type __i686.get_pc_thunk.bx, @function + .type __x86.get_pc_thunk.bx, @function #endif -__i686.get_pc_thunk.bx: +__x86.get_pc_thunk.bx: .cfi_startproc movl (%esp), %ebx ret .cfi_endproc #ifdef __ELF__ - .size __i686.get_pc_thunk.bx, . - __i686.get_pc_thunk.bx + .size __x86.get_pc_thunk.bx, . - __x86.get_pc_thunk.bx #endif #endif
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Tue, May 3, 2011 at 15:05, Ian Lance Taylor wrote: > > > 2011-05-03 Chris Demetriou > > > > * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. > > No "libgcc" in libgcc/ChangeLog. Fixed, sorry. (That's what I get for pasting from svn status output. 8-) > It is also OK if you s/__i686/__x86/ to correspond to your earlier > change. Either way is OK. Well, that change is not in trunk. Should that change move to trunk, yes, it's appropriate to change this as well (though strictly not *necessary*). (On the google branches, my plan was to rename, yes.) chris
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
Chris Demetriou writes: > 2011-05-03 Chris Demetriou > > * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. No "libgcc" in libgcc/ChangeLog. This is OK. It is also OK if you s/__i686/__x86/ to correspond to your earlier change. Either way is OK. Thanks. Ian
[trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
After I submitted http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02422.html, Rong Xu noted that the resulting sources, if configured --with-pic, would not actually build properly due to missing __i686.get_pc_thunk.bx (due to my renaming that to __x86...). While that issue was exposed by my change, in that configuration, in summary my change exposed a latent issue in morestack.S. (Callers of __i686.get_pc_thunk.bx (etc.) are also supposed to define those functions -- because who knows whether the other objects they're linked against will provide them.) So, I've added the __i686.get_pc_thunk.bx function (as generated by current GCC) to libgcc/config/i386/morestack.S. (I also added __ELF__ #ifdefs and a .size directive for the function, following the apparent conventions in the file.) bootstrapped (C only) on Ubuntu Lucid x86-64, and ran -m32/-m64 tests for compilers configured as normal, and also --with-pic. No regressions before/after. Ian, OK for trunk? chris [libgcc/ChangeLog] 2011-05-03 Chris Demetriou * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. [libgcc/ChangeLog] 2011-05-03 Chris Demetriou * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. Index: libgcc/config/i386/morestack.S === --- libgcc/config/i386/morestack.S (revision 173287) +++ libgcc/config/i386/morestack.S (working copy) @@ -449,6 +449,24 @@ .size __morestack, . - __morestack #endif +#if !defined(__x86_64__) && defined(__PIC__) +# Output the thunk to get PC into bx, since we use it above. +# (__i686 was already undef'd above; don't need to worry about it here.) + .section .text.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat + .globl __i686.get_pc_thunk.bx + .hidden __i686.get_pc_thunk.bx +#ifdef __ELF__ + .type __i686.get_pc_thunk.bx, @function +#endif +__i686.get_pc_thunk.bx: + .cfi_startproc + movl (%esp), %ebx + ret + .cfi_endproc +#ifdef __ELF__ + .size __i686.get_pc_thunk.bx, . - __i686.get_pc_thunk.bx +#endif +#endif # The exception table. This tells the personality routine to execute # the exception handler.