public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] x86/x32: relocation overflow detection
@ 2022-02-25 11:18 Jan Beulich
  2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jan Beulich @ 2022-02-25 11:18 UTC (permalink / raw)
  To: Binutils

While the first fix is x86-specific, the 2nd one actually is generic,
with just a new x86-specific testcase being added. Other architectures,
like RISC-V, look to also be affected and hence might benefit from new
testcases as well (but I'd like to leave that to people better knowing
those architectures).

1: x86: don't suppress overflow diagnostics in x32 mode
2: ELF32: don't silently truncate relocation addends

Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode
  2022-02-25 11:18 [PATCH 0/2] x86/x32: relocation overflow detection Jan Beulich
@ 2022-02-25 11:19 ` Jan Beulich
  2022-02-25 15:14   ` H.J. Lu
  2022-02-25 11:20 ` [PATCH 2/2] ELF32: don't silently truncate relocation addends Jan Beulich
  2022-03-21 14:16 ` [PATCH v2] " Jan Beulich
  2 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2022-02-25 11:19 UTC (permalink / raw)
  To: Binutils

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ELF32: don't silently truncate relocation addends
  2022-02-25 11:18 [PATCH 0/2] x86/x32: relocation overflow detection Jan Beulich
  2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
@ 2022-02-25 11:20 ` Jan Beulich
  2022-02-27 14:01   ` Alan Modra
  2022-03-21 14:16 ` [PATCH v2] " Jan Beulich
  2 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2022-02-25 11:20 UTC (permalink / raw)
  To: Binutils

At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
addends as 64-bit values, but store them in signed 32-bit fields when
generating the file without encountering any earlier error. When the
relocated field is a 64-bit one, the value resulting after processing
the relocation record when linking (or the latest when loading) may
thus be wrong due to the truncation.

With the code change in place, one x32 testcase actually triggers the
new diagnostic. That one case of too large a (negative) addend is being
adjusted alongside the addition of a new testcase to actually trigger
the new error. (Note that due to internal BFD behavior the relocation in
.data doesn't get processed anymore after the errors in .text.)

Note that in principle it is possible to express 64-bit relocations in
ELF32, but this would require .rel relocations, i.e. with the addend
stored in the 64-bit field being relocated. But I guess it would be a
lot of effort for little gain to actually support this.
---
I guess this may not be the only place which wants such a check, but at
least this looks to be one central place.

Originally I had the conditional as

  if ((int64_t) ptr->addend != (int32_t) ptr->addend)

but I thought it would be better to avoid the non-portability of the
conversion from unsigned to signed types. Afaict the way it is now it
ought to be portable, as it involves only a signed->unsigned conversion.

--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -997,6 +997,17 @@ elf_write_relocs (bfd *abfd, asection *s
 	  return;
 	}
 
+#if defined(BFD64) && ARCH_SIZE == 32
+      if (ptr->addend - INT32_MIN > UINT32_MAX)
+	{
+	  _bfd_error_handler (_("%pB: %pA+%"BFD_VMA_FMT"x: "
+				"relocation addend %"BFD_VMA_FMT"x too large"),
+			      abfd, sec, ptr->address, ptr->addend);
+	  *failedp = true;
+	  bfd_set_error (bfd_error_bad_value);
+	}
+#endif
+
       src_rela.r_offset = ptr->address + addr_offset;
       src_rela.r_info = ELF_R_INFO (n, ptr->howto->type);
       src_rela.r_addend = ptr->addend;
--- a/gas/testsuite/gas/i386/ilp32/ilp32.exp
+++ b/gas/testsuite/gas/i386/ilp32/ilp32.exp
@@ -38,6 +38,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     }
 
     run_list_test "reloc64" "--defsym _bad_=1"
+    run_list_test "reloc-2"
 
     set ASFLAGS "$old_ASFLAGS"
 }
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.l
@@ -0,0 +1,4 @@
+.*: \.text\+2:.*addend.*too large.*
+.*: \.text\+b:.*addend.*too large.*
+.*: Assembler messages:
+.*: Fatal error: .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.s
@@ -0,0 +1,7 @@
+	.text
+_start:
+	movabs	$x+0x123456789, %rax
+	movabs	x+0x123456789, %eax
+
+	.data
+	.quad x+0x123456789
--- a/gas/testsuite/gas/i386/ilp32/reloc64.d
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.d
@@ -61,7 +61,8 @@ Disassembly of section \.text:
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
-.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x1
+.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x7fffffff
+.*[ 	]+R_X86_64_64[ 	]+xtrn\-0x80000000
 Disassembly of section \.data:
 #...
 .*[ 	]+R_X86_64_32[ 	]+xtrn
--- a/gas/testsuite/gas/i386/ilp32/reloc64.l
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.l
@@ -51,17 +51,17 @@
 .*:175: Error: .*
 .*:176: Error: .*
 .*:177: Error: .*
-.*:189: Error: .*
-.*:192: Error: .* too large for field of 4 bytes at .*
+.*:190: Error: .*
 .*: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 .*
+.*:196: Error: .* too large for field of 4 bytes 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 .*
+.*:198: Error: .* too large for field of 2 bytes at .*
+.*:198: Error: .* too large for field of 1 byte at .*
+.*:201: Error: .* too large for field of 4 bytes at .*
 .*:202: Error: .* too large for field of 2 bytes at .*
-.*:203: Error: .* too large for field of 1 byte at .*
+.*:203: Error: .* too large for field of 2 bytes at .*
 .*:204: Error: .* too large for field of 1 byte at .*
+.*:205: 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
@@ -178,7 +178,8 @@ bad	.byte	xtrn@tpoff
 
 	.text
 	mov	xtrn@tpoff (%rbx), %eax
-	movabsq	$xtrn - 4294967295, %rbp
+	movabsq	$xtrn + 0x7fffffff, %rbx
+	movabsq	$xtrn - 0x80000000, %rbp
 
 	.data
 	.quad	xtrn


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode
  2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
@ 2022-02-25 15:14   ` H.J. Lu
  0 siblings, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2022-02-25 15:14 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Fri, Feb 25, 2022 at 3:19 AM Jan Beulich <jbeulich@suse.com> 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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ELF32: don't silently truncate relocation addends
  2022-02-25 11:20 ` [PATCH 2/2] ELF32: don't silently truncate relocation addends Jan Beulich
@ 2022-02-27 14:01   ` Alan Modra
  2022-02-28  7:21     ` Jan Beulich
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Modra @ 2022-02-27 14:01 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils, H.J. Lu, Nick Clifton

On Fri, Feb 25, 2022 at 12:20:50PM +0100, Jan Beulich wrote:
> At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
> addends as 64-bit values, but store them in signed 32-bit fields when
> generating the file without encountering any earlier error. When the
> relocated field is a 64-bit one, the value resulting after processing
> the relocation record when linking (or the latest when loading) may
> thus be wrong due to the truncation.

The patch causes regressions on cris, h8300, mips, and nios2.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ELF32: don't silently truncate relocation addends
  2022-02-27 14:01   ` Alan Modra
@ 2022-02-28  7:21     ` Jan Beulich
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2022-02-28  7:21 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils, H.J. Lu, Nick Clifton

On 27.02.2022 15:01, Alan Modra wrote:
> On Fri, Feb 25, 2022 at 12:20:50PM +0100, Jan Beulich wrote:
>> At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
>> addends as 64-bit values, but store them in signed 32-bit fields when
>> generating the file without encountering any earlier error. When the
>> relocated field is a 64-bit one, the value resulting after processing
>> the relocation record when linking (or the latest when loading) may
>> thus be wrong due to the truncation.
> 
> The patch causes regressions on cris, h8300, mips, and nios2.

Oh, I see. I'll look into that, and I should have clarified that just
like for "gas: retain whitespace between strings" I didn't do a wide
test run just yet (should have marked the one here "partly RFC" as well,
I guess). I'm sorry if this has caused you doing extra work.

Jan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ELF32: don't silently truncate relocation addends
  2022-02-25 11:18 [PATCH 0/2] x86/x32: relocation overflow detection Jan Beulich
  2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
  2022-02-25 11:20 ` [PATCH 2/2] ELF32: don't silently truncate relocation addends Jan Beulich
@ 2022-03-21 14:16 ` Jan Beulich
  2022-03-22 14:20   ` Nick Clifton
  2 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2022-03-21 14:16 UTC (permalink / raw)
  To: Binutils

At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
addends as 64-bit values, but store them in signed 32-bit fields when
generating the file without encountering any earlier error. When the
relocated field is a 64-bit one, the value resulting after processing
the relocation record when linking (or the latest when loading) may
thus be wrong due to the truncation.

With the code change in place, one x32 testcase actually triggers the
new diagnostic. That one case of too large a (negative) addend is being
adjusted alongside the addition of a new testcase to actually trigger
the new error. (Note that due to internal BFD behavior the relocation in
.data doesn't get processed anymore after the errors in .text.)

Note that in principle it is possible to express 64-bit relocations in
ELF32, but this would require .rel relocations, i.e. with the addend
stored in the 64-bit field being relocated. But I guess it would be a
lot of effort for little gain to actually support this.
---
v2: Restrict to RELA relocations, as was originally intended, and to
    relocations of fields wider than 32 bits.
---
I guess this may not be the only place which wants such a check, but at
least this looks to be one central place.

Originally I had the conditional as

  if ((int64_t) ptr->addend != (int32_t) ptr->addend)

but I thought it would be better to avoid the non-portability of the
conversion from unsigned to signed types. Afaict the way it is now it
ought to be portable, as it involves only a signed->unsigned conversion.

--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -997,6 +997,19 @@ elf_write_relocs (bfd *abfd, asection *s
 	  return;
 	}
 
+#if defined(BFD64) && ARCH_SIZE == 32
+      if (rela_hdr->sh_type == SHT_RELA
+	  && ptr->howto->bitsize > 32
+	  && ptr->addend - INT32_MIN > UINT32_MAX)
+	{
+	  _bfd_error_handler (_("%pB: %pA+%"BFD_VMA_FMT"x: "
+				"relocation addend %"BFD_VMA_FMT"x too large"),
+			      abfd, sec, ptr->address, ptr->addend);
+	  *failedp = true;
+	  bfd_set_error (bfd_error_bad_value);
+	}
+#endif
+
       src_rela.r_offset = ptr->address + addr_offset;
       src_rela.r_info = ELF_R_INFO (n, ptr->howto->type);
       src_rela.r_addend = ptr->addend;
--- a/gas/testsuite/gas/i386/ilp32/ilp32.exp
+++ b/gas/testsuite/gas/i386/ilp32/ilp32.exp
@@ -38,6 +38,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
     }
 
     run_list_test "reloc64" "--defsym _bad_=1"
+    run_list_test "reloc-2"
 
     set ASFLAGS "$old_ASFLAGS"
 }
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.l
@@ -0,0 +1,4 @@
+.*: \.text\+2:.*addend.*too large.*
+.*: \.text\+b:.*addend.*too large.*
+.*: Assembler messages:
+.*: Fatal error: .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/ilp32/reloc-2.s
@@ -0,0 +1,7 @@
+	.text
+_start:
+	movabs	$x+0x123456789, %rax
+	movabs	x+0x123456789, %eax
+
+	.data
+	.quad x+0x123456789
--- a/gas/testsuite/gas/i386/ilp32/reloc64.d
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.d
@@ -61,7 +61,8 @@ Disassembly of section \.text:
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
 .*[ 	]+R_X86_64_TPOFF32[ 	]+xtrn
-.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x1
+.*[ 	]+R_X86_64_64[ 	]+xtrn\+0x7fffffff
+.*[ 	]+R_X86_64_64[ 	]+xtrn\-0x80000000
 Disassembly of section \.data:
 #...
 .*[ 	]+R_X86_64_32[ 	]+xtrn
--- a/gas/testsuite/gas/i386/ilp32/reloc64.l
+++ b/gas/testsuite/gas/i386/ilp32/reloc64.l
@@ -51,17 +51,17 @@
 .*:175: Error: .*
 .*:176: Error: .*
 .*:177: Error: .*
-.*:189: Error: .*
-.*:192: Error: .* too large for field of 4 bytes at .*
+.*:190: Error: .*
 .*: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 .*
+.*:196: Error: .* too large for field of 4 bytes 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 .*
+.*:198: Error: .* too large for field of 2 bytes at .*
+.*:198: Error: .* too large for field of 1 byte at .*
+.*:201: Error: .* too large for field of 4 bytes at .*
 .*:202: Error: .* too large for field of 2 bytes at .*
-.*:203: Error: .* too large for field of 1 byte at .*
+.*:203: Error: .* too large for field of 2 bytes at .*
 .*:204: Error: .* too large for field of 1 byte at .*
+.*:205: 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
@@ -178,7 +178,8 @@ bad	.byte	xtrn@tpoff
 
 	.text
 	mov	xtrn@tpoff (%rbx), %eax
-	movabsq	$xtrn - 4294967295, %rbp
+	movabsq	$xtrn + 0x7fffffff, %rbx
+	movabsq	$xtrn - 0x80000000, %rbp
 
 	.data
 	.quad	xtrn


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] ELF32: don't silently truncate relocation addends
  2022-03-21 14:16 ` [PATCH v2] " Jan Beulich
@ 2022-03-22 14:20   ` Nick Clifton
  0 siblings, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2022-03-22 14:20 UTC (permalink / raw)
  To: Jan Beulich, Binutils

Hi Jan,

> At least x86-64's x32 sub-mode and RISC-V's 32-bit mode calculate
> addends as 64-bit values, but store them in signed 32-bit fields when
> generating the file without encountering any earlier error. When the
> relocated field is a 64-bit one, the value resulting after processing
> the relocation record when linking (or the latest when loading) may
> thus be wrong due to the truncation.

Patch approved - please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-03-22 14:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 11:18 [PATCH 0/2] x86/x32: relocation overflow detection Jan Beulich
2022-02-25 11:19 ` [PATCH 1/2] x86: don't suppress overflow diagnostics in x32 mode Jan Beulich
2022-02-25 15:14   ` H.J. Lu
2022-02-25 11:20 ` [PATCH 2/2] ELF32: don't silently truncate relocation addends Jan Beulich
2022-02-27 14:01   ` Alan Modra
2022-02-28  7:21     ` Jan Beulich
2022-03-21 14:16 ` [PATCH v2] " Jan Beulich
2022-03-22 14:20   ` Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).