Skip to content

Commit 337fae7

Browse files
committed
Address review comments
1 parent 43e04d8 commit 337fae7

File tree

3 files changed

+30
-5
lines changed

3 files changed

+30
-5
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,8 @@ class MCPlusBuilder {
610610
getRegUsedAsIndirectBranchDest(const MCInst &Inst,
611611
bool &IsAuthenticatedInternally) const {
612612
llvm_unreachable("not implemented");
613-
return 0;
613+
return 0; // Unreachable. A valid register should be returned by the
614+
// target implementation.
614615
}
615616

616617
/// Returns the register containing an address safely materialized by `Inst`

bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,6 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
753753
if (IsAuthenticated)
754754
return std::nullopt;
755755

756-
assert(*RetReg != BC.MIB->getNoRegister());
757756
LLVM_DEBUG({
758757
traceInst(BC, "Found RET inst", Inst);
759758
traceReg(BC, "RetReg", *RetReg);
@@ -779,7 +778,7 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
779778
if (IsAuthenticated)
780779
return std::nullopt;
781780

782-
assert(DestReg != BC.MIB->getNoRegister());
781+
assert(DestReg != BC.MIB->getNoRegister() && "Valid register expected");
783782
LLVM_DEBUG({
784783
traceInst(BC, "Found call inst", Inst);
785784
traceReg(BC, "Call destination reg", DestReg);
@@ -800,7 +799,6 @@ shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
800799
if (!SignedReg)
801800
return std::nullopt;
802801

803-
assert(*SignedReg != BC.MIB->getNoRegister());
804802
LLVM_DEBUG({
805803
traceInst(BC, "Found sign inst", Inst);
806804
traceReg(BC, "Signed reg", *SignedReg);

bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,11 +1000,37 @@ inst_ldraa_wb:
10001000
.type inst_ldrab_wb,@function
10011001
inst_ldrab_wb:
10021002
// CHECK-NOT: inst_ldrab_wb
1003-
ldraa x2, [x0]!
1003+
ldrab x2, [x0]!
10041004
pacda x0, x1
10051005
ret
10061006
.size inst_ldrab_wb, .-inst_ldrab_wb
10071007

1008+
// Non write-back forms of LDRA(A|B) instructions do not modify the address
1009+
// register, and thus do not make it safe.
1010+
1011+
.globl inst_ldraa_non_wb
1012+
.type inst_ldraa_non_wb,@function
1013+
inst_ldraa_non_wb:
1014+
// CHECK-LABEL: GS-PAUTH: signing oracle found in function inst_ldraa_non_wb, basic block {{[^,]+}}, at address
1015+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacdb x0, x1
1016+
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
1017+
ldraa x2, [x0]
1018+
pacdb x0, x1
1019+
ret
1020+
.size inst_ldraa_non_wb, .-inst_ldraa_non_wb
1021+
1022+
.globl inst_ldrab_non_wb
1023+
.type inst_ldrab_non_wb,@function
1024+
inst_ldrab_non_wb:
1025+
// CHECK-LABEL: GS-PAUTH: signing oracle found in function inst_ldrab_non_wb, basic block {{[^,]+}}, at address
1026+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1
1027+
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
1028+
autia x0, x3
1029+
ldrab x2, [x0]
1030+
pacda x0, x1
1031+
ret
1032+
.size inst_ldrab_non_wb, .-inst_ldrab_non_wb
1033+
10081034
.globl main
10091035
.type main,@function
10101036
main:

0 commit comments

Comments
 (0)