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]

PR ld/11458: fix assert in elf32-cris.c:elf_cris_copy_indirect_symbol


You'll run into this bug when linking a non-pic file into a DSO,
and there is a non-pic pc-relative relocation (as almost any
non-pic symbol reference with CRIS v32) to a versioned symbol in
glibc, like abort.  The "real world" fix is to recompile with
-fpic, but it's nicer when the linker doesn't barf without
explanation.

I noticed another bug also fixed below.  The sections collected
in the list weren't the sections containing the relocations but
the *relocation sections*.  The code using the list of sections
wasn't consistent in its usage, whether as the real section or
its relocation section.  With the buggy code, this affected the
warning output such that there was also warnings and TEXTREL
markers for non-pic pc-relative relocations from read-write
sections (which are currently only possible with the .reloc
pseudo).

Tested check-ld for crosses to cris-elf, cris-axis-linux-gnu.
The test-case will be committed when the previous patch for
multiple warning lines is approved.  The bfd patch is committed.

bfd:
	PR ld/11458
	* elf32-cris.c (elf_cris_copy_indirect_symbol): Remove invalid
	assert of empty pcrel_relocs_copied on the direct symbol.  Instead
	of moving the list from the indirect symbol to the direct symbol,
	merge into any existing list.
	(cris_elf_check_relocs): Store the original section in the
	pcrel_relocs_copied list, not the relocation section.
	(elf_cris_discard_excess_dso_dynamics): Adjust accordingly to find
	the relocation section, for reducing its size.  Change the
	BFD_ASSERT into a check for the section being read-only, and only
	emit warnings and TEXTREL marker when there's an entry for a
	read-only section.

ld/testsuite:
	PR ld/11458
	* ld-cris/pcrelcp-1.d, ld-cris/pcrelcp-1.s: New test.

--- /dev/null	2010-03-13 03:21:56.794002387 +0100
+++ ld-cris/pcrelcp-1.d	2010-03-31 00:34:02.000000000 +0200
@@ -0,0 +1,34 @@
+#as: --no-underscore --em=criself
+#ld: -shared -m crislinux -z nocombreloc
+#ld_after_inputfiles: tmpdir/libdso-1b.so
+#warning: \A[^\n]*\.o, section `.text', to symbol `expfn@@TST2':\n
+#warning: [^\n]*recompile with -fPIC\Z
+#readelf: -a
+
+# Building a DSO with (unrecommended) non-pic pc-relative references
+# to a versioned symbol in a library got caught by an assert in
+# elf_cris_copy_indirect_symbol wherein the list of pc-relative
+# references wasn't merged, but simply asserted to be NULL before
+# copied to, on the merged-to (direct) symbol.  For versioned symbols,
+# there was an "extra" copy made, to make a base-version symbol, where
+# the copied-from pc-relative list was NULL but the copied-to symbol
+# already had a list merged.
+
+# The list was used to emit warning messages, but incorrectly held the
+# relocation section for the reference, resulting in warnings being
+# emitted for any section with a pc-relative relocation.
+
+# The test checks that there's a warning message only for the
+# read-only sections section (.text) (not the read-write sections),
+# that the correct number of relocations is emitted and we also check
+# for the TEXTREL dynamic marker.
+
+#...
+ 0x00000016 \(TEXTREL\)[ 	]+0x0
+#...
+Relocation section '\.rela\.text' at offset .* contains 4 entries:
+#...
+Relocation section '\.rela\.data' at offset .* contains 8 entries:
+#...
+Relocation section '.rela.data2' at offset .* contains 16 entries:
+#pass
--- /dev/null	2010-03-13 03:21:56.794002387 +0100
+++ ld-cris/pcrelcp-1.s	2010-03-30 07:19:21.000000000 +0200
@@ -0,0 +1,50 @@
+	.symver x,expfn@TST2	; .symver required to make @ part of name.
+	.global _start
+	.type	_start,@function
+_start:
+	.dword	0,0,0,0
+	.reloc	0,R_CRIS_32_PCREL,expfn
+	.reloc	4,R_CRIS_32_PCREL,expfn
+	.reloc	8,R_CRIS_32_PCREL,x
+	.reloc	12,R_CRIS_32_PCREL,x
+.Lfe3:
+	.size	_start,.Lfe3-_start
+
+	.data
+	.global tab1
+	.type	tab1,@object
+tab1:
+	.dword	0,0,0,0,0,0,0,0
+	.reloc	0,R_CRIS_32_PCREL,expfn
+	.reloc	4,R_CRIS_32_PCREL,expfn
+	.reloc	8,R_CRIS_32_PCREL,expfn
+	.reloc	12,R_CRIS_32_PCREL,expfn
+	.reloc	16,R_CRIS_32_PCREL,x
+	.reloc	20,R_CRIS_32_PCREL,x
+	.reloc	24,R_CRIS_32_PCREL,x
+	.reloc	28,R_CRIS_32_PCREL,x
+	.size	tab1,.-tab1
+
+	.section .data2,"aw",@progbits
+	.global tab2
+	.type	tab2,@object
+tab2:
+	.dword	0,0,0,0,0,0,0,0
+	.dword	0,0,0,0,0,0,0,0
+	.reloc	0,R_CRIS_32_PCREL,expfn
+	.reloc	4,R_CRIS_32_PCREL,expfn
+	.reloc	8,R_CRIS_32_PCREL,expfn
+	.reloc	12,R_CRIS_32_PCREL,expfn
+	.reloc	16,R_CRIS_32_PCREL,expfn
+	.reloc	20,R_CRIS_32_PCREL,expfn
+	.reloc	24,R_CRIS_32_PCREL,expfn
+	.reloc	28,R_CRIS_32_PCREL,expfn
+	.reloc	32,R_CRIS_32_PCREL,x
+	.reloc	36,R_CRIS_32_PCREL,x
+	.reloc	40,R_CRIS_32_PCREL,x
+	.reloc	44,R_CRIS_32_PCREL,x
+	.reloc	48,R_CRIS_32_PCREL,x
+	.reloc	52,R_CRIS_32_PCREL,x
+	.reloc	56,R_CRIS_32_PCREL,x
+	.reloc	60,R_CRIS_32_PCREL,x
+	.size	tab1,.-tab2

Index: elf32-cris.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-cris.c,v
retrieving revision 1.107
diff -p -u -r1.107 elf32-cris.c
--- elf32-cris.c	4 Feb 2010 09:16:39 -0000	1.107
+++ elf32-cris.c	31 Mar 2010 03:35:11 -0000
@@ -3106,12 +3106,37 @@ elf_cris_copy_indirect_symbol (struct bf
       return;
     }
 
-  BFD_ASSERT (edir->pcrel_relocs_copied == NULL);
   BFD_ASSERT (edir->gotplt_offset == 0 || eind->gotplt_offset == 0);
 
 #define XMOVOPZ(F, OP, Z) edir->F OP eind->F; eind->F = Z
 #define XMOVE(F) XMOVOPZ (F, +=, 0)
-  XMOVOPZ (pcrel_relocs_copied, =, NULL);
+  if (eind->pcrel_relocs_copied != NULL)
+    {
+      if (edir->pcrel_relocs_copied != NULL)
+	{
+	  struct elf_cris_pcrel_relocs_copied **pp;
+	  struct elf_cris_pcrel_relocs_copied *p;
+
+	  /* Add reloc counts against the indirect sym to the direct sym
+	     list.  Merge any entries against the same section.  */
+	  for (pp = &eind->pcrel_relocs_copied; *pp != NULL;)
+	    {
+	      struct elf_cris_pcrel_relocs_copied *q;
+	      p = *pp;
+	      for (q = edir->pcrel_relocs_copied; q != NULL; q = q->next)
+		if (q->section == p->section)
+		  {
+		    q->count += p->count;
+		    *pp = p->next;
+		    break;
+		  }
+	      if (q == NULL)
+		pp = &p->next;
+	    }
+	  *pp = edir->pcrel_relocs_copied;
+	}
+      XMOVOPZ (pcrel_relocs_copied, =, NULL);
+    }
   XMOVE (gotplt_refcount);
   XMOVE (gotplt_offset);
   XMOVE (reg_got_refcount);
@@ -3682,7 +3707,7 @@ cris_elf_check_relocs (bfd *abfd,
 	    eh = elf_cris_hash_entry (h);
 
 	    for (p = eh->pcrel_relocs_copied; p != NULL; p = p->next)
-	      if (p->section == sreloc)
+	      if (p->section == sec)
 		break;
 
 	    if (p == NULL)
@@ -3693,7 +3718,7 @@ cris_elf_check_relocs (bfd *abfd,
 		  return FALSE;
 		p->next = eh->pcrel_relocs_copied;
 		eh->pcrel_relocs_copied = p;
-		p->section = sreloc;
+		p->section = sec;
 		p->count = 0;
 		p->r_type = r_type;
 	      }
@@ -3951,8 +3976,13 @@ elf_cris_discard_excess_dso_dynamics (h,
 	  || info->symbolic))
     {
       for (s = h->pcrel_relocs_copied; s != NULL; s = s->next)
-	s->section->size -= s->count * sizeof (Elf32_External_Rela);
-
+	{
+	  asection *sreloc
+	    = _bfd_elf_get_dynamic_reloc_section (s->section->owner,
+						  s->section,
+						  /*rela?*/ TRUE);
+	  sreloc->size -= s->count * sizeof (Elf32_External_Rela);
+	}
       return TRUE;
     }
 
@@ -3963,21 +3993,20 @@ elf_cris_discard_excess_dso_dynamics (h,
      late).  */
 
   for (s = h->pcrel_relocs_copied; s != NULL; s = s->next)
-    {
-      BFD_ASSERT ((s->section->flags & SEC_READONLY) != 0);
+    if ((s->section->flags & SEC_READONLY) != 0)
+      {
+	/* FIXME: How do we make this optionally a warning only?  */
+	(*_bfd_error_handler)
+	  (_("%B, section `%A', to symbol `%s':\n"
+	     "  relocation %s should not be used"
+	     " in a shared object; recompile with -fPIC"),
+	   s->section->owner,
+	   s->section,
+	   h->root.root.root.string,
+	   cris_elf_howto_table[s->r_type].name);
 
-      /* FIXME: How do we make this optionally a warning only?  */
-      (*_bfd_error_handler)
-	(_("%B, section `%A', to symbol `%s':\n"
-	   "  relocation %s should not be used"
-	   " in a shared object; recompile with -fPIC"),
-	 s->section->owner,
-	 s->section,
-	 h->root.root.root.string,
-	 cris_elf_howto_table[s->r_type].name);
-
-      info->flags |= DF_TEXTREL;
-    }
+	info->flags |= DF_TEXTREL;
+      }
 
   return TRUE;
 }

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]