This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR ld/15365: Restrict __ehdr_start's export class (was: RE: [COMMITTED PATCH] Use __ehdr_start, if available, as fallback for AT_PHDR.)

On Thu, 25 Apr 2013, Alan Modra wrote:

> You need to check link_info.hash->type == bfd_link_elf_hash_table
> here.

 Ah, actually I'm glad you mention this -- I've been wondering about this 
conditional and found it confusing.  Now what confused me was it was the 
only place I noticed to use it, but now I have checked again and it's not 
exactly true, except that all the other places use is_elf_hash_table.

 And what I've been wondering about is when actually the hash table could 
be non-ELF in ELF emulation code?  Linking non-ELF input files into ELF 
output?

>  If fact I think you could use bfd_elf_record_link_assignment
> to make __ehdr_start hidden, as follows.  If this passes testing then
> please commit.  Oh, and a bonus point if you fix
> bfd_elf_record_link_assignment to not turn STV_INTERNAL into
> STV_HIDDEN.

 Yes, it works and I like it better than my original proposal too.  I have 
adjusted the test cases accordingly for the symbol attributes being set 
differently with your change (STB_LOCAL/STV_DEFAULT rather than 
STB_GLOBAL/STV_HIDDEN).

 And (for the record) it turns I was confused as to when the linker-script 
symbol assignments are finalised -- usually a bit later, however it still 
seems the best place here to set the export class for the upcoming 
__ehdr_start symbol as at this point bfd_elf_record_link_assignment will 
already know which symbols have already come from --defsym command-line 
options or input objects and which will need to be supplied by the linker.

 Here's the final version.  The generic test case now only accepts local 
symbols and with the linker scripts now used with the new MIPS tests there 
is no need to limit them to certain MIPS targets only now, so I have 
removed the conditional.

 The MIPS segfault I mentioned in the other thread turned out (as I 
suspected) unrelated to this change.  I have filed PR ld/15428 to track it 
and KFAILed the relevant test case.

 Finally I'd prefer to apply the change to 
gld${EMULATION_NAME}_before_allocation in two steps -- the move of 
lang_for_each_statement into the conditional is a functionally independent 
change, hence I split it off.

 OK for these changes?

2013-05-02  Alan Modra  <amodra@gmail.com>

        ld/
	* emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation):
	Only call lang_for_each_statement if an ELF hash table is used.

and:

2013-05-02  Maciej W. Rozycki  <macro@codesourcery.com>

        gold/
        PR ld/15365
        * layout.cc (Layout::finalize): Make __ehdr_start STV_HIDDEN.

2013-05-02  Alan Modra  <amodra@gmail.com>

        ld/
        PR ld/15365
        * emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation):
        Restrict __ehdr_start's export class to no less than STV_HIDDEN.

2013-05-02  Maciej W. Rozycki  <macro@codesourcery.com>

        ld/testsuite/
        PR ld/15365
        * ld-elf/ehdr_start.d: Expect __ehdr_start to be STB_LOCAL.
        * ld-mips-elf/ehdr_start-1.nd: New test.
	* ld-mips-elf/ehdr_start-2.nd: New test.
	* ld-mips-elf/ehdr_start-1.ld: New test linker script.
	* ld-mips-elf/ehdr_start-2.ld: New test linker script.
        * ld-mips-elf/ehdr_start-new.s: New test source.
        * ld-mips-elf/ehdr_start-o32.s: New test source.
        * ld-mips-elf/mips-elf.exp: Run the new tests.

  Maciej

binutils-amodra-elf-hash-statement-assignment.diff
Index: binutils-fsf-trunk-quilt/ld/emultempl/elf32.em
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/emultempl/elf32.em	2013-05-02 14:44:11.093213916 +0100
+++ binutils-fsf-trunk-quilt/ld/emultempl/elf32.em	2013-05-02 14:47:10.123869755 +0100
@@ -1484,12 +1484,14 @@ gld${EMULATION_NAME}_before_allocation (
   bfd *abfd;
 
   if (is_elf_hash_table (link_info.hash))
-    _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
+    {
+      _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
 
-  /* If we are going to make any variable assignments, we need to let
-     the ELF backend know about them in case the variables are
-     referred to by dynamic objects.  */
-  lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
+      /* If we are going to make any variable assignments, we need to
+	 let the ELF backend know about them in case the variables are
+	 referred to by dynamic objects.  */
+      lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
+    }
 
   /* Let the ELF backend work out the sizes of any sections required
      by dynamic linking.  */

binutils-amodra-ehdr_start-hidden.diff
Index: binutils-fsf-trunk-quilt/gold/layout.cc
===================================================================
--- binutils-fsf-trunk-quilt.orig/gold/layout.cc	2013-05-02 15:16:36.000000000 +0100
+++ binutils-fsf-trunk-quilt/gold/layout.cc	2013-05-02 20:45:15.883057391 +0100
@@ -2713,7 +2713,7 @@ Layout::finalize(const Input_objects* in
     symtab->define_in_output_segment("__ehdr_start", NULL,
 				     Symbol_table::PREDEFINED, load_seg, 0, 0,
 				     elfcpp::STT_NOTYPE, elfcpp::STB_GLOBAL,
-				     elfcpp::STV_DEFAULT, 0,
+				     elfcpp::STV_HIDDEN, 0,
 				     Symbol::SEGMENT_START, true);
 
   // Set the file offsets of all the non-data sections we've seen so
Index: binutils-fsf-trunk-quilt/ld/emultempl/elf32.em
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/emultempl/elf32.em	2013-05-02 20:45:12.000000000 +0100
+++ binutils-fsf-trunk-quilt/ld/emultempl/elf32.em	2013-05-02 20:45:15.883057391 +0100
@@ -1487,6 +1487,13 @@ gld${EMULATION_NAME}_before_allocation (
     {
       _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
 
+      /* Make __ehdr_start hidden if it has been referenced, to
+	 prevent the symbol from being dynamic.  */
+      if (!bfd_elf_record_link_assignment (link_info.output_bfd, &link_info,
+					   "__ehdr_start", TRUE, TRUE))
+	einfo ("%P%F: failed to record assignment to %s: %E\n",
+	       "__ehdr_start");
+
       /* If we are going to make any variable assignments, we need to
 	 let the ELF backend know about them in case the variables are
 	 referred to by dynamic objects.  */
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/ehdr_start.d
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/testsuite/ld-elf/ehdr_start.d	2013-05-02 15:16:36.000000000 +0100
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-elf/ehdr_start.d	2013-05-02 20:45:15.883057391 +0100
@@ -4,5 +4,5 @@
 #target: *-*-linux* *-*-gnu* *-*-nacl*
 
 #...
-[0-9a-f]*000 [ADRT] __ehdr_start
+[0-9a-f]*000 [Adrt] __ehdr_start
 #pass
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-1.ld
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-1.ld	2013-05-02 20:45:15.883057391 +0100
@@ -0,0 +1,9 @@
+ENTRY (__start)
+SECTIONS
+{
+  . = 0x12300000 + SIZEOF_HEADERS;
+  .text : { *(.text) }
+  . = 0x23400000;
+  HIDDEN (_gp = ALIGN (16) + 0x7ff0);
+  .got : { *(.got) }
+}
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-1.nd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-1.nd	2013-05-02 20:45:15.883057391 +0100
@@ -0,0 +1,4 @@
+Symbol table '\.symtab' contains [0-9]+ entries:
+#...
+ *[0-9]+: 0*12300000 +0 (?:NOTYPE|OBJECT) +LOCAL +DEFAULT +[0-9]+ __ehdr_start
+#pass
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-2.ld
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-2.ld	2013-05-02 20:45:15.883057391 +0100
@@ -0,0 +1,10 @@
+ENTRY (__start)
+SECTIONS
+{
+  . = 0x12300000 + SIZEOF_HEADERS;
+  .text : { *(.text) }
+  . = 0x23400000;
+  __ehdr_start = .;
+  HIDDEN (_gp = ALIGN (16) + 0x7ff0);
+  .got : { *(.got) }
+}
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-2.nd
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-2.nd	2013-05-02 21:12:19.093869062 +0100
@@ -0,0 +1,4 @@
+Symbol table '\.symtab' contains [0-9]+ entries:
+#...
+ *[0-9]+: 0*23400000 +0 (?:NOTYPE|OBJECT) +LOCAL +DEFAULT +[0-9]+ __ehdr_start
+#pass
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-new.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-new.s	2013-05-02 20:45:15.883057391 +0100
@@ -0,0 +1,13 @@
+	.abicalls
+	.text
+	.weak	__ehdr_start
+	.globl	__start
+	.ent	__start
+	.frame	$29, 0, $31
+	.mask	0x00000000, 0
+__start:
+	.cplocal $2
+	.cpsetup $t9, $zero, __start
+	lw	$2, __ehdr_start
+	jr	$31
+	.end	__start
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-o32.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/ehdr_start-o32.s	2013-05-02 20:45:15.883057391 +0100
@@ -0,0 +1,14 @@
+	.abicalls
+	.text
+	.weak	__ehdr_start
+	.globl	__start
+	.ent	__start
+	.frame	$29, 0, $31
+	.mask	0x00000000, 0
+__start:
+	.set	noreorder
+	.cpload	$25
+	.set	reorder
+	lw	$2, __ehdr_start
+	jr	$31
+	.end	__start
Index: binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/ld/testsuite/ld-mips-elf/mips-elf.exp	2013-05-02 15:16:36.000000000 +0100
+++ binutils-fsf-trunk-quilt/ld/testsuite/ld-mips-elf/mips-elf.exp	2013-05-02 20:45:15.883057391 +0100
@@ -709,3 +709,28 @@ foreach { abi } $abis {
 		"readelf -A export-class-call16-${abi}.gd"] \
 	    "export-class-call16-${abi}.so"]]
 }
+
+# Magic __ehdr_start symbol tests.
+set abis [concat o32 [expr {$has_newabi ? "n32 n64" : ""}]]
+foreach { abi } $abis {
+    set suff [string map {o32 o32 n32 new n64 new} $abi]
+    run_ld_link_tests [list \
+        [list \
+            "MIPS magic __ehdr_start symbol test 1 ($abi)" \
+            "$abi_ldflags($abi) -T ehdr_start-1.ld" "" \
+            "$abi_asflags($abi)" \
+            [list ehdr_start-${suff}.s] \
+            [list "readelf -s ehdr_start-1.nd"] \
+            "ehdr_start-1-${abi}"]]
+    if [regexp "(?:n32|n64)" "$abi"] {
+	setup_kfail "mips*-*-*" "ld/15428"
+    }
+    run_ld_link_tests [list \
+        [list \
+            "MIPS magic __ehdr_start symbol test 2 ($abi)" \
+            "$abi_ldflags($abi) -T ehdr_start-2.ld" "" \
+            "$abi_asflags($abi)" \
+            [list ehdr_start-${suff}.s] \
+            [list "readelf -s ehdr_start-2.nd"] \
+            "ehdr_start-2-${abi}"]]
+}
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]