Bug 13049

Summary: Generates R_ARM_NONE relocations for stub_* symbols when using -fdata-sections
Product: binutils Reporter: Mike Hommey <mh-sourceware>
Component: ldAssignee: unassigned
Status: RESOLVED FIXED    
Severity: normal CC: amodra, christophe.lyon, nickc
Priority: P2    
Version: 2.22   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: Rename stub section
Rename stub sections in ARM, PPC and HPPA

Description Mike Hommey 2011-08-01 14:16:51 UTC
$ echo 'const char *stub_foo[] = { "a" };' > foo.c
$ gcc -shared -o foo.so foo.c -fPIC -fdata-sections
$ readelf -r foo.so

Relocation section '.rel.dyn' at offset 0x370 contains 7 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
000084e0  00000017 R_ARM_RELATIVE   
000084e4  00000017 R_ARM_RELATIVE   
000085f4  00000017 R_ARM_RELATIVE   
00000000  00000000 R_ARM_NONE       
000085e8  00000315 R_ARM_GLOB_DAT    00000000   __cxa_finalize
000085ec  00000415 R_ARM_GLOB_DAT    00000000   __gmon_start__
000085f0  00000515 R_ARM_GLOB_DAT    00000000   _Jv_RegisterClasses

Relocation section '.rel.plt' at offset 0x3a8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
000085e0  00000316 R_ARM_JUMP_SLOT   00000000   __cxa_finalize
000085e4  00000416 R_ARM_JUMP_SLOT   00000000   __gmon_start__

$ gcc -shared -o foo.so foo.c -fPIC
$ readelf -r foo.so

Relocation section '.rel.dyn' at offset 0x370 contains 7 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
000084e0  00000017 R_ARM_RELATIVE   
000084e4  00000017 R_ARM_RELATIVE   
000085f4  00000017 R_ARM_RELATIVE   
000085f8  00000017 R_ARM_RELATIVE   
000085e8  00000315 R_ARM_GLOB_DAT    00000000   __cxa_finalize
000085ec  00000415 R_ARM_GLOB_DAT    00000000   __gmon_start__
000085f0  00000515 R_ARM_GLOB_DAT    00000000   _Jv_RegisterClasses

Relocation section '.rel.plt' at offset 0x3a8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
000085e0  00000316 R_ARM_JUMP_SLOT   00000000   __cxa_finalize
000085e4  00000416 R_ARM_JUMP_SLOT   00000000   __gmon_start__

gold doesn't have this problem.
Comment 1 Nick Clifton 2011-10-18 15:40:18 UTC
Created attachment 6019 [details]
Rename stub section
Comment 2 Nick Clifton 2011-10-18 15:44:20 UTC
Hi Mike,

  This was an interesting one.  It turns out that using the word "stub" at the start of your variable was significant.  With -fdata-sections enabled the variable was placed into a section called .data.rel.local.stub_foo which confused the linker into thinking that it was a special section for containing other stubs.

  I have uploaded a patch which should fix the problem by choosing a name for the stub section suffix which is not valid as the start of the name of a C variable.  (Ie "__stub").  Please can you try it out and let me know if it works for you.

Cheers
  Nick
Comment 3 christophe.lyon@st.com 2011-10-19 15:41:10 UTC
Hi Nick,
I have 2 comments regarding your patch:
1- Could you make the comment in elf32-arm.c slightly more explicit about how a user could create such a name "by accident"? I mean, in 6 months will we remember that "by accident" means using -fdata-sections and a variable named "stub" ?

2- You should probably extend your fix to hppa and ppc (from which I originally derived the stub generation).

Thanks,

Christophe.
Comment 4 Nick Clifton 2011-10-20 14:30:47 UTC
Created attachment 6021 [details]
Rename stub sections in ARM, PPC and HPPA
Comment 5 Nick Clifton 2011-10-20 14:38:45 UTC
Hi Christophe,

  OK, I have uploaded a revised patch which contains an expanded comment, and similar fixes for the elf64-ppc.c and elf32-hppa.c files.  I have also removed the changes to the linker scripts, since these were not necessary.  (It was a case of me being too paranoid).

  Are you happy with this version of the patch ?

Cheers
  Nick
Comment 6 christophe.lyon@st.com 2011-10-20 16:08:24 UTC
Hi Nick,
The comment is much clearer indeed, thanks.
I have noticed a typo "defintion" near the end of the comment.

However, I am wondering whether prefixing with "__" is secure enough: in my understanding, the C standard defines the "__" namespace as reserved for tools, but as such it could very well be used by some GCC or GLIBC magic, or by a end-user, couldn't it?

What about using something like "_._" which could still be used in assembly, but much less likely? But I won't argue strongly on this point.

Thanks
Comment 7 Sourceware Commits 2011-10-20 16:27:16 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2011-10-20 16:27:04

Modified files:
	bfd            : ChangeLog elf32-arm.c elf64-ppc.c elf32-hppa.c 

Log message:
	PR ld/13049
	* elf32-arm.c (STUB_SUFFIX): Avoid collision with user namespace
	symbol names.
	* elf64-ppc.c (STUB_SUFFIX): Likewise.
	* elf32-hppa.c (STUB_SUFFIX): Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5496&r2=1.5497
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-arm.c.diff?cvsroot=src&r1=1.278&r2=1.279
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-ppc.c.diff?cvsroot=src&r1=1.367&r2=1.368
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-hppa.c.diff?cvsroot=src&r1=1.179&r2=1.180
Comment 8 Nick Clifton 2011-10-20 16:32:22 UTC
Hi Christophe,

  Thanks for spelling typo catch.

  You are right of course.  System type tools, including the compiler, can generate symbols with a double underscore prefix.  But I think that in this context the stub suffix *is* a system type symbol, and so it is correct to use the __ prefix.  Plus if they really want to a user could always generate a symbol with a _._ prefix, or any other combination that we might choose, so we cannot win  

  So - I have gone ahead and checked the patch in.

Cheers
  Nick
Comment 9 Alan Modra 2011-10-26 02:26:56 UTC
I don't see the need for any change of STUB_SUFFIX in elf64-ppc.c and elf32-hppa.c.  Only arm does that awful !strstr (stub_sec->name, STUB_SUFFIX) test to check for stub sections.
Comment 10 christophe.lyon@st.com 2011-10-26 08:14:52 UTC
Hi Alan,
I think you are right. I had forgotten about this difference betweem ARM and HPPA/PPC. For the record, it was introduced after this discussion: http://sourceware.org/ml/binutils/2009-03/msg00400.html

Nick, sorry for being too paranoid too.
Comment 11 Sourceware Commits 2011-10-26 09:48:02 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2011-10-26 09:47:58

Modified files:
	bfd            : ChangeLog elf64-ppc.c elf32-hppa.c 

Log message:
	PR ld/13049
	* elf64-ppc.c (STUB_SUFFIX): Revert previous delta.
	* elf32-hppa.c (STUB_SUFFIX): Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5505&r2=1.5506
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-ppc.c.diff?cvsroot=src&r1=1.368&r2=1.369
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-hppa.c.diff?cvsroot=src&r1=1.180&r2=1.181
Comment 12 Nick Clifton 2011-10-26 09:49:41 UTC
Hi Alan,

  Well I felt that there was no harm in being paranoid, but if you prefer the original stub section suffix then so be it.  I have reverted the changes to elf64-ppc.c and elf32-hppa.c.

Cheers
  Nick
Comment 13 Alan Modra 2011-10-26 11:31:33 UTC
Thanks Nick.  Incidentally, it looks to me that the only real problem with using strstr to check for stub sections is the one in elf32_arm_final_link_relocate,
introduced in http://sourceware.org/ml/binutils/2010-02/msg00460.html.

In all other cases you are dealing with sections in stub_bfd, all linker created so random user named sections aren't involved.  I suspect the following is the proper fix, but I don't play enough with arm to be sure.

Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.280
diff -u -p -r1.280 elf32-arm.c
--- bfd/elf32-arm.c	24 Oct 2011 12:52:37 -0000	1.280
+++ bfd/elf32-arm.c	26 Oct 2011 11:27:31 -0000
@@ -8069,7 +8069,8 @@ elf32_arm_final_link_relocate (reloc_how
 			  ".tls_vars") == 0)
 	  && ((r_type != R_ARM_REL32 && r_type != R_ARM_REL32_NOI)
 	      || !SYMBOL_CALLS_LOCAL (info, h))
-	  && (!strstr (input_section->name, STUB_SUFFIX))
+	  && !(input_bfd == globals->stub_bfd
+	       && strstr (input_section->name, STUB_SUFFIX))
 	  && (h == NULL
 	      || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
 	      || h->root.type != bfd_link_hash_undefweak)
Comment 14 Jackie Rosen 2014-02-16 17:50:40 UTC Comment hidden (spam)