Message ID | 20221127023840.32080-1-mark@harmstone.com |
---|---|
State | Accepted |
Headers |
Return-Path: <binutils-bounces+ouuuleilei=gmail.com@sourceware.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp4920472wrr; Sat, 26 Nov 2022 18:38:55 -0800 (PST) X-Google-Smtp-Source: AA0mqf4Q86kH58agfdxdmMP6o8/o0XGuvgWKoxGQT4t406M/l01PwCwPGQr06jTOXj2iJHh3Ym4m X-Received: by 2002:a17:906:1c86:b0:7bd:ae0f:5d4 with SMTP id g6-20020a1709061c8600b007bdae0f05d4mr4846022ejh.431.1669516735713; Sat, 26 Nov 2022 18:38:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669516735; cv=none; d=google.com; s=arc-20160816; b=WsDJ2vyPKRAmoUO3ldEHGA1TsSK044+0Xsi+16SS3Bb7Mo2aB0IYpGDRXAAfe5s6G8 89YkyO+eGR9v63WjWd4QISNXuYOZ0sBwMUVG7Kka6F26+3GqF/cpKBOmj2BVTsNG3mx8 anpdYZ1iuvdxjiMr4UbBDlgEiSGsmt9oZylncEQeYDajnDAp2nI1Q217+EzKci1V9JGC tqENq7vapSJg9IhfOnhOrBMqtHmccdm3XZY19pcDyi4YM0+CDHgJtsAgvoxGvXle8lkb IiFon387cp6teQycPXqmqHLGEZCZjzzLYo9ukQX1HYfvaQsbt4mnJ1aBdZQO2MtQH08N fdgg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature:dmarc-filter:delivered-to; bh=vim1+BrNodFuu0SYc69n2WJlKzRMEJVNZxmb+fj91Yo=; b=U8IYpADDkWtWzslayGYfrLBgFnG8RAJmnIOV6ia5ZuULyPm/ZKqSBjyv1bemq//Eum hbFr2skwoj/BCRU8RX2oDXws4fQ3PVy0D4Zcp28N0ItESpbRMu0MpQAm9HhLeEfVxAbB qYllpkqtFrgdWQxcI/4zHCXckeAhk/Rz7mR8jIuhQJSCRNNmWrKK9g4txAB+svGE4ZaZ KobluHxzu97lAQv3dsAazTPktYscatFwruAWrCfh/T6X6Q5HriMPfh54bNib3i54axTi EtGQl7gZJC12dl+wzhNKYojXnptJjyMlF/MJeRJmeAmRLHmHjE+EGn73vJQds3nfeSIs 4nTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20210112 header.b=P56zMBYU; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org" Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id j20-20020a05640211d400b00469634340b4si6217999edw.285.2022.11.26.18.38.55 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Nov 2022 18:38:55 -0800 (PST) Received-SPF: pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=fail header.i=@gmail.com header.s=20210112 header.b=P56zMBYU; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org" Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1E8AE385B18F for <ouuuleilei@gmail.com>; Sun, 27 Nov 2022 02:38:54 +0000 (GMT) X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id 92D063858C62 for <binutils@sourceware.org>; Sun, 27 Nov 2022 02:38:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 92D063858C62 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=harmstone.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-x331.google.com with SMTP id o30so6152979wms.2 for <binutils@sourceware.org>; Sat, 26 Nov 2022 18:38:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:sender:from:to:cc:subject:date :message-id:reply-to; bh=vim1+BrNodFuu0SYc69n2WJlKzRMEJVNZxmb+fj91Yo=; b=P56zMBYU6A9LG/w5+8HjKTQywrC+6TyvniCkpw7SAFp2JaLP43z+XNxpMNurdrjMfW SVupCFmHlyi2M846MnocaL/eaBZS1D6vK+tA/Z0wTweSi+0Ft5fg6R8/u1iXWs/DK/Fh 23E8ikx1SSa+WJd0NdBi/yZ4CX+qUbaCCS7ybQyjVVzzIBy3xR//nZif69w2Fjr6ln3q DCXMTxZ/EpzeSkmdKX0mqZ2PXGrG8YD0zjeZDH6MWaPta8imEoyZcammJlSEd1XNB4O6 cVDq0GJAS3FywWvUA2PFxXNH9cm6sfQnCI6kYt8X6QTlrR8G6vL7MmdsIkcI9MLN7IE3 3URg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:sender:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=vim1+BrNodFuu0SYc69n2WJlKzRMEJVNZxmb+fj91Yo=; b=pVTg8i7GERoX+YX6b4WgqTtd2Dv4lWbjx/So2peexfjth38bxIeC8gr4QIy0gP3BE9 +T/f/Fp1AJvCMVOE9+DcwZ/lPyoF2L7W7FdzkdXolPBlqY9EEbqsDNCTJNwMR+ydB+4u +43xNYOJc1RwrOYZ0BXn7y7GCaDk2hWQ4UFHe9yxFViUrkyjrcdvU6hDva8UsEKu693u n9On92KD9YaFZsxKl89kERW3kbq+CXbonQp8Fi1+YMTKCGsir7ixoVwLDwYlVBK18VZg gtijDYqi4AURDNJwUZNtyt57p6PqddUK7iP889JxxmAejslHbFsplfm90XfZieMO62C4 kBVg== X-Gm-Message-State: ANoB5pl0/prj02EGZBZUnoBDFeRErXbEzSppq+Gr4H8GvNtLkmlQWu2o ZKRq3vmwQcQfMHuT6J+idMPvg5hGK8M= X-Received: by 2002:a7b:c045:0:b0:3cf:6f5f:da0e with SMTP id u5-20020a7bc045000000b003cf6f5fda0emr19662351wmc.19.1669516726050; Sat, 26 Nov 2022 18:38:46 -0800 (PST) Received: from beren.harmstone.com ([2a02:8010:64ea:0:8eb8:7eff:fe53:9d5f]) by smtp.gmail.com with ESMTPSA id l35-20020a05600c1d2300b003cf878c4468sm15890384wms.5.2022.11.26.18.38.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Nov 2022 18:38:45 -0800 (PST) From: Mark Harmstone <mark@harmstone.com> To: binutils@sourceware.org Cc: Mark Harmstone <mark@harmstone.com> Subject: [PATCH] ld: Fix segfault in populate_publics_stream Date: Sun, 27 Nov 2022 02:38:39 +0000 Message-Id: <20221127023840.32080-1-mark@harmstone.com> X-Mailer: git-send-email 2.37.4 In-Reply-To: <20221125025433.26818-1-mark@harmstone.com> References: <20221125025433.26818-1-mark@harmstone.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list <binutils.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/binutils>, <mailto:binutils-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/binutils/> List-Post: <mailto:binutils@sourceware.org> List-Help: <mailto:binutils-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/binutils>, <mailto:binutils-request@sourceware.org?subject=subscribe> Errors-To: binutils-bounces+ouuuleilei=gmail.com@sourceware.org Sender: "Binutils" <binutils-bounces+ouuuleilei=gmail.com@sourceware.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750615180740482907?= X-GMAIL-MSGID: =?utf-8?q?1750615180740482907?= |
Series |
ld: Fix segfault in populate_publics_stream
|
|
Checks
Context | Check | Description |
---|---|---|
snail/binutils-gdb-check | success | Github commit url |
Commit Message
Mark Harmstone
Nov. 27, 2022, 2:38 a.m. UTC
--- ld/pdb.c | 3 +++ 1 file changed, 3 insertions(+)
Comments
On 27.11.2022 03:38, Mark Harmstone wrote: > --- a/ld/pdb.c > +++ b/ld/pdb.c > @@ -1413,6 +1413,9 @@ populate_publics_stream (bfd *stream, bfd *abfd, bfd *sym_rec_stream) Out of curiosity - which tree was this diff generated against? The line number here looks to be off by several hundred from what I see in the repo right now. > for (bfd *in = coff_data (abfd)->link_info->input_bfds; in; > in = in->link.next) > { > + if (!in->outsymbols) > + continue; > + > for (unsigned int i = 0; i < in->symcount; i++) > { > struct bfd_symbol *sym = in->outsymbols[i]; Why / when would in->outsymbols be NULL but in->symcount be non-zero? And if that was possible, why would it not also be possible that the array is smaller than in->symcount? (This is the kind of questions which arise when there's no description at all for a patch. Such a description could have clarified under what special conditions a NULL deref could happen despite it not being obviously possible.) Jan
> Out of curiosity - which tree was this diff generated against? The > line number here looks to be off by several hundred from what I > see in the repo right now. This is inteded to be applied with the other patches in the series, the ones beginning with "[PATCH v2] ld: Generate PDB string table" at https://sourceware.org/pipermail/binutils/2022-November/thread.html. Sorry, this probably wasn't obvious from a mail client. I've not numbered them as I'm not sure how many more there'll be, and if I wait for the previous patches to be accepted before submitting the next, I'll almost certainly miss the cut-off for the code freeze. > Why / when would in->outsymbols be NULL but in->symcount be non-zero? Try running the test in the DEBUG_S_LINES patch without this one - it'll fail because ld segfaults. outsymbols doesn't get set from within generate_reloc for the second object file, as it only has one non-loadable section. The "symbols" come from the .equs I'm using like #defines. > And if that was possible, why would it not also be possible that the > array is smaller than in->symcount? bfd_generic_link_read_symbols is called for each loadable section, and allocates the outsymbols array once. It was my mistake when I submitted my original patch for populate_publics_stream, in not realizing that it would break for object files without any loadable sections. Mark
On 28.11.2022 18:53, Mark Harmstone wrote: > > Out of curiosity - which tree was this diff generated against? The > > line number here looks to be off by several hundred from what I > > see in the repo right now. > > This is inteded to be applied with the other patches in the series, the ones > beginning with "[PATCH v2] ld: Generate PDB string table" at > https://sourceware.org/pipermail/binutils/2022-November/thread.html. > > Sorry, this probably wasn't obvious from a mail client. I've not numbered > them as I'm not sure how many more there'll be, and if I wait for the previous > patches to be accepted before submitting the next, I'll almost certainly miss > the cut-off for the code freeze. Such dependencies, if not otherwise obvious (like in a properly threaded and numbered series) need calling out in a post-commit-message remark. > > Why / when would in->outsymbols be NULL but in->symcount be non-zero? > > Try running the test in the DEBUG_S_LINES patch without this one - it'll fail > because ld segfaults. outsymbols doesn't get set from within generate_reloc > for the second object file, as it only has one non-loadable section. The > "symbols" come from the .equs I'm using like #defines. And then why would such symbols not need emitting debug info for? What you say above and ... > > And if that was possible, why would it not also be possible that the > > array is smaller than in->symcount? > > bfd_generic_link_read_symbols is called for each loadable section, and > allocates the outsymbols array once. It was my mistake when I submitted my > original patch for populate_publics_stream, in not realizing that it would > break for object files without any loadable sections. ... here makes me think that assuming it is the right thing to do, it wants not only properly describing in the patch, but should actually be accompanied by a code comment. Jan
> it ... should actually be accompanied by a code comment.
Not really, as it was a straightforward mistake in my original patch. If I'd
included a NULL check to begin with, I doubt anyone would have batted an
eyelid.
I could have easily worked around this in my test, either by adding in a dummy
section or by using the preprocessor rather than .equ, but figured that any
way of causing a segfault is a bug that needs to be fixed.
Mark
On 29.11.2022 18:47, Mark Harmstone wrote: > > it ... should actually be accompanied by a code comment. > > Not really, as it was a straightforward mistake in my original patch. If I'd > included a NULL check to begin with, I doubt anyone would have batted an > eyelid. Maybe. Depends on how close a review would have been done. > I could have easily worked around this in my test, either by adding in a dummy > section or by using the preprocessor rather than .equ, but figured that any > way of causing a segfault is a bug that needs to be fixed. Of course. Jan
diff --git a/ld/pdb.c b/ld/pdb.c index d133b3e1aaa..42bb1b3a91b 100644 --- a/ld/pdb.c +++ b/ld/pdb.c @@ -1413,6 +1413,9 @@ populate_publics_stream (bfd *stream, bfd *abfd, bfd *sym_rec_stream) for (bfd *in = coff_data (abfd)->link_info->input_bfds; in; in = in->link.next) { + if (!in->outsymbols) + continue; + for (unsigned int i = 0; i < in->symcount; i++) { struct bfd_symbol *sym = in->outsymbols[i];