* [PATCH] bfd: check for truncation with R_RISCV_32 relocations
@ 2024-01-30 18:21 Joseph Faulls
2024-01-30 23:38 ` Nelson Chu
2024-01-31 9:14 ` Andreas Schwab
0 siblings, 2 replies; 5+ messages in thread
From: Joseph Faulls @ 2024-01-30 18:21 UTC (permalink / raw)
To: binutils; +Cc: nelson, palmer
[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]
Relevant bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=31318
With high addresses, these relocations can be truncated. In these cases,
throw an error instead of silently truncating.
bfd/
* elfnn-riscv.c (perform_relocation): Check for overflow.
ld/
* ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
* ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d:
New test case.
---
bfd/elfnn-riscv.c | 3 +++
ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d | 4 ++++
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 +
3 files changed, 8 insertions(+)
create mode 100644 ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 8b27e3b8d6a..975b40e5f53 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -1865,6 +1865,9 @@ perform_relocation (const reloc_howto_type *howto,
}
case R_RISCV_32:
+ if (value & ~howto->dst_mask)
+ return bfd_reloc_overflow;
+ break;
case R_RISCV_64:
case R_RISCV_ADD8:
case R_RISCV_ADD16:
diff --git a/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d
new file mode 100644
index 00000000000..925d1cdb662
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d
@@ -0,0 +1,4 @@
+#source: data-reloc.s
+#as: -march=rv64i -mabi=lp64 -defsym __abs__=1
+#ld: -m[riscv_choose_lp64_emul] -Ttext 0x8000 --defsym _start=0x0 --defsym abs=0xc00000100 --defsym abs_local=0x200
+#error: .*relocation truncated to fit: R_RISCV_32
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 7e1281d826b..5a4d8728366 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -175,6 +175,7 @@ if [istarget "riscv*-*-*"] {
run_dump_test "uleb128"
run_dump_test "pr31179"
run_dump_test "pr31179-r"
+ run_dump_test "data-reloc-rv64-abs32-truncation"
run_ld_link_tests [list \
[list "Weak reference 32" "-T weakref.ld -m[riscv_choose_ilp32_emul]" "" \
"-march=rv32i -mabi=ilp32" {weakref32.s} \
--
2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations 2024-01-30 18:21 [PATCH] bfd: check for truncation with R_RISCV_32 relocations Joseph Faulls @ 2024-01-30 23:38 ` Nelson Chu 2024-01-31 14:08 ` Joseph Faulls 2024-01-31 9:14 ` Andreas Schwab 1 sibling, 1 reply; 5+ messages in thread From: Nelson Chu @ 2024-01-30 23:38 UTC (permalink / raw) To: Joseph Faulls; +Cc: binutils, palmer [-- Attachment #1: Type: text/plain, Size: 3084 bytes --] It's been a long time so I forgot the details, if we add this overflow checks for ADD/SUB/SET relocations, then the regression will fail since we may generate 32-bit debug information for the rv64 toolchain in gcc, and that's why we always silent the truncated error. Not sure if that also fails for now, we need to make sure of that at first. Nelson On Wed, Jan 31, 2024 at 2:21 AM Joseph Faulls <Joseph.Faulls@imgtec.com> wrote: > Relevant bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=31318 > > > > With high addresses, these relocations can be truncated. In these cases, > > throw an error instead of silently truncating. > > > > bfd/ > > * elfnn-riscv.c (perform_relocation): Check for overflow. > > > > ld/ > > * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. > > * ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d: > > New test case. > > --- > > bfd/elfnn-riscv.c | 3 +++ > > ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d | 4 ++++ > > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > > 3 files changed, 8 insertions(+) > > create mode 100644 > ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d > > > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > > index 8b27e3b8d6a..975b40e5f53 100644 > > --- a/bfd/elfnn-riscv.c > > +++ b/bfd/elfnn-riscv.c > > @@ -1865,6 +1865,9 @@ perform_relocation (const reloc_howto_type *howto, > > } > > > > case R_RISCV_32: > > + if (value & ~howto->dst_mask) > > + return bfd_reloc_overflow; > > + break; > > case R_RISCV_64: > > case R_RISCV_ADD8: > > case R_RISCV_ADD16: > > diff --git a/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d > b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d > > new file mode 100644 > > index 00000000000..925d1cdb662 > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d > > @@ -0,0 +1,4 @@ > > +#source: data-reloc.s > > +#as: -march=rv64i -mabi=lp64 -defsym __abs__=1 > > +#ld: -m[riscv_choose_lp64_emul] -Ttext 0x8000 --defsym _start=0x0 > --defsym abs=0xc00000100 --defsym abs_local=0x200 > > +#error: .*relocation truncated to fit: R_RISCV_32 > > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > index 7e1281d826b..5a4d8728366 100644 > > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > @@ -175,6 +175,7 @@ if [istarget "riscv*-*-*"] { > > run_dump_test "uleb128" > > run_dump_test "pr31179" > > run_dump_test "pr31179-r" > > + run_dump_test "data-reloc-rv64-abs32-truncation" > > run_ld_link_tests [list \ > > [list "Weak reference 32" "-T weakref.ld > -m[riscv_choose_ilp32_emul]" "" \ > > "-march=rv32i -mabi=ilp32" {weakref32.s} \ > > -- > > 2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations 2024-01-30 23:38 ` Nelson Chu @ 2024-01-31 14:08 ` Joseph Faulls 2024-02-21 11:02 ` Joseph Faulls 0 siblings, 1 reply; 5+ messages in thread From: Joseph Faulls @ 2024-01-31 14:08 UTC (permalink / raw) To: Nelson Chu, schwab; +Cc: binutils, palmer [-- Attachment #1: Type: text/plain, Size: 4370 bytes --] Replying to both comments * It's been a long time so I forgot the details, if we add this overflow checks for ADD/SUB/SET relocations, then the regression will fail since we may generate 32-bit debug information for the rv64 toolchain in gcc, and that's why we always silent the truncated error. Not sure if that also fails for now, we need to make sure of that at first. Yes, I initially did that and that’s exactly what happens. The debug information doesn’t seem to use R_RISCV_32 so it hasn’t caused any errors in the gcc test suite when the data and/or text is put at a high address. All the failures were truncation of other relocations like R_RISCV_HI20, RISCV_PCREL_HI20 and R_RISCV_GOT_HI20. * The overflow check belongs to the howto table. For the reason Nelson gave above, I don’t think this is a good idea. We’d see a lot of failures on truncation of debug information. Thanks, Joe From: Nelson Chu <nelson@rivosinc.com> Sent: Tuesday, January 30, 2024 11:39 PM To: Joseph Faulls <Joseph.Faulls@imgtec.com> Cc: binutils@sourceware.org; palmer@rivosinc.com Subject: [EXTERNAL] Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations *** CAUTION: This email originates from a source not known to Imagination Technologies. Think before you click a link or open an attachment *** It's been a long time so I forgot the details, if we add this overflow checks for ADD/SUB/SET relocations, then the regression will fail since we may generate 32-bit debug information for the rv64 toolchain in gcc, and that's why we always silent the truncated error. Not sure if that also fails for now, we need to make sure of that at first. Nelson On Wed, Jan 31, 2024 at 2:21 AM Joseph Faulls <Joseph.Faulls@imgtec.com<mailto:Joseph.Faulls@imgtec.com>> wrote: Relevant bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=31318 [sourceware.org]<https://urldefense.com/v3/__https:/sourceware.org/bugzilla/show_bug.cgi?id=31318__;!!KCwjcDI!wulN0Qao5f5Oo-QZuqVs3Oqa3M05CJfg5GTFUC0w5F2ASD-BVSAdYyHz60Izzx94bwpD3ks9VJPpDPvnnZb7$> With high addresses, these relocations can be truncated. In these cases, throw an error instead of silently truncating. bfd/ * elfnn-riscv.c (perform_relocation): Check for overflow. ld/ * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. * ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d: New test case. --- bfd/elfnn-riscv.c | 3 +++ ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d | 4 ++++ ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + 3 files changed, 8 insertions(+) create mode 100644 ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index 8b27e3b8d6a..975b40e5f53 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -1865,6 +1865,9 @@ perform_relocation (const reloc_howto_type *howto, } case R_RISCV_32: + if (value & ~howto->dst_mask) + return bfd_reloc_overflow; + break; case R_RISCV_64: case R_RISCV_ADD8: case R_RISCV_ADD16: diff --git a/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d new file mode 100644 index 00000000000..925d1cdb662 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d @@ -0,0 +1,4 @@ +#source: data-reloc.s +#as: -march=rv64i -mabi=lp64 -defsym __abs__=1 +#ld: -m[riscv_choose_lp64_emul] -Ttext 0x8000 --defsym _start=0x0 --defsym abs=0xc00000100 --defsym abs_local=0x200 +#error: .*relocation truncated to fit: R_RISCV_32 diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp index 7e1281d826b..5a4d8728366 100644 --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp @@ -175,6 +175,7 @@ if [istarget "riscv*-*-*"] { run_dump_test "uleb128" run_dump_test "pr31179" run_dump_test "pr31179-r" + run_dump_test "data-reloc-rv64-abs32-truncation" run_ld_link_tests [list \ [list "Weak reference 32" "-T weakref.ld -m[riscv_choose_ilp32_emul]" "" \ "-march=rv32i -mabi=ilp32" {weakref32.s} \ -- 2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] bfd: check for truncation with R_RISCV_32 relocations 2024-01-31 14:08 ` Joseph Faulls @ 2024-02-21 11:02 ` Joseph Faulls 0 siblings, 0 replies; 5+ messages in thread From: Joseph Faulls @ 2024-02-21 11:02 UTC (permalink / raw) To: Nelson Chu, schwab; +Cc: binutils, palmer [-- Attachment #1: Type: text/plain, Size: 4769 bytes --] Ping – any thoughts on this? From: Joseph Faulls Sent: Wednesday, January 31, 2024 2:08 PM To: Nelson Chu <nelson@rivosinc.com>; schwab@suse.de Cc: binutils@sourceware.org; palmer@rivosinc.com Subject: Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations Replying to both comments * It's been a long time so I forgot the details, if we add this overflow checks for ADD/SUB/SET relocations, then the regression will fail since we may generate 32-bit debug information for the rv64 toolchain in gcc, and that's why we always silent the truncated error. Not sure if that also fails for now, we need to make sure of that at first. Yes, I initially did that and that’s exactly what happens. The debug information doesn’t seem to use R_RISCV_32 so it hasn’t caused any errors in the gcc test suite when the data and/or text is put at a high address. All the failures were truncation of other relocations like R_RISCV_HI20, RISCV_PCREL_HI20 and R_RISCV_GOT_HI20. * The overflow check belongs to the howto table. For the reason Nelson gave above, I don’t think this is a good idea. We’d see a lot of failures on truncation of debug information. Thanks, Joe From: Nelson Chu <nelson@rivosinc.com<mailto:nelson@rivosinc.com>> Sent: Tuesday, January 30, 2024 11:39 PM To: Joseph Faulls <Joseph.Faulls@imgtec.com<mailto:Joseph.Faulls@imgtec.com>> Cc: binutils@sourceware.org<mailto:binutils@sourceware.org>; palmer@rivosinc.com<mailto:palmer@rivosinc.com> Subject: [EXTERNAL] Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations *** CAUTION: This email originates from a source not known to Imagination Technologies. Think before you click a link or open an attachment *** It's been a long time so I forgot the details, if we add this overflow checks for ADD/SUB/SET relocations, then the regression will fail since we may generate 32-bit debug information for the rv64 toolchain in gcc, and that's why we always silent the truncated error. Not sure if that also fails for now, we need to make sure of that at first. Nelson On Wed, Jan 31, 2024 at 2:21 AM Joseph Faulls <Joseph.Faulls@imgtec.com<mailto:Joseph.Faulls@imgtec.com>> wrote: Relevant bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=31318 [sourceware.org]<https://urldefense.com/v3/__https:/sourceware.org/bugzilla/show_bug.cgi?id=31318__;!!KCwjcDI!wulN0Qao5f5Oo-QZuqVs3Oqa3M05CJfg5GTFUC0w5F2ASD-BVSAdYyHz60Izzx94bwpD3ks9VJPpDPvnnZb7$> With high addresses, these relocations can be truncated. In these cases, throw an error instead of silently truncating. bfd/ * elfnn-riscv.c (perform_relocation): Check for overflow. ld/ * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. * ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d: New test case. --- bfd/elfnn-riscv.c | 3 +++ ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d | 4 ++++ ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + 3 files changed, 8 insertions(+) create mode 100644 ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index 8b27e3b8d6a..975b40e5f53 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -1865,6 +1865,9 @@ perform_relocation (const reloc_howto_type *howto, } case R_RISCV_32: + if (value & ~howto->dst_mask) + return bfd_reloc_overflow; + break; case R_RISCV_64: case R_RISCV_ADD8: case R_RISCV_ADD16: diff --git a/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d new file mode 100644 index 00000000000..925d1cdb662 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/data-reloc-rv64-abs32-truncation.d @@ -0,0 +1,4 @@ +#source: data-reloc.s +#as: -march=rv64i -mabi=lp64 -defsym __abs__=1 +#ld: -m[riscv_choose_lp64_emul] -Ttext 0x8000 --defsym _start=0x0 --defsym abs=0xc00000100 --defsym abs_local=0x200 +#error: .*relocation truncated to fit: R_RISCV_32 diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp index 7e1281d826b..5a4d8728366 100644 --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp @@ -175,6 +175,7 @@ if [istarget "riscv*-*-*"] { run_dump_test "uleb128" run_dump_test "pr31179" run_dump_test "pr31179-r" + run_dump_test "data-reloc-rv64-abs32-truncation" run_ld_link_tests [list \ [list "Weak reference 32" "-T weakref.ld -m[riscv_choose_ilp32_emul]" "" \ "-march=rv32i -mabi=ilp32" {weakref32.s} \ -- 2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bfd: check for truncation with R_RISCV_32 relocations 2024-01-30 18:21 [PATCH] bfd: check for truncation with R_RISCV_32 relocations Joseph Faulls 2024-01-30 23:38 ` Nelson Chu @ 2024-01-31 9:14 ` Andreas Schwab 1 sibling, 0 replies; 5+ messages in thread From: Andreas Schwab @ 2024-01-31 9:14 UTC (permalink / raw) To: Joseph Faulls; +Cc: binutils, nelson, palmer On Jan 30 2024, Joseph Faulls wrote: > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > index 8b27e3b8d6a..975b40e5f53 100644 > --- a/bfd/elfnn-riscv.c > +++ b/bfd/elfnn-riscv.c > @@ -1865,6 +1865,9 @@ perform_relocation (const reloc_howto_type *howto, > } > > case R_RISCV_32: > + if (value & ~howto->dst_mask) > + return bfd_reloc_overflow; > + break; The overflow check belongs to the howto table. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-21 11:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-30 18:21 [PATCH] bfd: check for truncation with R_RISCV_32 relocations Joseph Faulls 2024-01-30 23:38 ` Nelson Chu 2024-01-31 14:08 ` Joseph Faulls 2024-02-21 11:02 ` Joseph Faulls 2024-01-31 9:14 ` Andreas Schwab
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).