[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From 1837b2981d3c6e8cee5110acbf0fea99ba2b352c Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH 1/2] [BOLT] Gadget scanner: fix LR to be safe in leaf
functions without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 9cd5032204e78..2eadaf15d3a65 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index a32e4324aa923..2dadcef095863 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instructio
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From 7e1017e95313e9eb467d74d1c33246c75c78e837 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH 1/2] [BOLT] Gadget scanner: fix LR to be safe in leaf
functions without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index ce257ddbfac76..20c8c2511428c 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index a32e4324aa923..2dadcef095863 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instructio
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/kbeyls approved this pull request. This looks like a great simple improvement, thanks! https://github.com/llvm/llvm-project/pull/141824 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/kbeyls edited https://github.com/llvm/llvm-project/pull/141824 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From e3bef0621e4b78c8a89302acfca9a1360076f269 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH] [BOLT] Gadget scanner: fix LR to be safe in leaf functions
without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index e5bdade032488..05309a47aba40 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instruction is
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From e3bef0621e4b78c8a89302acfca9a1360076f269 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH] [BOLT] Gadget scanner: fix LR to be safe in leaf functions
without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index e5bdade032488..05309a47aba40 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instruction is
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From 54c1b3aeaa9b269c57f3d121e974156204cf2ed7 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH] [BOLT] Gadget scanner: fix LR to be safe in leaf functions
without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index e5bdade032488..05309a47aba40 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instruction is
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko updated
https://github.com/llvm/llvm-project/pull/141824
>From 54c1b3aeaa9b269c57f3d121e974156204cf2ed7 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH] [BOLT] Gadget scanner: fix LR to be safe in leaf functions
without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index e5bdade032488..05309a47aba40 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instruction is
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
atrosinenko wrote: Factored this out of #137224. https://github.com/llvm/llvm-project/pull/141824 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
llvmbot wrote:
@llvm/pr-subscribers-bolt
Author: Anatoly Trosinenko (atrosinenko)
Changes
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
Full diff: https://github.com/llvm/llvm-project/pull/141824.diff
5 Files Affected:
- (modified) bolt/lib/Passes/PAuthGadgetScanner.cpp (+5-9)
- (modified) bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s (+22-9)
- (modified)
bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s (-20)
- (modified) bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s (+2-28)
- (modified) bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s (-27)
``diff
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 2aacb38ee19a9..6327a2da54d5b 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
state_is_reset_after_indirect_branch_nocfg, at address
-// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
+// Without CFG, the state is reset at labels, assuming every register that can
+// be clobbered in the function was actually clobbered.
+
+.globl lr_untouched_nocfg
+.type lr_untouched_nocfg,@function
+lr_untouched_nocfg:
+// CHECK-NOT: lr_untouched_nocfg
+adr x2, 1f
+br x2
+1:
+ret
+.size lr_untouched_nocfg, .-lr_untouched_nocfg
+
+.globl lr_clobbered_nocfg
+.type lr_clobbered_nocfg,@function
+lr_clobbered_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function
lr_clobbered_nocfg, at address
+// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
// CHECK-NEXT: The 0 instructions that write to the affected registers after
any authent
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko ready_for_review https://github.com/llvm/llvm-project/pull/141824 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
atrosinenko wrote: > [!WARNING] > This pull request is not mergeable via GitHub because a downstack PR is > open. Once all requirements are satisfied, merge this PR as a stack href="https://app.graphite.dev/github/pr/llvm/llvm-project/141824?utm_source=stack-comment-downstack-mergeability-warning"; > >on Graphite. > https://graphite.dev/docs/merge-pull-requests";>Learn more * **#141824** https://app.graphite.dev/github/pr/llvm/llvm-project/141824?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> 👈 https://app.graphite.dev/github/pr/llvm/llvm-project/141824?utm_source=stack-comment-view-in-graphite"; target="_blank">(View in Graphite) * **#136183** https://app.graphite.dev/github/pr/llvm/llvm-project/136183?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/>: 1 other dependent PR ([#137224](https://github.com/llvm/llvm-project/pull/137224) https://app.graphite.dev/github/pr/llvm/llvm-project/137224?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/>) * **#136151** https://app.graphite.dev/github/pr/llvm/llvm-project/136151?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#135663** https://app.graphite.dev/github/pr/llvm/llvm-project/135663?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#136147** https://app.graphite.dev/github/pr/llvm/llvm-project/136147?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#135662** https://app.graphite.dev/github/pr/llvm/llvm-project/135662?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#135661** https://app.graphite.dev/github/pr/llvm/llvm-project/135661?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#134146** https://app.graphite.dev/github/pr/llvm/llvm-project/134146?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#133461** https://app.graphite.dev/github/pr/llvm/llvm-project/133461?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * **#135073** https://app.graphite.dev/github/pr/llvm/llvm-project/135073?utm_source=stack-comment-icon"; target="_blank">https://static.graphite.dev/graphite-32x32-black.png"; alt="Graphite" width="10px" height="10px"/> * `main` This stack of pull requests is managed by https://graphite.dev?utm-source=stack-comment";>Graphite. Learn more about https://stacking.dev/?utm_source=stack-comment";>stacking. https://github.com/llvm/llvm-project/pull/141824 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG (PR #141824)
https://github.com/atrosinenko created
https://github.com/llvm/llvm-project/pull/141824
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
>From 7d38c3ebb3dd7f67f87b494e2dfe6e6c4ca29787 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko
Date: Wed, 14 May 2025 23:12:13 +0300
Subject: [PATCH] [BOLT] Gadget scanner: fix LR to be safe in leaf functions
without CFG
After a label in a function without CFG information, use a reasonably
pessimistic estimation of register state (assume that any register that
can be clobbered in this function was actually clobbered) instead of the
most pessimistic "all registers are unsafe". This is the same estimation
as used by the dataflow variant of the analysis when the preceding
instruction is not known for sure.
Without this, leaf functions without CFG information are likely to have
false positive reports about non-protected return instructions, as
1) LR is unlikely to be signed and authenticated in a leaf function and
2) LR is likely to be used by a return instruction near the end of the
function and
3) the register state is likely to be reset at least once during the
linear scan through the function
---
bolt/lib/Passes/PAuthGadgetScanner.cpp| 14 +++--
.../AArch64/gs-pacret-autiasp.s | 31 +--
.../AArch64/gs-pauth-authentication-oracles.s | 20
.../AArch64/gs-pauth-debug-output.s | 30 ++
.../AArch64/gs-pauth-signing-oracles.s| 27
5 files changed, 29 insertions(+), 93 deletions(-)
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp
b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 2aacb38ee19a9..6327a2da54d5b 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -737,19 +737,14 @@ template class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
-// the same way basic blocks are processed by data-flow analysis, assuming
-// pessimistically that all registers are unsafe at the start of each sequence.
+// the same way basic blocks are processed by data-flow analysis, with the same
+// pessimistic estimation of the initial state at the start of each sequence
+// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;
- /// Creates a state with all registers marked unsafe (not to be confused
- /// with empty state).
- SrcState createUnsafeState() const {
-return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
- }
-
public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
}
void run() override {
+const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public
SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
-S = createUnsafeState();
+S = DefaultState;
}
// Attach the state *before* this instruction executes.
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index df0a83be00986..627f8eb20ab9c 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction
-// Expected false positive: without CFG, the state is reset to all-unsafe
-// after an unconditional branch.
-
-.globl state_is_reset_after_indirect_branch_nocfg
-.type state_is_reset_after_indirect_branch_nocfg,@function
-state_is_reset_after_indirect_branch_nocfg:
-// CHECK-LABEL: GS
