| Message ID | 4E5BBA27.4070805@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 29, 2011 at 9:11 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: > We currently generate > > __i686.get_pc_thunk.bx: > movl (%esp), %ebx > ret > in PIC binaries. This can cause problems if the assembly output ends up > in a .S file which is then compiled again: __i686 is a predefined macro > and expands to "1". This happens in glibc when compiling csu/crti.S, > which is generated from initfini.c; presumably this worked before > Richard removed the !TARGET_DEEP_BRANCH_PREDICTION code. > A simple way of fixing it would be to change the name of the thunk, as > below. Of course, libc also has copies of this code, so worst case we'd > end up with two of the thunks, which doesn't seem like a massive problem. > > Tested on i686-linux (with a few failures which I can attribute to a > different patch in the tree). > Doesn't Google have a patch to change it to __x86.get_pc_thunk.bx?
On Mon, Aug 29, 2011 at 9:36 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Aug 29, 2011 at 9:11 AM, Bernd Schmidt <bernds@codesourcery.com> wrote: >> We currently generate >> >> __i686.get_pc_thunk.bx: >> movl (%esp), %ebx >> ret >> in PIC binaries. This can cause problems if the assembly output ends up >> in a .S file which is then compiled again: __i686 is a predefined macro >> and expands to "1". This happens in glibc when compiling csu/crti.S, >> which is generated from initfini.c; presumably this worked before >> Richard removed the !TARGET_DEEP_BRANCH_PREDICTION code. >> A simple way of fixing it would be to change the name of the thunk, as >> below. Of course, libc also has copies of this code, so worst case we'd >> end up with two of the thunks, which doesn't seem like a massive problem. >> >> Tested on i686-linux (with a few failures which I can attribute to a >> different patch in the tree). >> > > Doesn't Google have a patch to change it to __x86.get_pc_thunk.bx? > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02422.html
On 08/29/11 18:37, H.J. Lu wrote: > On Mon, Aug 29, 2011 at 9:36 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> Doesn't Google have a patch to change it to __x86.get_pc_thunk.bx? >> > > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02422.html Cool. I guess I'll approve and commit this version, unless someone sees a reason not to do this. Will wait until tomorrow or so. Bernd
On Mon, Aug 29, 2011 at 06:38:11PM +0200, Bernd Schmidt wrote: > On 08/29/11 18:37, H.J. Lu wrote: > > On Mon, Aug 29, 2011 at 9:36 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > > >> Doesn't Google have a patch to change it to __x86.get_pc_thunk.bx? > >> > > > > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02422.html > > Cool. I guess I'll approve and commit this version, unless someone sees > a reason not to do this. Will wait until tomorrow or so. I don't care which name we use, but I agree it is useful to switch. There is a small possibility it might break some programs that call into __i686.get_pc_thunk.* in inline assembly etc., but it will be easy to notice and easy to autoconf. Jakub
Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 178030) +++ config/i386/i386.c (working copy) @@ -8339,7 +8339,7 @@ get_pc_thunk_name (char name[32], unsign gcc_assert (!TARGET_64BIT); if (USE_HIDDEN_LINKONCE) - sprintf (name, "__i686.get_pc_thunk.%s", reg_names[regno]); + sprintf (name, "__i686_get_pc_thunk.%s", reg_names[regno]); else ASM_GENERATE_INTERNAL_LABEL (name, "LPR", regno); }