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]

powerpc64 call stubs


This brings powerpc64 ld in line with gold regarding calls allowed to
go via a plt call stub or toc-adjusting stub without a following nop.
A long time ago ld allowed tail calls, but this is wrong because we'll
return to the caller's caller with the wrong toc pointer.  I fixed
that for shared libraries but allowed tail calls in an executable for
some reason.  Probably just muddy thinking on my part, because there's
no difference between an executable and a shared library regarding the
need to restore the toc pointer.  Perhaps it was because some testcase
failed, most likely due to the g++ issue I comment on in the patch.

bfd/
	* elf64-ppc.c (struct ppc_stub_hash_entry): Delete "addend".
	(ppc64_elf_size_stubs): Don't set "addend".
	(ppc64_elf_relocate_section): Don't allow calls via
	toc-adjusting stubs without a following nop even in an
	executable, except for self-calls and both libc_start_main
	and .libc_start_main.
gold/
	* powerpc.cc (Target_powerpc::Relocate::relocate): Update self-call
	comment.

Index: bfd/elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.414
diff -u -p -r1.414 elf64-ppc.c
--- bfd/elf64-ppc.c	3 Jul 2013 00:45:50 -0000	1.414
+++ bfd/elf64-ppc.c	3 Jul 2013 02:13:16 -0000
@@ -3618,9 +3618,6 @@ struct ppc_stub_hash_entry {
   struct ppc_link_hash_entry *h;
   struct plt_entry *plt_ent;
 
-  /* And the reloc addend that this was derived from.  */
-  bfd_vma addend;
-
   /* Where this stub is being called from, or, in the case of combined
      stub sections, the first input section in the group.  */
   asection *id_sec;
@@ -11752,7 +11749,6 @@ ppc64_elf_size_stubs (struct bfd_link_in
 		    }
 		  stub_entry->h = hash;
 		  stub_entry->plt_ent = plt_ent;
-		  stub_entry->addend = irela->r_addend;
 
 		  if (stub_entry->h != NULL)
 		    htab->stub_globals += 1;
@@ -13015,60 +13011,89 @@ ppc64_elf_relocate_section (bfd *output_
 	    {
 	      bfd_boolean can_plt_call = FALSE;
 
+	      /* All of these stubs will modify r2, so there must be a
+		 branch and link followed by a nop.  The nop is
+		 replaced by an insn to restore r2.  */
 	      if (rel->r_offset + 8 <= input_section->size)
 		{
-		  unsigned long nop;
-		  nop = bfd_get_32 (input_bfd, contents + rel->r_offset + 4);
-		  if (nop == NOP
-		      || nop == CROR_151515 || nop == CROR_313131)
-		    {
-		      if (h != NULL
-			  && (h == htab->tls_get_addr_fd
-			      || h == htab->tls_get_addr)
-			  && !htab->no_tls_get_addr_opt)
+		  unsigned long br;
+
+		  br = bfd_get_32 (input_bfd,
+				   contents + rel->r_offset);
+		  if ((br & 1) != 0)
+		    {
+		      unsigned long nop;
+
+		      nop = bfd_get_32 (input_bfd,
+					contents + rel->r_offset + 4);
+		      if (nop == NOP
+			  || nop == CROR_151515 || nop == CROR_313131)
 			{
-			  /* Special stub used, leave nop alone.  */
+			  if (h != NULL
+			      && (h == htab->tls_get_addr_fd
+				  || h == htab->tls_get_addr)
+			      && !htab->no_tls_get_addr_opt)
+			    {
+			      /* Special stub used, leave nop alone.  */
+			    }
+			  else
+			    bfd_put_32 (input_bfd, LD_R2_40R1,
+					contents + rel->r_offset + 4);
+			  can_plt_call = TRUE;
 			}
-		      else
-			bfd_put_32 (input_bfd, LD_R2_40R1,
-				    contents + rel->r_offset + 4);
-		      can_plt_call = TRUE;
 		    }
 		}
 
-	      if (!can_plt_call)
+	      if (!can_plt_call && h != NULL)
 		{
-		  if (stub_entry->stub_type == ppc_stub_plt_call
-		      || stub_entry->stub_type == ppc_stub_plt_call_r2save)
-		    {
-		      /* If this is a plain branch rather than a branch
-			 and link, don't require a nop.  However, don't
-			 allow tail calls in a shared library as they
-			 will result in r2 being corrupted.  */
-		      unsigned long br;
-		      br = bfd_get_32 (input_bfd, contents + rel->r_offset);
-		      if (info->executable && (br & 1) == 0)
-			can_plt_call = TRUE;
-		      else
-			stub_entry = NULL;
-		    }
-		  else if (h != NULL
-			   && strcmp (h->elf.root.root.string,
-				      ".__libc_start_main") == 0)
+		  const char *name = h->elf.root.root.string;
+
+		  if (*name == '.')
+		    ++name;
+
+		  if (strncmp (name, "__libc_start_main", 17) == 0
+		      && (name[17] == 0 || name[17] == '@'))
 		    {
-		      /* Allow crt1 branch to go via a toc adjusting stub.  */
+		      /* Allow crt1 branch to go via a toc adjusting
+			 stub.  Other calls that never return could do
+			 the same, if we could detect such.  */
 		      can_plt_call = TRUE;
 		    }
-		  else
+		}
+
+	      if (!can_plt_call)
+		{
+		  /* g++ as of 20130507 emits self-calls without a
+		     following nop.  This is arguably wrong since we
+		     have conflicting information.  On the one hand a
+		     global symbol and on the other a local call
+		     sequence, but don't error for this special case.
+		     It isn't possible to cheaply verify we have
+		     exactly such a call.  Allow all calls to the same
+		     section.  */
+		  asection *code_sec = sec;
+
+		  if (get_opd_info (sec) != NULL)
 		    {
-		      info->callbacks->einfo
-			(_("%P: %H: call to `%T' lacks nop, can't restore toc; "
-			   "recompile with -fPIC"),
-			   input_bfd, input_section, rel->r_offset, sym_name);
+		      bfd_vma off = (relocation + addend
+				     - sec->output_section->vma
+				     - sec->output_offset);
 
-		      bfd_set_error (bfd_error_bad_value);
-		      ret = FALSE;
+		      opd_entry_value (sec, off, &code_sec, NULL, FALSE);
 		    }
+		  if (code_sec == input_section)
+		    can_plt_call = TRUE;
+		}
+
+	      if (!can_plt_call)
+		{
+		  info->callbacks->einfo
+		    (_("%P: %H: call to `%T' lacks nop, can't restore toc; "
+		       "recompile with -fPIC"),
+		     input_bfd, input_section, rel->r_offset, sym_name);
+
+		  bfd_set_error (bfd_error_bad_value);
+		  ret = FALSE;
 		}
 
 	      if (can_plt_call
Index: gold/powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.93
diff -u -p -r1.93 powerpc.cc
--- gold/powerpc.cc	27 Jun 2013 23:20:35 -0000	1.93
+++ gold/powerpc.cc	3 Jul 2013 02:13:16 -0000
@@ -6346,10 +6346,13 @@ Target_powerpc<size, big_endian>::Reloca
 	    }
 	  if (!can_plt_call)
 	    {
-	      // This is not an error in one special case: A self
-	      // call.  It isn't possible to cheaply verify we have
-	      // such a call so just check for a call to the same
-	      // section.
+	      // g++ as of 20130507 emits self-calls without a
+	      // following nop.  This is arguably wrong since we have
+	      // conflicting information.  On the one hand a global
+	      // symbol and on the other a local call sequence, but
+	      // don't error for this special case.
+	      // It isn't possible to cheaply verify we have exactly
+	      // such a call.  Allow all calls to the same section.
 	      bool ok = false;
 	      Address code = value;
 	      if (gsym->source() == Symbol::FROM_OBJECT

-- 
Alan Modra
Australia Development Lab, IBM


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