From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by sourceware.org (Postfix) with ESMTPS id 40D943858D28 for ; Fri, 25 Feb 2022 15:15:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 40D943858D28 Received: by mail-pg1-x531.google.com with SMTP id 12so4947722pgd.0 for ; Fri, 25 Feb 2022 07:15:12 -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=R9bBGpdIz7ev1MVsvThFv4WuwO1993NK/h3F2nvCF4I=; b=2xNZgS8cFRwC9UmZBqdaTd8tSEdNYQb9vl7NW4nxJ6F0AvbwKlOpje+/HXzSRPhNL+ FtWHjFRoOYMRokAGDLfez2YTBDfv/HWinbUfqmxPrRjecCqd7xqoMzk5DEBvr830NHbh MFlh/TfXoWDBvigdC1LxsH7QuIe9CtNqk0VNXpogJpPsfVjec9oN2nslkFL1Eqo4a/Iw 3wf77nV7gRQoXKZuA6rZnbRvZd1VcCTZvmEo1jzTz3k/2OJC5RntE2zCWlIIPKegf9Y+ Awjpmca42GZiIbuxnG+RdpYL2NjvH7Dcy2cFmcj2o0YUd7d9bnAQfWnt8bS+KCcVsSDG 2bOA== X-Gm-Message-State: AOAM531v8MSJtNTbVAvfZ/TDep6sXSQos0jDyNTX86+MpXe/zFKDarcQ rPqgw6YhSS+RHovhSuTFycwPgaUk7hhIlCmOw1atXBE5 X-Google-Smtp-Source: ABdhPJxeAwRHnRX1U7BMZqpG2oC2ZoIAcxIWl/6n4aBXn8PoY0iuZ22WXJCWi+MTEWzFVm1TyOGE4saJNIKS/UnQur4= X-Received: by 2002:aa7:8882:0:b0:4e1:4531:e3c8 with SMTP id z2-20020aa78882000000b004e14531e3c8mr8092413pfe.76.1645802111117; Fri, 25 Feb 2022 07:15:11 -0800 (PST) MIME-Version: 1.0 References: <9d8645a1-c97d-ef32-0f96-366f0ae43f06@suse.com> In-Reply-To: <9d8645a1-c97d-ef32-0f96-366f0ae43f06@suse.com> From: "H.J. Lu" Date: Fri, 25 Feb 2022 07:14:35 -0800 Message-ID: Subject: Re: [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode To: Jan Beulich Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3020.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.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: Fri, 25 Feb 2022 15:15:13 -0000 On Fri, Feb 25, 2022 at 3:19 AM Jan Beulich wrote: > > Unlike in 64-bit mode, where values wrap at the 64-bit boundary anyway, > there's no wrapping at the 32-bit boundary here, and hence overflow > detection shouldn't be suppressed just because rela relocations are > going to be used. > > The extra check against NO_RELOC is actually a result of an ilp32 test > otherwise failing. But thinking about it, reporting overflows for > not-really-relocations (typically because of earlier errors) makes > little sense in general. Perhaps this should even be extended to non- > 64-bit modes. > --- > After 862be3fb9a6d ("Disallow 64bit relocations in x32 mode") introduced > the checking for 64-bit relocations in tc_gen_reloc(), d7921315bafb > ("Check R_X86_64_32 overflow and allow R_X86_64_64 for x32") removed > BFD_RELOC_64 from the set again. Hence besides the error messages > triggered by the change here for 32-bit (and smaller) relocations, for > that 64-bit one some diagnostic is needed as well. But that'll be the > topic of another patch, as that's affecting at least RISC-V as well. > > Of course I have to admit that I don't really see why e.g. > BFD_RELOC_64_PCREL cannot be represented (as the diagnostic puts it) - > it's all a matter of whether the addend fits. The relocated field can > hold all values. > > As to NO_RELOC: While looking around I didn't find uses of this value > which aren't accompanied by as_bad() invocations. Yet I'm still somewhat > uncertain whether the added check may not end up suppressing necessary > overflow checks. > > Note that the as_bad() triggered here is actually unhelpful: It reports > the already truncated value (which does fit in 32 bits), leaving it to > the user to infer that the real value was actually larger. > > Note further that similar diagnostics ought to be emitted for 32-bit > code, but they are hidden by the effects of a442cac5084e ("ix86: wrap > constants"). That change effectively dealt with one aspect of > inconsistencies between BFD64 and !BFD64 builds by introducing another > one: Numbers wider than 32 bits ar represented as bignum in !BFD64 > builds, and bignums won't be accepted as relocation addends. Hence we > have yet again a case where code would assemble in one case, but > encounter errors in the other. It _may_ be acceptable in this case, > where wider-than-32-bits values are supplied from the source file rather > than being the result of calculations overflowing in 32 bits. > > --- a/gas/config/tc-i386.c > +++ b/gas/config/tc-i386.c > @@ -12790,7 +12790,8 @@ md_apply_fix (fixS *fixP, valueT *valP, > #endif > else if (use_rela_relocations) > { > - fixP->fx_no_overflow = 1; > + if (!disallow_64bit_reloc || fixP->fx_r_type == NO_RELOC) > + fixP->fx_no_overflow = 1; > /* Remember value for tc_gen_reloc. */ > fixP->fx_addnumber = value; > value = 0; > --- a/gas/testsuite/gas/i386/ilp32/reloc64.l > +++ b/gas/testsuite/gas/i386/ilp32/reloc64.l > @@ -52,3 +52,16 @@ > .*:176: Error: .* > .*:177: Error: .* > .*:189: Error: .* > +.*:192: Error: .* too large for field of 4 bytes at .* > +.*:193: Error: .* too large for field of 4 bytes at .* > +.*:194: Error: .* too large for field of 4 bytes at .* > +.*:195: Error: .* too large for field of 4 bytes at .* > +.*:196: Error: .* too large for field of 2 bytes at .* > +.*:196: Error: .* too large for field of 1 byte at .* > +.*:197: Error: .* too large for field of 2 bytes at .* > +.*:197: Error: .* too large for field of 1 byte at .* > +.*:200: Error: .* too large for field of 4 bytes at .* > +.*:201: Error: .* too large for field of 2 bytes at .* > +.*:202: Error: .* too large for field of 2 bytes at .* > +.*:203: Error: .* too large for field of 1 byte at .* > +.*:204: Error: .* too large for field of 1 byte at .* > --- a/gas/testsuite/gas/i386/ilp32/reloc64.s > +++ b/gas/testsuite/gas/i386/ilp32/reloc64.s > @@ -187,3 +187,18 @@ bad .byte xtrn@tpoff > .long xtrn@got - 4 > .long xtrn@got + 4 > bad .long xtrn@plt - . > + > + .text > +bad add $x+0x123456789, %rax > +bad add $x+0x123456789, %eax > +bad add x+0x123456789, %eax > +bad add x+0x123456789(%eax), %eax > +bad enter $x+0x123456789, $x+0x123456789 > +bad enter $x+0x12345, $x+0x123 > + > + .data > +bad .long x+0x123456789 > +bad .word x+0x123456789 > +bad .word x+0x12345 > +bad .byte x+0x123456789 > +bad .byte x+0x123 > LGTM. Thanks. -- H.J.