Message ID | 20241105225338.1279732-1-amerey@redhat.com |
---|---|
State | Committed |
Headers |
Return-Path: <elfutils-devel-bounces~patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 795913858D35 for <patchwork@sourceware.org>; Tue, 5 Nov 2024 22:53:55 +0000 (GMT) X-Original-To: elfutils-devel@sourceware.org Delivered-To: elfutils-devel@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 5E8023858D20 for <elfutils-devel@sourceware.org>; Tue, 5 Nov 2024 22:53:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5E8023858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5E8023858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730847225; cv=none; b=ndOPzTExHUdd+rwmiiA6fiE9RidaQKpZmLy7F7wXc2/dh50uQBMozU3tYPI6/BJfFFsvW9mObDKvtN4rBLxnkR5/9yd4KuY52YN1GE0A2hDhgX1dBsfqYsB4UV29pdoCDAcf6UXjJb9sgjgJxPWqG6kZ/P/tWdkDaZtyhVRFQqU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730847225; c=relaxed/simple; bh=4Vw1KN16zQaE4Yk5q+NBGkr6QjwpNtqbNRT//iAV5gQ=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=eFpBPxrp3s/+0hJHcCacG/6V/9EszXwIcyATO5Q/FHNft0y2jd0+HnmPMyOKk+hG/CCPeRfjk8KoaQUOVxTECsthn5Kz9nXaVRYAjsMimzMKsdBUN668MIDYsm6yHXw1MqBsMMaqVaL9UUYdCpERmeUZ7Md+GvqUu+g3s9NuUjk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730847223; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=SH0g1+9wcPE7O3OD1kJ/zxgeaRV1hnDx+l+vJwO0YyU=; b=MpfArGDzLyGEFlfx+1e4gWbdsyOoeBTqRgma/M7KT2VLNxsC6wU1rmuKLk1UgTOIYh/3L+ XHV1vejqU8lIvhm9imtIkTzp3REAhpkdBwMglwQTKupuRXnifxuiuo9Wx8+nJpAKaMyRCr WcnbDOW7sXBTaAsuJltdSqUHhro9GZU= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-67-yWc-duLSPdmtfd6055-GJw-1; Tue, 05 Nov 2024 17:53:41 -0500 X-MC-Unique: yWc-duLSPdmtfd6055-GJw-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id E17A01956048 for <elfutils-devel@sourceware.org>; Tue, 5 Nov 2024 22:53:40 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.22.64.103]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 481A8195607C; Tue, 5 Nov 2024 22:53:40 +0000 (UTC) From: Aaron Merey <amerey@redhat.com> To: elfutils-devel@sourceware.org Cc: Aaron Merey <amerey@redhat.com> Subject: [PATCH] strip: Ignore --reloc-debug-sections-only for non-ET_REL files. Date: Tue, 5 Nov 2024 17:53:38 -0500 Message-ID: <20241105225338.1279732-1-amerey@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Elfutils-devel mailing list <elfutils-devel.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/elfutils-devel>, <mailto:elfutils-devel-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/elfutils-devel/> List-Post: <mailto:elfutils-devel@sourceware.org> List-Help: <mailto:elfutils-devel-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/elfutils-devel>, <mailto:elfutils-devel-request@sourceware.org?subject=subscribe> Errors-To: elfutils-devel-bounces~patchwork=sourceware.org@sourceware.org |
Series |
strip: Ignore --reloc-debug-sections-only for non-ET_REL files.
|
|
Commit Message
Aaron Merey
Nov. 5, 2024, 10:53 p.m. UTC
strip --reloc-debug-sections-only is expected to be a no-op for
non-ET_REL files. This was not enforced in the code. Sections
were copied over to a new output file and normally its contents
would be identical to the input file.
However the output file is not identical to a non-ET_REL input
file if the linker organized sections such that the indices of
SHF_ALLOC sections are not in a contigous group.
In this case strip will modify sections in order to keep all SHF_ALLOC
sections in a contiguous group.
Fix this by ignoring --reloc-debug-sections-only for non-ET_REL files.
https://sourceware.org/bugzilla/show_bug.cgi?id=32253
Signed-off-by: Aaron Merey <amerey@redhat.com>
---
src/strip.c | 7 +++++++
1 file changed, 7 insertions(+)
Comments
Hi Aaron, On Tue, Nov 05, 2024 at 05:53:38PM -0500, Aaron Merey wrote: > strip --reloc-debug-sections-only is expected to be a no-op for > non-ET_REL files. This was not enforced in the code. Sections > were copied over to a new output file and normally its contents > would be identical to the input file. > > However the output file is not identical to a non-ET_REL input > file if the linker organized sections such that the indices of > SHF_ALLOC sections are not in a contigous group. > > In this case strip will modify sections in order to keep all SHF_ALLOC > sections in a contiguous group. > > Fix this by ignoring --reloc-debug-sections-only for non-ET_REL files. > > https://sourceware.org/bugzilla/show_bug.cgi?id=32253 > > Signed-off-by: Aaron Merey <amerey@redhat.com> > --- > src/strip.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/strip.c b/src/strip.c > index 403e0f6f..3812fb17 100644 > --- a/src/strip.c > +++ b/src/strip.c > @@ -1139,6 +1139,13 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, > > if (reloc_debug_only) > { > + if (ehdr->e_type != ET_REL) > + { > + /* Only ET_REL files should have debug relocations to remove. */ > + error (0, 0, _("Ignoring --reloc-debug-sections-only for " \ > + "non-ET_REL file '%s'"), fname); > + goto fail_close; > + } Do we have to fail here? I think it is nicer for the user to just turn this into an warning with if (ehdr->e_type != ET_REL) else if (handle_debug_relocs (...)) > if (handle_debug_relocs (elf, ebl, newelf, ehdr, fname, shstrndx, > &lastsec_offset, &lastsec_size) != 0) > { Cheers, Mark
Hi Mark, On Wed, Nov 6, 2024 at 2:35 PM Mark Wielaard <mark@klomp.org> wrote: > > if (reloc_debug_only) > > { > > + if (ehdr->e_type != ET_REL) > > + { > > + /* Only ET_REL files should have debug relocations to remove. */ > > + error (0, 0, _("Ignoring --reloc-debug-sections-only for " \ > > + "non-ET_REL file '%s'"), fname); > > + goto fail_close; > > + } > > Do we have to fail here? I think it is nicer for the user to just > turn this into an warning with > > if (ehdr->e_type != ET_REL) else if (handle_debug_relocs (...)) If we don't fail then strip will need to do more work copying sections over to the 'newelf' handle in order to properly overwrite the input file with newelf. Since --reloc-debug-sections-only should be a no-op for non-ET_REL files it makes sense to skip that extra work and avoid overwriting the input file. Aaron
Hi Aaron, On Wed, Nov 06, 2024 at 04:15:30PM -0500, Aaron Merey wrote: > On Wed, Nov 6, 2024 at 2:35 PM Mark Wielaard <mark@klomp.org> wrote: > > > if (reloc_debug_only) > > > { > > > + if (ehdr->e_type != ET_REL) > > > + { > > > + /* Only ET_REL files should have debug relocations to remove. */ > > > + error (0, 0, _("Ignoring --reloc-debug-sections-only for " \ > > > + "non-ET_REL file '%s'"), fname); > > > + goto fail_close; > > > + } > > > > Do we have to fail here? I think it is nicer for the user to just > > turn this into an warning with > > > > if (ehdr->e_type != ET_REL) else if (handle_debug_relocs (...)) > > If we don't fail then strip will need to do more work copying sections > over to the 'newelf' handle in order to properly overwrite the input file > with newelf. > > Since --reloc-debug-sections-only should be a no-op for non-ET_REL files > it makes sense to skip that extra work and avoid overwriting the input file. Aha, so by not goto done you skip the actual (supposed to do nothing) work. goto fail_close isn't so much "failing", but more about closing without doing any more work. Which is fine because when --reloc-debug-sections-only "No other stripping is performed". OK. Sorry for being dense. Cheers, Mark
On Wed, Nov 6, 2024 at 6:32 PM Mark Wielaard <mark@klomp.org> wrote: > > Hi Aaron, > > On Wed, Nov 06, 2024 at 04:15:30PM -0500, Aaron Merey wrote: > > On Wed, Nov 6, 2024 at 2:35 PM Mark Wielaard <mark@klomp.org> wrote: > > > > if (reloc_debug_only) > > > > { > > > > + if (ehdr->e_type != ET_REL) > > > > + { > > > > + /* Only ET_REL files should have debug relocations to remove. */ > > > > + error (0, 0, _("Ignoring --reloc-debug-sections-only for " \ > > > > + "non-ET_REL file '%s'"), fname); > > > > + goto fail_close; > > > > + } > > > > > > Do we have to fail here? I think it is nicer for the user to just > > > turn this into an warning with > > > > > > if (ehdr->e_type != ET_REL) else if (handle_debug_relocs (...)) > > > > If we don't fail then strip will need to do more work copying sections > > over to the 'newelf' handle in order to properly overwrite the input file > > with newelf. > > > > Since --reloc-debug-sections-only should be a no-op for non-ET_REL files > > it makes sense to skip that extra work and avoid overwriting the input file. > > Aha, so by not goto done you skip the actual (supposed to do nothing) > work. goto fail_close isn't so much "failing", but more about closing > without doing any more work. Which is fine because when > --reloc-debug-sections-only "No other stripping is performed". > > OK. Sorry for being dense. Thanks Mark, pushed. Aaron
diff --git a/src/strip.c b/src/strip.c index 403e0f6f..3812fb17 100644 --- a/src/strip.c +++ b/src/strip.c @@ -1139,6 +1139,13 @@ handle_elf (int fd, Elf *elf, const char *prefix, const char *fname, if (reloc_debug_only) { + if (ehdr->e_type != ET_REL) + { + /* Only ET_REL files should have debug relocations to remove. */ + error (0, 0, _("Ignoring --reloc-debug-sections-only for " \ + "non-ET_REL file '%s'"), fname); + goto fail_close; + } if (handle_debug_relocs (elf, ebl, newelf, ehdr, fname, shstrndx, &lastsec_offset, &lastsec_size) != 0) {