From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by sourceware.org (Postfix) with ESMTPS id EB6F43857C7E for ; Wed, 9 Mar 2022 14:27:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EB6F43857C7E Received: by mail-pj1-x102c.google.com with SMTP id mg21-20020a17090b371500b001bef9e4657cso5487838pjb.0 for ; Wed, 09 Mar 2022 06:27:47 -0800 (PST) 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=cbJBhUCcTr1jkSBv+k6U6kZM2/ZCGwe/bKjUeig8Xlw=; b=EJ0CzRJtDVZudD1XF47VsXTlfmrpNfHyA+14lg3MroDuUs8wWH5mlvMAaUYtC64KdW 6jZjig6bZYvGZkAT5YbSgTFpTwK6mxTP3r4IPUqBFJ38hOIJMpIlp5vkcE+P0mUpP0UJ j9mPCgAIeeLLYARe+hqKQ3Ftllcpt2DOB9c/GVCns6d4cMkCwtWEOdxQQr0zFiPJOWl4 mz5V2b9t10BhCVHwrS9VjpssByEWB2+cwSgkEoAeUr8dL5ZUFTbrcst1RvMuRlQUYHtr XyGExeS2qhNJbqfW5bpx77D9oEplxSv32ZyVEbawZ6Ms7Q5fd2zDtbteema2awAQNZft +4mQ== X-Gm-Message-State: AOAM530tmN4E+PW/fcv7dQiQjZ48hCE9WmEOjRi64m8ZyuNzKNjDKXHv AbbdhyycMQz5wTiS3wmFNZJAneV+q+ZHXByPNKfFQHOX X-Google-Smtp-Source: ABdhPJw8hgpK7vWlxDImzdbtfjZcpIua3w6RkACDFpmlv7gGd18qYNoHxwXz49C6XaBpC0sGnNZgxlpIgWtBNVrc6fs= X-Received: by 2002:a17:902:b410:b0:14b:e53:7aa0 with SMTP id x16-20020a170902b41000b0014b0e537aa0mr22777271plr.101.1646836066810; Wed, 09 Mar 2022 06:27:46 -0800 (PST) MIME-Version: 1.0 References: <319f39e5-1f17-23ef-e3fa-2169876aa31c@suse.com> <5ac79eef-290c-5f77-2387-99be18e10ee8@suse.com> In-Reply-To: <5ac79eef-290c-5f77-2387-99be18e10ee8@suse.com> From: "H.J. Lu" Date: Wed, 9 Mar 2022 06:27:10 -0800 Message-ID: Subject: Re: [PATCH v2 1/3] x86-64/ELF: permit relaxed overflow checking for 32-bit PC-relative relocs To: Jan Beulich Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3020.1 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 09 Mar 2022 14:27:50 -0000 On Wed, Mar 9, 2022 at 12:21 AM Jan Beulich wrote: > > On 04.03.2022 15:18, H.J. Lu wrote: > > On Fri, Mar 04, 2022 at 02:34:58PM +0100, Jan Beulich wrote: > >> Right now it is impossible to encode certain valid 32-bit mode > >> constructs; see the respective new test case. Note that there are > >> further 32-bit PC-relative relocations, but I don't think they make a > >> lot of sense to use in mixed-bitness code, so they're not having > >> overrides put in place. > >> > >> Putting in place a new testcase, I'd like to note that the two existing > >> ones (pcrel16 and pcrel16abs) appear to be pretty pointless: They don't > >> expect any error despite supposedly checking for overflow, and in fact > >> there can't possibly be any error for the > >> - former since gas doesn't emit any relocation in the first place there, > >> - latter because the way the relocation gets expressed by gas doesn't > >> allow the linker to notice the overflow; it should be detected by gas > >> if at all, but see above (an error would be reported here for x86-64 > >> afaict, but this test doesn't get re-used there). > >> --- > >> TBD: I didn't put thoughts yet into also making this work when linking > >> ELF to PE. > >> > >> Note that I'm not sure at all whether this propagation of the struct > >> elf_linker_x86_params pointer is actually acceptable. But this is the > >> 5th or 6th try I made, with all others having been worse or not even > >> working out. Hence I'd need pretty detailed guidance on how else the > >> information could be made available. > >> --- > >> v2: Re-base and split. > >> > >> --- a/bfd/elf-linker-x86.h > >> +++ b/bfd/elf-linker-x86.h > >> @@ -28,6 +28,13 @@ enum elf_x86_prop_report > >> prop_report_shstk = 1 << 3 /* Report missing SHSTK property. */ > >> }; > >> > >> +/* Control of PC32 (on 64-bit) overflow check strictness. */ > >> +enum elf_x86_pcrel_relocs > >> +{ > >> + pcrel_relocs_default, > >> + pcrel_relocs_lax, > >> +}; > >> + > >> /* Used to pass x86-specific linker options from ld to bfd. */ > >> struct elf_linker_x86_params > >> { > >> @@ -64,6 +71,9 @@ struct elf_linker_x86_params > >> /* Report relative relocations. */ > >> unsigned int report_relative_reloc : 1; > >> > >> + /* Strictness of PC32 (on 64-bit) overflow checks. */ > >> + enum elf_x86_pcrel_relocs pcrel_relocs; > >> + > >> /* X86-64 ISA level needed. */ > >> unsigned int isa_level; > >> > >> --- a/bfd/elf64-x86-64.c > >> +++ b/bfd/elf64-x86-64.c > >> @@ -192,6 +192,15 @@ static reloc_howto_type x86_64_elf_howto > >> false) > >> }; > >> > >> +static reloc_howto_type x86_64_howto_pc32_lax = > >> + HOWTO(R_X86_64_PC32, 0, 2, 32, true, 0, complain_overflow_bitfield, > >> + bfd_elf_generic_reloc, "R_X86_64_PC32", false, 0, 0xffffffff, true); > >> + > >> +static reloc_howto_type x86_64_howto_pc32_bnd_lax = > >> + HOWTO(R_X86_64_PC32_BND, 0, 2, 32, true, 0, complain_overflow_bitfield, > >> + bfd_elf_generic_reloc, "R_X86_64_PC32_BND", false, 0, 0xffffffff, > >> + true); > >> + > >> /* Map BFD relocs to the x86_64 elf relocs. */ > >> struct elf_reloc_map > >> { > >> @@ -248,6 +257,30 @@ static const struct elf_reloc_map x86_64 > >> }; > >> > >> static reloc_howto_type * > >> +elf_x86_64_reloc_override (const bfd *abfd, reloc_howto_type *howto) > >> +{ > >> + const struct elf_linker_x86_params *params = elf_x86_tdata (abfd)->params; > >> + > >> + switch (howto->type) > >> + { > >> + default: > >> + break; > >> + > >> + case R_X86_64_PC32: > >> + if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax) > >> + break; > >> + return &x86_64_howto_pc32_lax; > >> + > >> + case R_X86_64_PC32_BND: > >> + if (params == NULL || params->pcrel_relocs != pcrel_relocs_lax) > >> + break; > >> + return &x86_64_howto_pc32_bnd_lax; > >> + } > >> + > >> + return howto; > >> +} > >> + > >> +static reloc_howto_type * > >> elf_x86_64_rtype_to_howto (bfd *abfd, unsigned r_type) > >> { > >> unsigned i; > >> @@ -275,7 +308,7 @@ elf_x86_64_rtype_to_howto (bfd *abfd, un > >> else > >> i = r_type - (unsigned int) R_X86_64_vt_offset; > >> BFD_ASSERT (x86_64_elf_howto_table[i].type == r_type); > >> - return &x86_64_elf_howto_table[i]; > >> + return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]); > >> } > >> > >> /* Given a BFD reloc type, return a HOWTO structure. */ > >> @@ -313,7 +346,7 @@ elf_x86_64_reloc_name_lookup (bfd *abfd, > >> for (i = 0; i < ARRAY_SIZE (x86_64_elf_howto_table); i++) > >> if (x86_64_elf_howto_table[i].name != NULL > >> && strcasecmp (x86_64_elf_howto_table[i].name, r_name) == 0) > >> - return &x86_64_elf_howto_table[i]; > >> + return elf_x86_64_reloc_override (abfd, &x86_64_elf_howto_table[i]); > >> > >> return NULL; > >> } > >> @@ -1846,6 +1879,9 @@ elf_x86_64_scan_relocs (bfd *abfd, struc > >> > >> BFD_ASSERT (is_x86_elf (abfd, htab)); > >> > >> + /* Make command line controlled settings accessible from the object. */ > >> + elf_x86_tdata (abfd)->params = htab->params; > >> + > >> /* Get the section contents. */ > >> if (elf_section_data (sec)->this_hdr.contents != NULL) > >> contents = elf_section_data (sec)->this_hdr.contents; > >> --- a/bfd/elfxx-x86.h > >> +++ b/bfd/elfxx-x86.h > >> @@ -702,6 +702,9 @@ struct elf_x86_obj_tdata > >> /* R_*_RELATIVE relocation in GOT for this local symbol has been > >> processed. */ > >> char *relative_reloc_done; > >> + > >> + /* Container holding command line controlled linker settings. */ > >> + const struct elf_linker_x86_params *params; > >> }; > >> > >> enum elf_x86_plt_type > >> --- /dev/null > >> +++ b/gas/testsuite/gas/i386/code32.d > >> @@ -0,0 +1,3 @@ > >> +#name: x86-64 code32 > >> +#as: -mx86-used-note=no --generate-missing-build-notes=no > >> +#readelf: -n > >> --- /dev/null > >> +++ b/gas/testsuite/gas/i386/code32.s > >> @@ -0,0 +1,11 @@ > >> + .code32 > >> + .text > >> + .section .text.0, "ax", @progbits > >> + .type func0, @function > >> +func0: > >> + call func1 > >> + ret > >> + .section .text.1, "ax", @progbits > >> + .type func1, @function > >> +func1: > >> + jmp func0 > >> --- a/gas/testsuite/gas/i386/i386.exp > >> +++ b/gas/testsuite/gas/i386/i386.exp > >> @@ -1331,6 +1331,7 @@ if [gas_64_check] then { > >> run_dump_test "x86-64-property-8" > >> run_dump_test "x86-64-property-9" > >> run_dump_test "x86-64-property-14" > >> + run_dump_test "code32" > >> > >> if {[istarget "*-*-linux*"]} then { > >> run_dump_test "x86-64-align-branch-3" > >> --- a/ld/emulparams/elf32_x86_64.sh > >> +++ b/ld/emulparams/elf32_x86_64.sh > >> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin > >> source_sh ${srcdir}/emulparams/extern_protected_data.sh > >> source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh > >> source_sh ${srcdir}/emulparams/reloc_overflow.sh > >> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh > >> source_sh ${srcdir}/emulparams/call_nop.sh > >> source_sh ${srcdir}/emulparams/cet.sh > >> source_sh ${srcdir}/emulparams/x86-report-relative.sh > >> --- a/ld/emulparams/elf_x86_64.sh > >> +++ b/ld/emulparams/elf_x86_64.sh > >> @@ -2,6 +2,7 @@ source_sh ${srcdir}/emulparams/plt_unwin > >> source_sh ${srcdir}/emulparams/extern_protected_data.sh > >> source_sh ${srcdir}/emulparams/dynamic_undefined_weak.sh > >> source_sh ${srcdir}/emulparams/reloc_overflow.sh > >> +source_sh ${srcdir}/emulparams/pcrel-relocs.sh > >> source_sh ${srcdir}/emulparams/call_nop.sh > >> source_sh ${srcdir}/emulparams/cet.sh > >> source_sh ${srcdir}/emulparams/x86-report-relative.sh > >> --- /dev/null > >> +++ b/ld/emulparams/pcrel-relocs.sh > >> @@ -0,0 +1,11 @@ > >> +PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS=' > >> + fprintf (file, _("\ > >> + -z lax-pcrel-relocs Lax PC-relative relocation overflow checks\n")); > >> +' > >> +PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS=' > >> + else if (strcmp (optarg, "lax-pcrel-relocs") == 0) > >> + params.pcrel_relocs = pcrel_relocs_lax; > >> +' > >> + > >> +PARSE_AND_LIST_OPTIONS="$PARSE_AND_LIST_OPTIONS $PARSE_AND_LIST_OPTIONS_STRICT_PCREL_RELOCS" > >> +PARSE_AND_LIST_ARGS_CASE_Z="$PARSE_AND_LIST_ARGS_CASE_Z $PARSE_AND_LIST_ARGS_CASE_Z_STRICT_PCREL_RELOCS" > >> --- a/ld/ld.texi > >> +++ b/ld/ld.texi > >> @@ -1372,6 +1372,12 @@ missing properties in input files. @opt > >> the linker issue an error for missing properties in input files. > >> Supported for Linux/x86_64. > >> > >> +@item lax-pcrel-relocs > >> +Relax relocation overflow checks for certain 32-bit PC-relative relocations > >> +which, when used by 32-bit code inside a 64-bit object, may require a > >> +larger range of values to be considered valid. > >> +Supported for x86-64 ELF targets. > >> + > > > > I think the check should be turned on automatically. Can you use a GNU > > property bit to tell linker that a larger range of values should be > > checked for R_X86_64_PC32 > > I'm not convinced that would be desirable - the relaxed checking, after > all, also affects relocations to 64-bit mode. Hence certain overflows > won't be detected anymore. Therefore I'd expect people to make use of > the new option only if they really have any affected relocations in > 32-bit code. Additionally there's no way I can see to set such a > property indicator when encountering the relocations in question only > in data definitions, unless you wanted to tie the setting of the > indicator to the mere use of .code{16,32} anywhere in the source (which > would feel way to aggressive to me). IMO this level of control can only > be achieved via command line option (without (a) becoming much more > intrusive or (b) introducing new relocation types). A new relocation type sounds better. > If this was to be "automated", the assembler would need to let the > linker know the boundaries between 64-bit and non-64-bit code, and > (yet more difficult) between data used by 64-bit code and such used > by 32-bit code. > > > and issue an error for R_X86_64_PC32_BND? > > Why should this result in an error? > > Jan > -- H.J.