Skip to content

Conversation

@aaupov
Copy link
Contributor

@aaupov aaupov commented Oct 9, 2024

Lift the restriction that builtin_unreachable has to be strictly within
MaxSize of a function.

Test Plan: added builtin_unreachable.s

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Lift the restriction that builtin_unreachable has to be strictly within
MaxSize of a function.

Test Plan: added builtin_unreachable.s


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

2 Files Affected:

  • (modified) bolt/lib/Core/BinaryFunction.cpp (+3-2)
  • (added) bolt/test/X86/builtin_unreachable.s (+33)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 27c8ccefedee10..8b1f441a3a01da 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1365,8 +1365,9 @@ Error BinaryFunction::disassemble() {
           if (containsAddress(TargetAddress)) {
             TargetSymbol = getOrCreateLocalLabel(TargetAddress);
           } else {
-            if (TargetAddress == getAddress() + getSize() &&
-                TargetAddress < getAddress() + getMaxSize() &&
+            if (BC.isELF() && !BC.getBinaryDataAtAddress(TargetAddress) &&
+                TargetAddress == getAddress() + getSize() &&
+                TargetAddress <= getAddress() + getMaxSize() &&
                 !(BC.isAArch64() &&
                   BC.handleAArch64Veneer(TargetAddress, /*MatchOnly*/ true))) {
               // Result of __builtin_unreachable().
diff --git a/bolt/test/X86/builtin_unreachable.s b/bolt/test/X86/builtin_unreachable.s
new file mode 100644
index 00000000000000..ab533629d1a846
--- /dev/null
+++ b/bolt/test/X86/builtin_unreachable.s
@@ -0,0 +1,33 @@
+## Check that BOLT properly identifies a jump to builtin_unreachable
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: ld.lld -q -o %t %t.o
+# RUN: llvm-bolt %t -o %t.null -lite=0 -print-disasm | FileCheck %s
+# CHECK:      callq bar
+# CHECK-NEXT: nop
+
+.text
+.globl main
+.type main, @function
+main:
+  call foo
+  .size main, .-main
+
+.section .mytext.bar, "ax"
+.globl  bar
+.type	bar, @function
+bar:
+  ud2
+	.size	bar, .-bar
+
+.section .mytext.foo, "ax"
+.globl	foo
+.type	foo, @function
+foo:
+.cfi_startproc
+  callq bar
+  jmp .Lunreachable
+  ret
+  .cfi_endproc
+	.size	foo, .-foo
+.Lunreachable:

Created using spr 1.3.4

static cl::opt<bool> PrintOffsets("print-offsets",
cl::desc("print basic block offsets"),
cl::Hidden, cl::cat(BoltOptCategory));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when there's a data object at TargetAddress?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants