From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb33.google.com (mail-yb1-xb33.google.com [IPv6:2607:f8b0:4864:20::b33]) by sourceware.org (Postfix) with ESMTPS id B9E07385C32E for ; Fri, 24 Jun 2022 05:29:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B9E07385C32E Received: by mail-yb1-xb33.google.com with SMTP id i7so2679757ybe.11 for ; Thu, 23 Jun 2022 22:29:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=WWgF1akUCANUxWv7g748mAyRjqOL2TaQPQkCjWqJve8=; b=ACx3JUbOAA/MwLCcch5jayEgX38ijviUXtp37YHaiFzR6ugX4GdBTNa+rk8nlTp4x9 fxy8qC+YYMXJK5JNZIP/DnJ2gs3uef8qfKsXbmhBNfTM6GT7a3U0IpdAPZHXwPFIfe7V exfZPAgLyhKb571OTSe2NaqYRIT0l1qyfc9ewMMX6DJQ4YQqxW7Xns7dJdoyjz78jZQ7 Eo9odz5sBhJ3mvBuREK6YKaqJCXWY8Yh9MC/IAKGj2fKwptT9zjZY4VjXv5qCTbiQF2e VGmhIa1Zq55zS0DVSHY1nRVXOqDADgw2h/0N2JOrs3sU7X1Mqe2D9Z/e7WkooBwzJlPE 5kuA== X-Gm-Message-State: AJIora+yXwJ470MpjMOnSML6dU12BFvP+YeQAv6kd3lYfa8OSwJaGc6H fKR6pMOC3I5pxtTfWy6fSyJ+mtdfA7HtlrkjUKY2K2WPVKa6Qg== X-Google-Smtp-Source: AGRyM1sQRnC3FgyvF/lf2BFzKRffZULVQSsqK81Lx3yaXwvlIVv8INylAeJ4oLE8HYmUGMhmI8SfTfBVoTOS5lWhuNw= X-Received: by 2002:a05:6902:14e:b0:64f:d2eb:2df0 with SMTP id p14-20020a056902014e00b0064fd2eb2df0mr13674101ybh.557.1656048548038; Thu, 23 Jun 2022 22:29:08 -0700 (PDT) MIME-Version: 1.0 References: <20220623151353.62139-1-ishitatsuyuki@gmail.com> <20220623151353.62139-4-ishitatsuyuki@gmail.com> <48fc0f61-7c17-cd94-be02-31b83dd386ff@suse.com> In-Reply-To: <48fc0f61-7c17-cd94-be02-31b83dd386ff@suse.com> From: Tatsuyuki Ishi Date: Fri, 24 Jun 2022 14:28:57 +0900 Message-ID: Subject: Re: [PATCH v2 3/6] objcopy: Remove SHT_LLVM_ADDRSIG sections by default. To: Jan Beulich Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jun 2022 05:29:10 -0000 Hi Jan, thanks again for your review. > > --- a/binutils/objcopy.c > > +++ b/binutils/objcopy.c > > @@ -1316,10 +1316,20 @@ is_mergeable_note_section (bfd * abfd, asection * sec) > > return false; > > } > > > > +static bool > > +is_addrsig_section (bfd *abfd, asection *sec) > > +{ > > + if (bfd_get_flavour (abfd) == bfd_target_elf_flavour > > + && elf_section_data (sec)->this_hdr.sh_type == SHT_LLVM_ADDRSIG) > > + return true; > > + > > + return false; > > +} > > Not sure if this would conflict with unwritten style expectations, > but when I see such I always wonder: Why not a simple return > statement, without explicit use of true/false (or 1/0)? I just blindly copied the style of is_mergeable_note_section above, but I think your suggestion also makes sense. If there's no objections I will turn that into a direct return in the next revision. > > @@ -1348,6 +1358,20 @@ is_strip_section_1 (bfd *abfd ATTRIBUTE_UNUSED, asection *sec) > > return true; > > } > > > > + /* addrsig needs to be updated if the symtab is altered, but we don't know > > + how to read nor write it, so just throw it away to not confuse > > + downstream tools. */ > > + if (is_addrsig_section (abfd, sec)) > > + { > > + non_fatal (_ ("warning: removing section %s to prevent corrupt addrsig " > > + "information"), > > + bfd_section_name (sec)); > > + non_fatal (_ ("pass --remove-section=%1$s to suppress warning, or " > > + "--keep-section=%1$s to override"), > > + bfd_section_name (sec)); > > + return true; > > + } > > Isn't this too aggressive? The comment says "if the symtab is altered", > but the code looks to request stripping unconditionally. Sorry, the way the comment is written is indeed quite confusing, so I can update that. But there's probably not too much value in checking whether symtab is altered here, since it has been thrown away (unconditionally) by LLD from the sh_link heuristic in prior versions of binutils anyway. Tatsuyuki