Skip to content

[lld][Hexagon] Fix R_HEX_B22_PCREL range checks (#115925) #116906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 25, 2024
Merged

[lld][Hexagon] Fix R_HEX_B22_PCREL range checks (#115925) #116906

merged 1 commit into from
Nov 25, 2024

Conversation

@androm3da
Copy link
Member

@androm3da androm3da commented Nov 20, 2024

Range checks for R_HEX_B22_PCREL did not account for the fact that offset is measured in instructions, not bytes.

Add a test for all range-checked relocations.

@androm3da androm3da self-assigned this Nov 20, 2024
@androm3da androm3da added this to the LLVM 19.X Release milestone Nov 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024 •
edited
Loading

@llvm/pr-subscribers-backend-hexagon

@llvm/pr-subscribers-lld

Author: Brian Cain (androm3da)

Changes

Range checks for R_HEX_B22_PCREL did not account for the fact that offset is measured in instructions, not bytes.

Add a test for all range-checked relocations.

Full diff: https://github.com/llvm/llvm-project/pull/116906.diff

3 Files Affected:

  • (modified) lld/ELF/Arch/Hexagon.cpp (+1-1)
  • (modified) lld/test/ELF/hexagon-jump-error.s (+1-1)
  • (modified) lld/test/ELF/hexagon.s (+40-1)
diff --git a/lld/ELF/Arch/Hexagon.cpp b/lld/ELF/Arch/Hexagon.cpp
index 56cf96fd177042..8bcd28309f8b38 100644
--- a/lld/ELF/Arch/Hexagon.cpp
+++ b/lld/ELF/Arch/Hexagon.cpp
@@ -329,7 +329,7 @@ void Hexagon::relocate(uint8_t *loc, const Relocation &rel,
   case R_HEX_B22_PCREL:
   case R_HEX_GD_PLT_B22_PCREL:
   case R_HEX_PLT_B22_PCREL:
-    checkInt(loc, val, 22, rel);
+    checkInt(loc, val, 24, rel);
     or32le(loc, applyMask(0x1ff3ffe, val >> 2));
     break;
   case R_HEX_B22_PCREL_X:
diff --git a/lld/test/ELF/hexagon-jump-error.s b/lld/test/ELF/hexagon-jump-error.s
index fec873827e573d..53860b5daf2b16 100644
--- a/lld/test/ELF/hexagon-jump-error.s
+++ b/lld/test/ELF/hexagon-jump-error.s
@@ -25,7 +25,7 @@ if (p0) jump #1f
 .section b15, "ax"
 1:
 
-# CHECK: relocation R_HEX_B22_PCREL out of range: 8388612 is not in [-2097152, 2097151]
+# CHECK: relocation R_HEX_B22_PCREL out of range: 8388612 is not in [-8388608, 8388607]
 jump #1f
 .space (1<<23)
 .section b22, "ax"
diff --git a/lld/test/ELF/hexagon.s b/lld/test/ELF/hexagon.s
index 8ef9b8eead8f19..b1576fb47d81af 100644
--- a/lld/test/ELF/hexagon.s
+++ b/lld/test/ELF/hexagon.s
@@ -1,7 +1,9 @@
 # REQUIRES: hexagon
 # RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %S/Inputs/hexagon.s -o %t1.o
-# RUN: ld.lld %t.o %t1.o -o %t
+# RUN: ld.lld %t.o %t1.o -o %t --Ttext=0x200b4 --section-start=b_1000000=0x1000000 \
+# RUN:  --section-start=b_1000400=0x1000400 --section-start=b_1004000=0x1004000 \
+# RUN:  --section-start=b_1010000=0x1010000 --section-start=b_1800000=0x1800000
 # RUN: llvm-objdump --no-print-imm-hex -d %t | FileCheck %s
 
 # Note: 131584 == 0x20200
@@ -221,3 +223,40 @@ r0 = memw(r1+##_start)
 
 memw(r0+##_start) = r1
 # CHECK: memw(r0+##131644) = r1
+
+
+## Tests for maximum branch ranges reachable without trampolines.
+
+.section b_1000000, "ax"
+## The nop makes sure the first jump is within range.
+nop
+{ r0 = #0; jump #b_1000400 } // R_HEX_B9_PCREL
+if (r0==#0) jump:t #b_1004000 // R_HEX_B13_PCREL
+if (p0) jump #b_1010000 // R_HEX_B15_PCREL
+jump #b_1800000 // R_HEX_B22_PCREL
+
+.section b_1000400, "ax"
+nop
+
+.section b_1004000, "ax"
+nop
+
+.section b_1010000, "ax"
+nop
+
+.section b_1800000, "ax"
+nop
+
+## Make sure we got the right relocations.
+# RUN: llvm-readelf -r %t.o | FileCheck %s --check-prefix=REL
+# REL: R_HEX_B9_PCREL         00000000   b_1000400
+# REL: R_HEX_B13_PCREL        00000000   b_1004000
+# REL: R_HEX_B15_PCREL        00000000   b_1010000
+# REL: R_HEX_B22_PCREL        00000000   b_1800000
+
+# CHECK: 01000000 <b_1000000>:
+# CHECK-NEXT: 1000000: {{.*}} {  nop }
+# CHECK-NEXT: 1000004: {{.*}} {  r0 = #0 ; jump 0x1000400 }
+# CHECK-NEXT: 1000008: {{.*}} {  if (r0==#0) jump:t 0x1004000 }
+# CHECK-NEXT: 100000c: {{.*}} {  if (p0) jump:nt 0x1010000 }
+# CHECK-NEXT: 1000010: {{.*}} {  jump 0x1800000 }

@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-lld-elf

Author: Brian Cain (androm3da)

Changes

Range checks for R_HEX_B22_PCREL did not account for the fact that offset is measured in instructions, not bytes.

Add a test for all range-checked relocations.

Full diff: https://github.com/llvm/llvm-project/pull/116906.diff

3 Files Affected:

  • (modified) lld/ELF/Arch/Hexagon.cpp (+1-1)
  • (modified) lld/test/ELF/hexagon-jump-error.s (+1-1)
  • (modified) lld/test/ELF/hexagon.s (+40-1)
diff --git a/lld/ELF/Arch/Hexagon.cpp b/lld/ELF/Arch/Hexagon.cpp
index 56cf96fd177042..8bcd28309f8b38 100644
--- a/lld/ELF/Arch/Hexagon.cpp
+++ b/lld/ELF/Arch/Hexagon.cpp
@@ -329,7 +329,7 @@ void Hexagon::relocate(uint8_t *loc, const Relocation &rel,
   case R_HEX_B22_PCREL:
   case R_HEX_GD_PLT_B22_PCREL:
   case R_HEX_PLT_B22_PCREL:
-    checkInt(loc, val, 22, rel);
+    checkInt(loc, val, 24, rel);
     or32le(loc, applyMask(0x1ff3ffe, val >> 2));
     break;
   case R_HEX_B22_PCREL_X:
diff --git a/lld/test/ELF/hexagon-jump-error.s b/lld/test/ELF/hexagon-jump-error.s
index fec873827e573d..53860b5daf2b16 100644
--- a/lld/test/ELF/hexagon-jump-error.s
+++ b/lld/test/ELF/hexagon-jump-error.s
@@ -25,7 +25,7 @@ if (p0) jump #1f
 .section b15, "ax"
 1:
 
-# CHECK: relocation R_HEX_B22_PCREL out of range: 8388612 is not in [-2097152, 2097151]
+# CHECK: relocation R_HEX_B22_PCREL out of range: 8388612 is not in [-8388608, 8388607]
 jump #1f
 .space (1<<23)
 .section b22, "ax"
diff --git a/lld/test/ELF/hexagon.s b/lld/test/ELF/hexagon.s
index 8ef9b8eead8f19..b1576fb47d81af 100644
--- a/lld/test/ELF/hexagon.s
+++ b/lld/test/ELF/hexagon.s
@@ -1,7 +1,9 @@
 # REQUIRES: hexagon
 # RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %S/Inputs/hexagon.s -o %t1.o
-# RUN: ld.lld %t.o %t1.o -o %t
+# RUN: ld.lld %t.o %t1.o -o %t --Ttext=0x200b4 --section-start=b_1000000=0x1000000 \
+# RUN:  --section-start=b_1000400=0x1000400 --section-start=b_1004000=0x1004000 \
+# RUN:  --section-start=b_1010000=0x1010000 --section-start=b_1800000=0x1800000
 # RUN: llvm-objdump --no-print-imm-hex -d %t | FileCheck %s
 
 # Note: 131584 == 0x20200
@@ -221,3 +223,40 @@ r0 = memw(r1+##_start)
 
 memw(r0+##_start) = r1
 # CHECK: memw(r0+##131644) = r1
+
+
+## Tests for maximum branch ranges reachable without trampolines.
+
+.section b_1000000, "ax"
+## The nop makes sure the first jump is within range.
+nop
+{ r0 = #0; jump #b_1000400 } // R_HEX_B9_PCREL
+if (r0==#0) jump:t #b_1004000 // R_HEX_B13_PCREL
+if (p0) jump #b_1010000 // R_HEX_B15_PCREL
+jump #b_1800000 // R_HEX_B22_PCREL
+
+.section b_1000400, "ax"
+nop
+
+.section b_1004000, "ax"
+nop
+
+.section b_1010000, "ax"
+nop
+
+.section b_1800000, "ax"
+nop
+
+## Make sure we got the right relocations.
+# RUN: llvm-readelf -r %t.o | FileCheck %s --check-prefix=REL
+# REL: R_HEX_B9_PCREL         00000000   b_1000400
+# REL: R_HEX_B13_PCREL        00000000   b_1004000
+# REL: R_HEX_B15_PCREL        00000000   b_1010000
+# REL: R_HEX_B22_PCREL        00000000   b_1800000
+
+# CHECK: 01000000 <b_1000000>:
+# CHECK-NEXT: 1000000: {{.*}} {  nop }
+# CHECK-NEXT: 1000004: {{.*}} {  r0 = #0 ; jump 0x1000400 }
+# CHECK-NEXT: 1000008: {{.*}} {  if (r0==#0) jump:t 0x1004000 }
+# CHECK-NEXT: 100000c: {{.*}} {  if (p0) jump:nt 0x1010000 }
+# CHECK-NEXT: 1000010: {{.*}} {  jump 0x1800000 }

@androm3da
Copy link
Member Author

androm3da commented Nov 20, 2024

Fixes #116798

@tru tru force-pushed the bcain/lld_pcrel22_range branch from 6a1fd8a to e80925b Compare November 25, 2024 08:38
@quic-akaryaki @tru
[lld][Hexagon] Fix R_HEX_B22_PCREL range checks (llvm#115925)
e80925b 
Range checks for R_HEX_B22_PCREL did not account for the fact that
offset is measured in instructions, not bytes.

Add a test for all range-checked relocations.
@tru tru merged commit e80925b into llvm:release/19.x Nov 25, 2024
@github-actions
Copy link

github-actions bot commented Nov 25, 2024

@androm3da (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SidManning SidManning SidManning approved these changes

@tru tru Awaiting requested review from tru

@nathanchance nathanchance Awaiting requested review from nathanchance

@quic-akaryaki quic-akaryaki Awaiting requested review from quic-akaryaki

Assignees

@androm3da androm3da

Labels

backend:Hexagon lld:ELF lld

Projects

LLVM Release Status
Status: Done

Milestone

LLVM 19.X Release

Development

Successfully merging this pull request may close these issues.

5 participants

@androm3da @llvmbot @SidManning @tru @quic-akaryaki