Patchwork Change i386 PIC thunk name

login
register
mail settings
Submitter Bernd Schmidt
Date Aug. 29, 2011, 4:11 p.m.
Message ID <4E5BBA27.4070805@codesourcery.com>
Download mbox | patch
Permalink /patch/112107/
State New
Headers show

Comments

Bernd Schmidt - Aug. 29, 2011, 4:11 p.m.
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).


Bernd
* config/i386/i386.c (get_pc_thunk_name): Change prefix to
	"__i686_get_pc_thunk.".
H.J. Lu - Aug. 29, 2011, 4:36 p.m.
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?
H.J. Lu - Aug. 29, 2011, 4:37 p.m.
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
Bernd Schmidt - Aug. 29, 2011, 4:38 p.m.
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
Jakub Jelinek - Aug. 29, 2011, 4:49 p.m.
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

Patch

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);
 }