[llvm] 93164db - [llvm][AArch64] Fix BTI after returns_twice when call has no attributes

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 15 07:30:43 PST 2023


Author: David Spickett
Date: 2023-02-15T15:30:37Z
New Revision: 93164dba086df98d50bccf210e3e65115342c483

URL: https://github.com/llvm/llvm-project/commit/93164dba086df98d50bccf210e3e65115342c483
DIFF: https://github.com/llvm/llvm-project/commit/93164dba086df98d50bccf210e3e65115342c483.diff

LOG: [llvm][AArch64] Fix BTI after returns_twice when call has no attributes

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

Added: 
    

Modified: 
    llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
    llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
    llvm/test/CodeGen/AArch64/setjmp-bti.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index ba7b54aa6e48d..7f34970486979 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -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();
   }

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index 3b06fbee19a5a..e603414b3766b 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -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;

diff  --git a/llvm/test/CodeGen/AArch64/setjmp-bti.ll b/llvm/test/CodeGen/AArch64/setjmp-bti.ll
index 0f1940921e428..79a6bafffc8d8 100644
--- a/llvm/test/CodeGen/AArch64/setjmp-bti.ll
+++ b/llvm/test/CodeGen/AArch64/setjmp-bti.ll
@@ -18,8 +18,9 @@
 ;
 ; void bbb(void) {
 ;   setjmp(0);
+;   setjmp(0); // With the attributes removed.
 ;   int (*fnptr)(ptr) = setjmp;
-;   fnptr(0);
+;   fnptr(0); // With attributes added.
 ;   notsetjmp();
 ; }
 
@@ -27,6 +28,8 @@ 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
@@ -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
@@ -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
 }


        


More information about the llvm-commits mailing list