From owner-svn-src-stable-11@freebsd.org Thu Oct 25 13:46:29 2018 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 21B521076DCF; Thu, 25 Oct 2018 13:46:29 +0000 (UTC) (envelope-from emaste@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C8B7A8318D; Thu, 25 Oct 2018 13:46:28 +0000 (UTC) (envelope-from emaste@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A98AB129B3; Thu, 25 Oct 2018 13:46:28 +0000 (UTC) (envelope-from emaste@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w9PDkSsB021537; Thu, 25 Oct 2018 13:46:28 GMT (envelope-from emaste@FreeBSD.org) Received: (from emaste@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w9PDkSgf021536; Thu, 25 Oct 2018 13:46:28 GMT (envelope-from emaste@FreeBSD.org) Message-Id: <201810251346.w9PDkSgf021536@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: emaste set sender to emaste@FreeBSD.org using -f From: Ed Maste Date: Thu, 25 Oct 2018 13:46:28 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r339710 - stable/11/contrib/elftoolchain/elfcopy X-SVN-Group: stable-11 X-SVN-Commit-Author: emaste X-SVN-Commit-Paths: stable/11/contrib/elftoolchain/elfcopy X-SVN-Commit-Revision: 339710 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Oct 2018 13:46:29 -0000 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();