svn commit: r339710 - stable/11/contrib/elftoolchain/elfcopy

[Ed Maste]

Author: emaste
Date: Thu Oct 25 13:46:28 2018
New Revision: 339710
URL: https://svnweb.freebsd.org/changeset/base/339710

Log:
  elfcopy: avoid stripping relocations from static binaries
  
  MFC r339350: elfcopy: delete filter_reloc, it is broken and unnecessary
  
  elfcopy contained logic to filter individual relocations in STRIP_ALL
  mode.  However, this is not valid; relocations emitted by the linker are
  required, unless they apply to an entire section being removed (which is
  handled by other logic in elfcopy).
  
  Note that filter_reloc was also buggy: for RELA relocation sections it
  operated on uninitialized rel.r_info resulting in invalid operation.
  
  The logic most likely needs to be inverted: instead of removing
  relocations because their associated symbols are being removed, we must
  keep symbols referenced by relocations.  That said, in practice we do
  not encounter this code path today: objects being stripped are either
  dynamically linked binaries which retain .dynsym, or static binaries
  with no relocations.
  
  Just remove filter_reloc.  This fixes certain cases including statically
  linked binaries containing ifuncs.  Stripping binaries with relocations
  referencing removed symbols was already broken, and after this change
  may still be broken in a different way.
  
  MFC r339451: objcopy: restore behaviour required by GCC's build
  
  In r339350 filter_reloc() was removed, to fix the case of stripping
  statically linked binaries with relocations (which may come from ifunc
  use, for example).  As a side effect this changed the behaviour when
  stripping object files - the output was broken both before and after
  r339350, in different ways.  Unfortunately GCC's build process relies
  on the previous behaviour, so:
  
  - Revert r339350, restoring filter_reloc().
  - Fix an unitialized variable use (commited as r3638 in ELF Tool Chain).
  - Change filter_reloc() to omit relocations referencing removed
    symbols, while retaining relocations with no symbol reference.
  - Retain the entire relocation section if it references the dynamic
    symbol table (fix from kaiw in D17596).
  
  PR:		232176
  Sponsored by:	The FreeBSD Foundation

Modified:
  stable/11/contrib/elftoolchain/elfcopy/sections.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/contrib/elftoolchain/elfcopy/sections.c
==============================================================================
--- stable/11/contrib/elftoolchain/elfcopy/sections.c	Thu Oct 25 13:37:57 2018	(r339709)
+++ stable/11/contrib/elftoolchain/elfcopy/sections.c	Thu Oct 25 13:46:28 2018	(r339710)
@@ -689,7 +689,7 @@ filter_reloc(struct elfcopy *ecp, struct section *s)
 	Elf32_Rela	*rela32;
 	Elf64_Rela	*rela64;
 	Elf_Data	*id;
-	uint64_t	 cap, n, nrels;
+	uint64_t	 cap, n, nrels, sym;
 	int		 elferr, i;
 
 	if (gelf_getshdr(s->is, &ish) == NULL)
@@ -698,15 +698,13 @@ filter_reloc(struct elfcopy *ecp, struct section *s)
 
 	/* We don't want to touch relocation info for dynamic symbols. */
 	if ((ecp->flags & SYMTAB_EXIST) == 0) {
-		if (ish.sh_link == 0 || ecp->secndx[ish.sh_link] == 0) {
-			/*
-			 * This reloc section applies to the symbol table
-			 * that was stripped, so discard whole section.
-			 */
-			s->nocopy = 1;
-			s->sz = 0;
-		}
-		return;
+		/*
+		 * No symbol table in output.  If sh_link points to a section
+		 * that exists in the output object, this relocation section
+		 * is for dynamic symbols.  Don't touch it.
+		 */
+		if (ish.sh_link != 0 && ecp->secndx[ish.sh_link] != 0)
+			return;
 	} else {
 		/* Symbol table exist, check if index equals. */
 		if (ish.sh_link != elf_ndxscn(ecp->symtab->is))
@@ -747,28 +745,45 @@ filter_reloc(struct elfcopy *ecp, struct section *s)
 			if (gelf_getrel(id, i, &rel) != &rel)
 				errx(EXIT_FAILURE, "gelf_getrel failed: %s",
 				    elf_errmsg(-1));
+			sym = GELF_R_SYM(rel.r_info);
 		} else {
 			if (gelf_getrela(id, i, &rela) != &rela)
 				errx(EXIT_FAILURE, "gelf_getrel failed: %s",
 				    elf_errmsg(-1));
+			sym = GELF_R_SYM(rela.r_info);
 		}
-		name = elf_strptr(ecp->ein, elf_ndxscn(ecp->strtab->is),
-		    GELF_R_SYM(rel.r_info));
-		if (name == NULL)
-			errx(EXIT_FAILURE, "elf_strptr failed: %s",
-			    elf_errmsg(-1));
-		if (lookup_symop_list(ecp, name, SYMOP_KEEP) != NULL) {
-			if (ecp->oec == ELFCLASS32) {
-				if (s->type == SHT_REL)
-					COPYREL(rel, 32);
-				else
-					COPYREL(rela, 32);
-			} else {
-				if (s->type == SHT_REL)
-					COPYREL(rel, 64);
-				else
-					COPYREL(rela, 64);
-			}
+		/*
+		 * If a relocation references a symbol and we are omitting
+		 * either that symbol or the entire symbol table we cannot
+		 * produce valid output, and so just omit the relocation.
+		 * Broken output like this is generally not useful, but some
+		 * uses of elfcopy/strip rely on it - for example, GCC's build
+		 * process uses it to check for build reproducibility by
+		 * stripping objects and comparing them.
+		 *
+		 * Relocations that do not reference a symbol are retained.
+		 */
+		if (sym != 0) {
+			if (ish.sh_link == 0 || ecp->secndx[ish.sh_link] == 0)
+				continue;
+			name = elf_strptr(ecp->ein, elf_ndxscn(ecp->strtab->is),
+			    sym);
+			if (name == NULL)
+				errx(EXIT_FAILURE, "elf_strptr failed: %s",
+				    elf_errmsg(-1));
+			if (lookup_symop_list(ecp, name, SYMOP_KEEP) == NULL)
+				continue;
+		}
+		if (ecp->oec == ELFCLASS32) {
+			if (s->type == SHT_REL)
+				COPYREL(rel, 32);
+			else
+				COPYREL(rela, 32);
+		} else {
+			if (s->type == SHT_REL)
+				COPYREL(rel, 64);
+			else
+				COPYREL(rela, 64);
 		}
 	}
 	elferr = elf_errno();