Skip to content

Commit

Permalink
[llvm][AArch64] Fix BTI after returns_twice when call has no attributes
Browse files Browse the repository at this point in the history
Previously we were looking for the returns twice attribute by manually
getting the function attributes from the call. This meant that we only
found attributes on the call itself, not what it was calling.

So if you had:
%call1 = call i32 @setjmp(ptr noundef null)

We would not BTI protect that even though setjmp clearly needs it.

Clang happens to produce:
%call = call i32 @setjmp(ptr noundef null) #0 ; returns_twice

So all valid calls were protected. This is not guaranteed,
the frontend may choose not to put attributes on the call.

It is undefined behaviour to call setjmp indirectly
(https://pubs.opengroup.org/onlinepubs/9699919799/functions/setjmp.html)
but as I misused the APIs here I think it's worth fixing up regardless.

Added comments to the test file where the IR differs from what
clang would output.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D144082
  • Loading branch information
DavidSpickett committed Feb 15, 2023
1 parent 2a2a6bf commit 93164db
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
Expand Up @@ -7173,7 +7173,7 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
bool IsSibCall = false;
bool GuardWithBTI = false;

if (CLI.CB && CLI.CB->getAttributes().hasFnAttr(Attribute::ReturnsTwice) &&
if (CLI.CB && CLI.CB->hasFnAttr(Attribute::ReturnsTwice) &&
!Subtarget->noBTIAtReturnTwice()) {
GuardWithBTI = FuncInfo->branchTargetEnforcement();
}
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
Expand Up @@ -1266,8 +1266,7 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
Opc = AArch64::BLR_RVMARKER;
// A call to a returns twice function like setjmp must be followed by a bti
// instruction.
else if (Info.CB &&
Info.CB->getAttributes().hasFnAttr(Attribute::ReturnsTwice) &&
else if (Info.CB && Info.CB->hasFnAttr(Attribute::ReturnsTwice) &&
!Subtarget.noBTIAtReturnTwice() &&
MF.getInfo<AArch64FunctionInfo>()->branchTargetEnforcement())
Opc = AArch64::BLR_BTI;
Expand Down
15 changes: 13 additions & 2 deletions llvm/test/CodeGen/AArch64/setjmp-bti.ll
Expand Up @@ -18,15 +18,18 @@
;
; void bbb(void) {
; setjmp(0);
; setjmp(0); // With the attributes removed.
; int (*fnptr)(ptr) = setjmp;
; fnptr(0);
; fnptr(0); // With attributes added.
; notsetjmp();
; }

define void @bbb() {
; BTI-LABEL: bbb:
; BTI: bl setjmp
; BTI-NEXT: hint #36
; BTI: bl setjmp
; BTI-NEXT: hint #36
; BTI: blr x{{[0-9]+}}
; BTI-NEXT: hint #36
; BTI: bl notsetjmp
Expand All @@ -35,6 +38,8 @@ define void @bbb() {
; BTISLS-LABEL: bbb:
; BTISLS: bl setjmp
; BTISLS-NEXT: hint #36
; BTISLS: bl setjmp
; BTISLS-NEXT: hint #36
; BTISLS: bl __llvm_slsblr_thunk_x{{[0-9]+}}
; BTISLS-NEXT: hint #36
; BTISLS: bl notsetjmp
Expand All @@ -43,16 +48,22 @@ define void @bbb() {
; NOBTI-LABEL: bbb:
; NOBTI: bl setjmp
; NOBTI-NOT: hint #36
; NOBTI: bl setjmp
; NOBTI-NOT: hint #36
; NOBTI: blr x{{[0-9]+}}
; NOBTI-NOT: hint #36
; NOBTI: bl notsetjmp
; NOBTI-NOT: hint #36
entry:
%fnptr = alloca ptr, align 8
; The frontend may apply attributes to the call, but it doesn't have to. We
; should be looking at the call base, which looks past that to the called function.
%call = call i32 @setjmp(ptr noundef null) #0
%call1 = call i32 @setjmp(ptr noundef null)
store ptr @setjmp, ptr %fnptr, align 8
%0 = load ptr, ptr %fnptr, align 8
%call1 = call i32 %0(ptr noundef null) #0
; Clang does not attach the attribute here but if it did, it should work.
%call2 = call i32 %0(ptr noundef null) #0
call void @notsetjmp()
ret void
}
Expand Down

0 comments on commit 93164db

Please sign in to comment.