From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123363 invoked by alias); 7 Feb 2020 02:23:35 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 123300 invoked by uid 89); 7 Feb 2020 02:23:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=93195, Multiple, group's X-HELO: mail-pg1-f196.google.com Received: from mail-pg1-f196.google.com (HELO mail-pg1-f196.google.com) (209.85.215.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Feb 2020 02:23:32 +0000 Received: by mail-pg1-f196.google.com with SMTP id w21so301040pgl.9 for ; Thu, 06 Feb 2020 18:23:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=WtPda6eBLWXfw2Y8eg7A/iVht8ZQo+O/WLLTtsaGuCM=; b=XbLvqmc+fH1SYYiSlwww7WVA5JtIGn/eFoXb8zF2qeRzJdW3x1FNn30dPTsF/SwAlw lG1TwenSzThFf5JlQA8hr6mSqfHa311VtkHbOSA6F9SaWxl9oLSMnvCPfHeSDn1PLwzG fJSrn7IWpxkNcWphJfiuOIkclrhSgQ3vFbv0DkQ4m0s1X3xpBg+a87Oe6vs7UoJGiIGO 4QLzwTZRzZkuY5wPFuMxXp+lUuGaMjuRnaiCBDe6kpKEVrqlhh3hA5MXMcaixPyNzoSE 26waCwdQolvNKSjLpZffBNFRvqYcnHCXH2SE7P5wK3e2AEfR8X7GiWNrNlmMxYZIpeYb jTYA== Return-Path: Received: from gnu-cfl-2.localdomain (c-73-93-86-59.hsd1.ca.comcast.net. [73.93.86.59]) by smtp.gmail.com with ESMTPSA id u7sm677181pfh.128.2020.02.06.18.23.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2020 18:23:30 -0800 (PST) Received: by gnu-cfl-2.localdomain (Postfix, from userid 1000) id A67DDC0372; Thu, 6 Feb 2020 18:23:28 -0800 (PST) Date: Fri, 07 Feb 2020 02:23:00 -0000 From: "H.J. Lu" To: Fangrui Song , Binutils , Szabolcs Nagy Subject: [PATCH] ld: Issue an error for GC on __patchable_function_entries section Message-ID: <20200207022328.GA444595@gmail.com> References: <20200201162613.2023476-1-hjl.tools@gmail.com> <20200201173429.ep5siwkbhz5osehk@google.com> <20200201182148.722e4cc57xy2vyxb@gmail.com> <20200203005722.q7dyk6bajvxfdunp@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00107.txt.bz2 On Sun, Feb 02, 2020 at 05:17:48PM -0800, H.J. Lu wrote: > On Sun, Feb 2, 2020 at 4:57 PM Fangrui Song wrote: > > > > On 2020-02-02, H.J. Lu wrote: > > >On Sat, Feb 1, 2020 at 10:21 AM Fangrui Song wrote: > > >> > > >> On 2020-02-01, H.J. Lu wrote: > > >> >On Sat, Feb 1, 2020 at 9:34 AM Fangrui Song wrote: > > >> >> > > >> >> On 2020-02-01, H.J. Lu wrote: > > >> >> >After all text sections have been garbage collected, if a > > >> >> >__patchable_function_entries section references a section which > > >> >> >wasn't marked, mark it with SEC_EXCLUDE and return NULL. Otherwise, > > >> >> >keep it. > > >> >> > > > >> >> >Should it be handled in _bfd_elf_gc_mark_extra_sections? > > >> >> > > >> >> Thanks for paying attention to these feature requests. > > >> >> > > >> >> I referenced GNU as and ld requests at > > >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 > > >> >> If we > > >> >> > > >> >> * implement SHF_LINK_ORDER > > >> > > > >> >I am not sure if overloading (abusing?) SHF_LINK_ORDER is a good idea. > > >> > > >> It is an extension, but it is agreed by multiple parties on > > >> https://groups.google.com/d/msg/generic-abi/_CbBM6T6WeM/eGF9A0AnAAAJ , > > >> including HP-UX and Solaris developers. > > >> > > >> >> * allow multiple sections with the same name ("unique") > > >> > > > >> >This is orthogonal to this. I have a question on assembly syntax: > > >> > > > >> >https://sourceware.org/bugzilla/show_bug.cgi?id=25380#c1 > > >> > > > >> >> * teach GCC to use SHF_LINK_ORDER and "unique" (see https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html) > > >> >> > > >> >> An ad-hoc gc marking will be unnecessary. > > >> > > > >> >We need to scan relocations in _patchable_function_entries section for > > >> >references to garbage collected sections. We can either check section > > >> >name or a SHF_XXX. But I don't know if SHF_LINK_ORDER is a good > > >> >approach. > > >> > > >> We don't need to if we use multiple __patchable_function_entries > > >> sections. Multiple sections are a must due to > > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195#c1 (COMDAT) > > >> > > >> > A symbol table entry with STB_LOCAL binding that is defined relative > > >> > to one of a group's sections, and that is contained in a symbol table > > >> > section that is not part of the group, must be discarded if the group > > >> > members are discarded. References to this symbol table entry from > > >> > outside the group are not allowed. > > >> > > >> The __patchable_function_entries creation logic I implemented in clang: > > >> > > >> foreach function foo > > >> find the first function label defined in foo's section, name it $associated > > >> ($associated can have 2 reasonable values, w/ or w/o -ffunction-sections) > > >> get or create an id for $associated, say, $unique > > >> > > >> if foo is in a COMDAT named $comdat > > >> .section __patchable_function_entries,"awo",@progbits,$associated,comdat,$comdat,unique,$unique > > >> else > > >> .section __patchable_function_entries,"awo",@progbits,$associated,unique,$unique > > >> > > >> This approach uses feature requests I referenced in *direct* links of > > >> https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html plus > > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492#c2 , > > >> and addresses all bugs I filed. > > >> > > > > > >Here is a linker patch to issue an error to avoid corrupt > > >linker output. > > > > > >-- > > >H.J. > > > > A typo in the description. > > > > - .section __patchable_function_entries,"awo",%progbits,_start > > + .section __patchable_function_entries,"aw",%progbits > > > > Fixed. > Now, gas supports the section flag 'o' in .section directive. I will submit a GCC patch to use it. Here is the updated patch to issue an error for garbage collection on __patchable_function_entries section without linked-to section. OK for master? Thanks. H.J. --- __patchable_function_entries section is generated by a compiler with -fpatchable-function-entry=XX. The assembly code looks like this: --- .text .globl _start .type _start, %function _start: .section __patchable_function_entries,"aw",%progbits .dc.a .LPFE1 .text .LPFE1: .byte 0 --- But --gc-sections will silently remove __patchable_function_entries section and generate corrupt result. This patch disallows garbage collection on __patchable_function_entries section without linked-to section. bfd/ PR ld/25490 * elflink.c (_bfd_elf_gc_mark_extra_sections): Issue an error for garbage collection on __patchable_function_entries section without linked-to section. ld/ PR ld/25490 * testsuite/ld-elf/pr25490-1.d: New file. * testsuite/ld-elf/pr25490-1.s: Likewise. --- bfd/elflink.c | 7 +++++++ ld/testsuite/ld-elf/pr25490-1.d | 3 +++ ld/testsuite/ld-elf/pr25490-1.s | 9 +++++++++ 3 files changed, 19 insertions(+) create mode 100644 ld/testsuite/ld-elf/pr25490-1.d create mode 100644 ld/testsuite/ld-elf/pr25490-1.s diff --git a/bfd/elflink.c b/bfd/elflink.c index 30a572d32de..3add9f18bd7 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -13365,6 +13365,13 @@ _bfd_elf_gc_mark_extra_sections (struct bfd_link_info *info, && (isec->flags & SEC_DEBUGGING) && CONST_STRNEQ (isec->name, ".debug_line.")) debug_frag_seen = TRUE; + else if (strcmp (bfd_section_name (isec), + "__patchable_function_entries") == 0 + && elf_linked_to_section (isec) == NULL) + info->callbacks->einfo (_("%F%P: %pB(%pA): error: " + "need linked-to section " + "for --gc-sections\n"), + isec->owner, isec); } /* If no non-note alloc section in this file will be kept, then diff --git a/ld/testsuite/ld-elf/pr25490-1.d b/ld/testsuite/ld-elf/pr25490-1.d new file mode 100644 index 00000000000..3f582645899 --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.d @@ -0,0 +1,3 @@ +#ld: --gc-sections -e _start +#target: [check_gc_sections_available] +#error: .*\(__patchable_function_entries\): error: need linked-to section for --gc-sections diff --git a/ld/testsuite/ld-elf/pr25490-1.s b/ld/testsuite/ld-elf/pr25490-1.s new file mode 100644 index 00000000000..51ba1ea8014 --- /dev/null +++ b/ld/testsuite/ld-elf/pr25490-1.s @@ -0,0 +1,9 @@ + .text + .globl _start + .type _start, %function +_start: + .section __patchable_function_entries,"aw",%progbits + .dc.a .LPFE1 + .text +.LPFE1: + .byte 0 -- 2.24.1