[BOLT] pacret-scanner: fix regression test failure #128576
Merged
+3
−77
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
... which is caused by a seemingly recent change in BOLTs basic block calculation, where function calls seem to be ending basic blocks? I don't have a pointer to the commit that caused this change. I'll be looking for that later. For now, I'm trying to get the regression tests passing again.
kbeyls
requested review from
aaupov,
maksfb,
rafaelauler,
ayermolo,
dcci and
yota9
as code owners
kbeyls
removed request for
aaupov,
dcci,
maksfb,
rafaelauler,
yota9 and
ayermolo
|
@llvm/pr-subscribers-bolt Author: Kristof Beyls (kbeyls) Changes... which is caused by a seemingly recent change in BOLTs basic block calculation, where function calls seem to be ending basic blocks? I don't have a pointer to the commit that caused this change. I'll be looking for that later. For now, I'm trying to get the regression tests passing again. Full diff: https://github.com/llvm/llvm-project/pull/128576.diff 1 Files Affected:
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index 92aca0efd4418..cb81a63289f43 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -18,10 +18,6 @@ f1:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: ret
@@ -39,15 +35,11 @@ f_intermediate_overwrite1:
add x0, x0, #3
autiasp
ldp x29, x30, [sp], #16
-// CHECK-LABEL: GS-PACRET: non-protected ret found in function f_intermediate_overwrite1, basic block .LBB
+// CHECK-LABEL: GS-PACRET: non-protected ret found in function f_intermediate_overwrite1, basic block {{[0-9a-zA-Z.]+}}
// CHECK-NEXT: The return instruction is {{[0-9a-f]+}}: ret
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: autiasp
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
@@ -71,10 +63,6 @@ f_intermediate_overwrite2:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mov x30, x0
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autiasp
@@ -114,10 +102,6 @@ f_intermediate_overwrite3:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mov w30, w0
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autiasp
@@ -142,10 +126,6 @@ f_nonx30_ret:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mov x16, x30
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: mov x16, x30
@@ -225,10 +205,7 @@ f_callclobbered_x30:
// CHECK-LABEL: GS-PACRET: non-protected ret found in function f_callclobbered_x30, basic block {{[0-9a-zA-Z.]+}}, at address
// CHECK-NEXT: The return instruction is {{[0-9a-f]+}}: ret
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
-// CHECK-NEXT: 1. {{[0-9a-f]+}}: bl g@PLT
-// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
-// CHECK-NEXT: {{[0-9a-f]+}}: ret
+// CHECK-NEXT: 1. {{[0-9a-f]+}}: bl
ret
.size f_callclobbered_x30, .-f_callclobbered_x30
@@ -239,10 +216,7 @@ f_callclobbered_calleesaved:
// CHECK-LABEL: GS-PACRET: non-protected ret found in function f_callclobbered_calleesaved, basic block {{[0-9a-zA-Z.]+}}, at address
// CHECK-NEXT: The return instruction is {{[0-9a-f]+}}: ret x19
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
-// CHECK-NEXT: 1. {{[0-9a-f]+}}: bl g@PLT
-// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
-// CHECK-NEXT: {{[0-9a-f]+}}: ret x19
+// CHECK-NEXT: 1. {{[0-9a-f]+}}: bl
// x19, according to the Arm ABI (AAPCS) is a callee-saved register.
// Therefore, if function g respects the AAPCS, it should not write
// anything to x19. However, we can't know whether function g actually
@@ -330,10 +304,6 @@ f_autia1716:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autia1716
@@ -356,10 +326,6 @@ f_autib1716:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autib1716
@@ -382,10 +348,6 @@ f_autiax12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autia x12, sp
@@ -408,10 +370,6 @@ f_autibx12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autib x12, sp
@@ -463,10 +421,6 @@ f_autdax12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autda x12, sp
@@ -489,10 +443,6 @@ f_autdbx12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autdb x12, sp
@@ -544,10 +494,6 @@ f_autizax12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autiza x12
@@ -570,10 +516,6 @@ f_autizbx12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autizb x12
@@ -625,10 +567,6 @@ f_autdzax12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autdza x12
@@ -651,10 +589,6 @@ f_autdzbx12:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autdzb x12
@@ -913,10 +847,6 @@ f_autia171615:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autia171615
@@ -939,10 +869,6 @@ f_autib171615:
// CHECK-NEXT: The 1 instructions that write to the return register after any authentication are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: This happens in the following basic block:
-// CHECK-NEXT: {{[0-9a-f]+}}: paciasp
-// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]!
-// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp
-// CHECK-NEXT: {{[0-9a-f]+}}: bl g@PLT
// CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3
// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10
// CHECK-NEXT: {{[0-9a-f]+}}: autib171615
|
|
Confirmed fixed, thanks! |
There were no recent changes to the way we build BBs. Please keep us posted of your findings. |
My git bisect run ended up pointing to commit 52fc6ff. I'd need to find some time to understand why this commit is causing the behavioural change in |
It seems only calls to unresolved symbols are affected, in that a new basic block is started after a call to an unresolved symbol, but not in a call to a resolved symbol. I'm guessing this doesn't happen (often) in real binaries, but does happen frequently in our regression tests. A small reproducer showing this is: #!/bin/bash
CLANG=./bin/clang
BOLT=./bin/llvm-bolt
cat << EOF > ./test_bolt_bb.s
.text
.globl f1
.type f1,@function
f1:
stp x29, x30, [sp, #-16]!
mov x29, sp
bl foo // unresolved symbol
ldp x29, x30, [sp], #16
ret
.size f1, .-f1
.globl f2
.type f2,@function
f2:
stp x29, x30, [sp, #-16]!
mov x29, sp
bl f1 // resolved symbol
ldp x29, x30, [sp], #16
ret
.size f2, .-f2
EOF
$CLANG -fuse-ld=lld -Wl,--unresolved-symbols=ignore-all --target=aarch64-linux-gnu -nostartfiles -nostdlib -ffreestanding ./test_bolt_bb.s -o ./test_bolt_bb.exe
$BOLT -o tmp.exe --print-cfg --no-threads ./test_bolt_bb.exeproduces: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... which is caused by a seemingly recent change in BOLTs basic block calculation, where function calls seem to be ending basic blocks? I don't have a pointer to the commit that caused this change. I'll be looking for that later. For now, I'm trying to get the regression tests passing again.