[ARM][AArch64] Change module flags values.

DanielKristofKiss · DanielKristofKiss · commit b89e0c7609d0 · 2024-03-09T16:49:07.000+01:00
Module flag used to indicate the feature to be propagated to the
function. As now the frontend emits all attributes accoringly let's
help the automerger to only do work when old and new bitcodes are
merged.
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1145,22 +1145,29 @@ void CodeGenModule::Release() {
                               "tag-stack-memory-buildattr", 1);
 
   if (T.isARM() || T.isThumb() || T.isAArch64()) {
+    // Previously 1 is used and meant for the backed to derive the function
+    // attribute form it. 2 now means function attributes already set for all
+    // functions in this module, so no need to propagate those from the module
+    // flag. Value is only used in case of LTO module merge because the backend
+    // will see all required function attribute set already. Value is used
+    // before modules got merged. Any posive value means the feature is active
+    // and required binary markings need to be emit accordingly.
     if (LangOpts.BranchTargetEnforcement)
       getModule().addModuleFlag(llvm::Module::Min, "branch-target-enforcement",
-                                1);
+                                2);
     if (LangOpts.BranchProtectionPAuthLR)
       getModule().addModuleFlag(llvm::Module::Min, "branch-protection-pauth-lr",
-                                1);
+                                2);
     if (LangOpts.GuardedControlStack)
-      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "guarded-control-stack", 2);
     if (LangOpts.hasSignReturnAddress())
-      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 1);
+      getModule().addModuleFlag(llvm::Module::Min, "sign-return-address", 2);
     if (LangOpts.isSignReturnAddressScopeAll())
       getModule().addModuleFlag(llvm::Module::Min, "sign-return-address-all",
-                                1);
+                                2);
     if (!LangOpts.isSignReturnAddressWithAKey())
       getModule().addModuleFlag(llvm::Module::Min,
-                                "sign-return-address-with-bkey", 1);
+                                "sign-return-address-with-bkey", 2);
   }
 
   if (CodeGenOpts.StackClashProtector)
diff --git a/clang/test/CodeGen/aarch64-sign-return-address.c b/clang/test/CodeGen/aarch64-sign-return-address.c
@@ -22,17 +22,17 @@
 // NONE-NOT:  !"branch-target-enforcement"
 // ALL-NOT:   !"branch-target-enforcement"
 // PART-NOT:  !"branch-target-enforcement"
-// BTE:       !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:       !{i32 8, !"branch-target-enforcement", i32 2}
 // B-KEY-NOT: !"branch-target-enforcement"
 
 // NONE-NOT:  !"sign-return-address"
-// ALL:   !{i32 8, !"sign-return-address", i32 1}
-// PART:  !{i32 8, !"sign-return-address", i32 1}
+// ALL:   !{i32 8, !"sign-return-address", i32 2}
+// PART:  !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:   !"sign-return-address"
-// B-KEY: !{i32 8, !"sign-return-address", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address", i32 2}
 
 // NONE-NOT:  !"sign-return-address-all"
-// ALL:   !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:   !{i32 8, !"sign-return-address-all", i32 2}
 // PART-NOT:  !"sign-return-address-all"
 // BTE-NOT:   !"sign-return-address-all"
 // B-KEY-NOT: !"sign-return-address-all"
@@ -41,6 +41,6 @@
 // ALL-NOT:   !"sign-return-address-with-bkey"
 // PART-NOT:  !"sign-return-address-with-bkey"
 // BTE-NOT:   !"sign-return-address-with-bkey"
-// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 1}
+// B-KEY: !{i32 8, !"sign-return-address-with-bkey", i32 2}
 
 void foo() {}
diff --git a/clang/test/CodeGen/arm-branch-protection-attr-2.c b/clang/test/CodeGen/arm-branch-protection-attr-2.c
@@ -18,16 +18,16 @@
 // NONE-NOT: !"branch-target-enforcement"
 // PART-NOT: !"branch-target-enforcement"
 // ALL-NOT:  !"branch-target-enforcement"
-// BTE:      !{i32 8, !"branch-target-enforcement", i32 1}
+// BTE:      !{i32 8, !"branch-target-enforcement", i32 2}
 
 // NONE-NOT: !"sign-return-address"
-// PART:     !{i32 8, !"sign-return-address", i32 1}
-// ALL:      !{i32 8, !"sign-return-address", i32 1}
+// PART:     !{i32 8, !"sign-return-address", i32 2}
+// ALL:      !{i32 8, !"sign-return-address", i32 2}
 // BTE-NOT:  !"sign-return-address"
 
 // NONE-NOT: !"sign-return-address-all", i32 0}
 // PART-NOT: !"sign-return-address-all", i32 0}
-// ALL:      !{i32 8, !"sign-return-address-all", i32 1}
+// ALL:      !{i32 8, !"sign-return-address-all", i32 2}
 // BTE-NOT:  !"sign-return-address-all", i32 0}
 
 void foo() {}
diff --git a/clang/test/Frontend/arm-ignore-branch-protection-option.c b/clang/test/Frontend/arm-ignore-branch-protection-option.c
@@ -15,4 +15,4 @@ __attribute__((target("arch=cortex-m0"))) void f() {}
 // CHECK-NOT:  attributes { {{.*}} "branch-target-enforcement"
 
 /// Check that there are branch protection module attributes despite the warning.
-// CHECK: !{i32 8, !"branch-target-enforcement", i32 1}
+// CHECK: !{i32 8, !"branch-target-enforcement", i32 2}
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5178,8 +5178,8 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
     Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
 }
 
-// Check if the module attribute is present and not zero.
-static bool isModuleAttributeSet(Module &M, const StringRef &ModAttr) {
+// Check if the module attribute is present and set to one.
+static bool isModuleAttributeOne(Module &M, const StringRef &ModAttr) {
   const auto *Attr =
       mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
   return Attr && Attr->isOne();
@@ -5197,31 +5197,36 @@ void llvm::CopyModuleAttrToFunctions(Module &M) {
   if (!T.isThumb() && !T.isARM() && !T.isAArch64())
     return;
 
-  StringRef SignTypeValue = "none";
-  if (isModuleAttributeSet(M, "sign-return-address-all"))
+  bool BTE = isModuleAttributeOne(M, "branch-target-enforcement");
+  bool BPPLR = isModuleAttributeOne(M, "branch-protection-pauth-lr");
+  bool GCS = isModuleAttributeOne(M, "guarded-control-stack");
+  bool SRA = isModuleAttributeOne(M, "sign-return-address");
+
+  if (!BTE && !BPPLR && !GCS && !SRA)
+    return;
+
+  StringRef SignTypeValue = "non-leaf";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-all"))
     SignTypeValue = "all";
-  else if (isModuleAttributeSet(M, "sign-return-address"))
-    SignTypeValue = "non-leaf";
-
-  StringRef BTEValue =
-      isModuleAttributeSet(M, "branch-target-enforcement") ? "true" : "false";
-  StringRef BPPLValue =
-      isModuleAttributeSet(M, "branch-protection-pauth-lr") ? "true" : "false";
-  StringRef GCSValue =
-      isModuleAttributeSet(M, "guarded-control-stack") ? "true" : "false";
-  StringRef SignKeyValue =
-      isModuleAttributeSet(M, "sign-return-address-with-bkey") ? "b_key"
-                                                               : "a_key";
+
+  StringRef SignKeyValue = "a_key";
+  if (SRA && isModuleAttributeOne(M, "sign-return-address-with-bkey"))
+    SignKeyValue = "b_key";
 
   for (Function &F : M.getFunctionList()) {
     if (F.isDeclaration())
       continue;
 
-    SetFunctionAttrIfNotSet(F, "sign-return-address", SignTypeValue);
-    SetFunctionAttrIfNotSet(F, "branch-target-enforcement", BTEValue);
-    SetFunctionAttrIfNotSet(F, "branch-protection-pauth-lr", BPPLValue);
-    SetFunctionAttrIfNotSet(F, "guarded-control-stack", GCSValue);
-    SetFunctionAttrIfNotSet(F, "sign-return-address-key", SignKeyValue);
+    if (SRA) {
+      SetFunctionAttrIfNotSet(F, "sign-return-address", SignTypeValue);
+      SetFunctionAttrIfNotSet(F, "sign-return-address-key", SignKeyValue);
+    }
+    if (BTE)
+      SetFunctionAttrIfNotSet(F, "branch-target-enforcement", "true");
+    if (BPPLR)
+      SetFunctionAttrIfNotSet(F, "branch-protection-pauth-lr", "true");
+    if (GCS)
+      SetFunctionAttrIfNotSet(F, "guarded-control-stack", "true");
   }
 }
 
diff --git a/llvm/test/LTO/AArch64/Inputs/bar.ll b/llvm/test/LTO/AArch64/Inputs/bar.ll
@@ -0,0 +1,22 @@
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+define dso_local void @bar() #0 {
+entry:
+  ret void
+}
+
+define dso_local void @baz() #1 {
+entry:
+  ret void
+}
+
+attributes #0 = { noinline nounwind optnone uwtable }
+attributes #1 = { noinline nounwind optnone uwtable "branch-target-enforcement"="true" }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 8, !"branch-target-enforcement", i32 2}
+!1 = !{i32 8, !"sign-return-address", i32 2}
+!2 = !{i32 8, !"sign-return-address-all", i32 2}
+!3 = !{i32 8, !"sign-return-address-with-bkey", i32 2}
diff --git a/llvm/test/LTO/AArch64/link-sign-return-address.ll b/llvm/test/LTO/AArch64/link-sign-return-address.ll
@@ -3,10 +3,11 @@
 ;
 ; RUN: llvm-as %s -o %t1.bc
 ; RUN: llvm-as %p/Inputs/foo.ll -o %t2.bc
+; RUN: llvm-as %p/Inputs/bar.ll -o %t3.bc
 ; RUN: llvm-lto -exported-symbol main \
 ; RUN:          -exported-symbol foo \
 ; RUN:          -filetype=obj \
-; RUN:           %t2.bc %t1.bc \
+; RUN:           %t3.bc %t2.bc %t1.bc \
 ; RUN:           -o %t1.exe 2>&1
 ; RUN: llvm-objdump -d %t1.exe | FileCheck --check-prefix=CHECK-DUMP %s
 ; RUN: llvm-readelf -n %t1.exe | FileCheck --allow-empty --check-prefix=CHECK-PROP %s
@@ -15,10 +16,14 @@ target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-unknown-linux-gnu"
 
 declare i32 @foo();
+declare void @baz();
+declare void @bar();
 
 define i32 @main() {
 entry:
   %add = call i32 @foo()
+  call void @bar()
+  call void @baz()
   ret i32 %add
 }
 
@@ -28,6 +33,12 @@ entry:
 !2 = !{i32 8, !"sign-return-address-all", i32 0}
 !3 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
 
+
+; CHECK-DUMP: <bar>:
+; CHECK-DUMP:     ret
+; CHECK-DUMP: <baz>:
+; CHECK-DUMP:     bti     c
+; CHECK-DUMP:     ret
 ; CHECK-DUMP: <foo>:
 ; CHECK-DUMP:     pacibsp
 ; CHECK-DUMP:     mov     w0, #0x2a
@@ -36,7 +47,9 @@ entry:
 ; CHECK-DUMP: <main>:
 ; CHECK-DUMP-NOT:  paciasp
 ; CHECK-DUMP:      str     x30,
-; CHECK-DUMP:      bl      0x14 <main+0x4>
+; CHECK-DUMP:      bl      0x20 <main+0x4>
+; CHECK-DUMP:      bl      0x0 <bar>
+; CHECK-DUMP:      bl      0x4 <baz>
 
 ; `main` doesn't support PAC sign-return-address while `foo` does, so in the binary
 ; we should not see anything.
diff --git a/llvm/test/Linker/link-arm-and-thumb.ll b/llvm/test/Linker/link-arm-and-thumb.ll
@@ -13,12 +13,11 @@ entry:
   ret i32 %add
 }
 
-; CHECK: define i32 @main() [[MAIN_ATTRS:#[0-9]+]]
+; CHECK: define i32 @main() {
 ; CHECK: define i32 @foo(i32 %a, i32 %b) [[ARM_ATTRS:#[0-9]+]]
 ; CHECK: define i32 @bar(i32 %a, i32 %b) [[THUMB_ATTRS:#[0-9]+]]
 
-; CHECK: attributes [[MAIN_ATTRS]] = { {{.*}} }
-; CHECK: attributes [[ARM_ATTRS]] = { {{.*}} "target-features"="-thumb-mode" }
-; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}} "target-features"="+thumb-mode" }
+; CHECK: attributes [[ARM_ATTRS]] = { {{.*}}"target-features"="-thumb-mode" }
+; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}}"target-features"="+thumb-mode" }
 
 ; STDERR-NOT: warning: Linking two modules of different target triples:
