From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32f.google.com (mail-ot1-x32f.google.com [IPv6:2607:f8b0:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id 385FD3851C21 for ; Wed, 16 Dec 2020 13:36:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 385FD3851C21 Received: by mail-ot1-x32f.google.com with SMTP id w3so22876456otp.13 for ; Wed, 16 Dec 2020 05:36:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5XCpW+qITKG1c6H/1XtFeNT3064DsDqjJHlJ8BJXdws=; b=hvEZZIYjkrvdnjUB7Ck3VWLcO3JRB8KPCzD+LkSipKfLKrsSMaQfNMXSsNP+vsiwof 4RfMYhYG9rpqTvi2+4GRXTTWE+d5c6Ctj/Fc88mRyYV03Xc7m7mclScQnYPUYLlJV/1W aalBqkOH4ljXTA3pHbDxLHcaKZr2EEQYmDU2yCuZclki9IMg7rEn7Aow3ekS1qqlrM/B dn/rjHlukJpXWZJ6oBZqOwlMQaZWszmYvafy8r72xjToypA34Jr4T7yuGGG7yqjXrJrs kE4fUkoXIoTW1eJ7VppKYUDKXdP64HvqL3vGnEwQY1jB9GlLlrC/YJF07CYqVBEN79hf 1tGA== X-Gm-Message-State: AOAM531fgKDTIH8eTX3wPkC+ELQavYvdBp7wzOH439S+nSbIqrsrWb7r ggMLcOzIWxOa3c7XD6JOMqq1tI9oq/mOEVmITtE= X-Google-Smtp-Source: ABdhPJwNWhA2PoqoToFPUuYmLl+pTpn/jPwxHCZugGie51w1Fa8iiTrX9FfPMBeY2hKw/MXmiGtYg93q71zVB9ewY94= X-Received: by 2002:a05:6830:10d2:: with SMTP id z18mr25896801oto.90.1608125800549; Wed, 16 Dec 2020 05:36:40 -0800 (PST) MIME-Version: 1.0 References: <20200207025710.474289-1-hjl.tools@gmail.com> <8193889c-5f8a-62f9-b064-f28ab8aa030e@redhat.com> <20201215204843.GZ3788@tucnak> In-Reply-To: <20201215204843.GZ3788@tucnak> From: "H.J. Lu" Date: Wed, 16 Dec 2020 05:36:04 -0800 Message-ID: Subject: Re: [PATCH] varasm: Fix up __patchable_function_entries handling To: Jakub Jelinek Cc: Jeff Law , GCC Patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3033.6 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 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Dec 2020 13:36:45 -0000 On Wed, Dec 16, 2020 at 5:05 AM Jakub Jelinek wrote: > > On Wed, Dec 02, 2020 at 05:15:21AM -0800, H.J. Lu via Gcc-patches wrote: > > gcc/ > > > > PR middle-end/93195 > > PR middle-end/93197 > > * configure.ac (HAVE_GAS_SECTION_LINK_ORDER): New. Define 1 if > > the assembler supports the section flag 'o' for specifying > > section with link-order. > > * output.h (SECTION_LINK_ORDER): New. Defined to 0x8000000. > > (SECTION_MACH_DEP): Changed from 0x8000000 to 0x10000000. > > * targhooks.c (default_print_patchable_function_entry): Pass > > SECTION_LINK_ORDER to switch_to_section if the section flag 'o' > > works. Pass current_function_decl to switch_to_section. > > * varasm.c (default_elf_asm_named_section): Use 'o' flag for > > SECTION_LINK_ORDER if assembler supports it. > > * config.in: Regenerated. > > * configure: Likewise. > > Dunno if it is an assembler bug or gcc bug, but this SECTION_LINK_ORDER > stuff doesn't seem to work properly. > > If I compile: > static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((patchable_function_entry(0, 0))) int foo (int x) > { > return x + 1; > } > > static inline __attribute__((__gnu_inline__)) __attribute__((__unused__)) __attribute__((patchable_function_entry(0, 0))) int bar (int x) > { > return x + 2; > } > > int > baz (int x) > { > return foo (x) + 1; > } > > int > qux (int x) > { > return bar (x) + 2; > } > (distilled from aarch64 Linux kernel) with > -O2 -fpatchable-function-entry=2 on aarch64 compiler configured against > latest binutils, I get: > ... > .section __patchable_function_entries,"awo",@progbits,baz > ... > .section __patchable_function_entries > ... > in the assembly, but when it is assembled, one gets: > [ 4] __patchable_function_entries PROGBITS 0000000000000000 000060 000008 00 WAL 1 0 8 > [ 5] .rela__patchable_function_entries RELA 0000000000000000 000280 000018 18 I 12 4 8 > [ 6] __patchable_function_entries PROGBITS 0000000000000000 000068 000008 00 0 0 8 > [ 7] .rela__patchable_function_entries RELA 0000000000000000 000298 000018 18 I 12 6 8 > i.e. one writable allocated section with SHF_LINK_ORDER and another > non-allocated non-writable without link order. In the kernel case there is > always one entry in the WAL section and then dozens or more in the > non-allocated one. > The kernel then fails to link: > WARNING: modpost: vmlinux.o (__patchable_function_entries): unexpected non-allocatable section. > Did you forget to use "ax"/"aw" in a .S file? > Note that for example contains > section definitions for use in .S files. > ld: .init.data has both ordered [`__patchable_function_entries' in init/main.o] and unordered [`.init.data' in ./drivers/firmware/efi/libstub/vsprintf.stub.o] sections > ld: final link failed: bad value > make: *** [Makefile:1175: vmlinux] Error 1 > > If it is correct that the assembler requires full section flags for the > SECTION_LINK_ORDER .section directives in every case (like it does for gas is correct. > comdat or for retain), then we should do something like the following > untested change, but if it is gas bug, it should be fixed there. > > 2020-12-15 Jakub Jelinek > > * varasm.c (default_elf_asm_named_section): Always force > section flags even for sections with SECTION_LINK_ORDER flag. > > --- gcc/varasm.c.jj 2020-12-13 17:07:53.910477664 +0100 > +++ gcc/varasm.c 2020-12-15 21:33:35.169314414 +0100 > @@ -6781,10 +6781,10 @@ default_elf_asm_named_section (const cha > > /* If we have already declared this section, we can use an > abbreviated form to switch back to it -- unless this section is > - part of a COMDAT groups or with SHF_GNU_RETAIN, in which case GAS > - requires the full declaration every time. */ > + part of a COMDAT groups or with SHF_GNU_RETAIN or with SHF_LINK_ORDER, > + in which case GAS requires the full declaration every time. */ > if (!(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)) > - && !(flags & SECTION_RETAIN) > + && !(flags & (SECTION_RETAIN | SECTION_LINK_ORDER)) > && (flags & SECTION_DECLARED)) > { > fprintf (asm_out_file, "\t.section\t%s\n", name); > > > Jakub > LGTM. But I can't approve it. Thanks. -- H.J.