[IR] Allow absence for Min module flags and make AArch64 BTI/PAC-RET flags backward compatible

[MaskRay]

Summary

D123493 introduced llvm::Module::Min to encode module flags metadata for AArch64
BTI/PAC-RET. llvm::Module::Min does not take effect when the flag is absent in
one module. This behavior is misleading and does not address backward
compatibility problems (when a bitcode with "branch-target-enforcement"==1 and
another without the flag are merged, the merge result is 1 instead of 0).

To address the problems, require Min flags to be non-negative and treat absence
as having a value of zero. For an old bitcode without
"branch-target-enforcement"/"sign-return-address", its value is as if 0.

[danielkiss]

Thanks, LGTM.

[MaskRay]

Update LangRef.rst
Add comment

llvm/lib/IR/Verifier.cpp

Verifier::visitModuleFlag(const MDNode *Op,
   switch (MFB) {
   case Module::Error:
   case Module::Warning:
   case Module::Override:
     // These behavior types accept any value.
     break;
 
   case Module::Min: {
-    Check(mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2)),
-          "invalid value for 'min' module flag (expected constant integer)",
+    auto *V = mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
+    Check(V && V->getValue().isNonNegative(),
+          "invalid value for 'min' module flag (expected constant non-negative "
+          "integer)",
           Op->getOperand(2));
     break;
   }
 
   case Module::Max: {
     Check(mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2)),
           "invalid value for 'max' module flag (expected constant integer)",
           Op->getOperand(2));

llvm/lib/Linker/IRMover.cpp

for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I)
       DstModFlags->addOperand(SrcModFlags->getOperand(I));
 
     return Error::success();
   }
 
   // First build a map of the existing module flags and requirements.
   DenseMap<MDString *, std::pair<MDNode *, unsigned>> Flags;
   SmallSetVector<MDNode *, 16> Requirements;
+  SmallVector<unsigned, 0> Mins;
+  DenseSet<MDString *> SeenMin;
   for (unsigned I = 0, E = DstModFlags->getNumOperands(); I != E; ++I) {
     MDNode *Op = DstModFlags->getOperand(I);
-    ConstantInt *Behavior = mdconst::extract<ConstantInt>(Op->getOperand(0));
+    uint64_t Behavior =
+        mdconst::extract<ConstantInt>(Op->getOperand(0))->getZExtValue();
     MDString *ID = cast<MDString>(Op->getOperand(1));
 
-    if (Behavior->getZExtValue() == Module::Require) {
+    if (Behavior == Module::Require) {
       Requirements.insert(cast<MDNode>(Op->getOperand(2)));
     } else {
+      if (Behavior == Module::Min)
+        Mins.push_back(I);
       Flags[ID] = std::make_pair(Op, I);
     }
   }
 
   // Merge in the flags from the source module, and also collect its set of
   // requirements.
   for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I) {
     MDNode *SrcOp = SrcModFlags->getOperand(I);
     ConstantInt *SrcBehavior =
         mdconst::extract<ConstantInt>(SrcOp->getOperand(0));
     MDString *ID = cast<MDString>(SrcOp->getOperand(1));
     MDNode *DstOp;
     unsigned DstIndex;
     std::tie(DstOp, DstIndex) = Flags.lookup(ID);
     unsigned SrcBehaviorValue = SrcBehavior->getZExtValue();
+    SeenMin.insert(ID);
 
     // If this is a requirement, add it and continue.
     if (SrcBehaviorValue == Module::Require) {
       // If the destination module does not already have this requirement, add
       // it.
       if (Requirements.insert(cast<MDNode>(SrcOp->getOperand(2)))) {
         DstModFlags->addOperand(SrcOp);
       }
       continue;
     }
 
     // If there is no existing flag with this ID, just add it.
     if (!DstOp) {
+      if (SrcBehaviorValue == Module::Min) {
+        Mins.push_back(DstModFlags->getNumOperands());
+        SeenMin.erase(ID);
+      }
       Flags[ID] = std::make_pair(SrcOp, DstModFlags->getNumOperands());
       DstModFlags->addOperand(SrcOp);
       continue;
     }
 
     // Otherwise, perform a merge.
     ConstantInt *DstBehavior =
         mdconst::extract<ConstantInt>(DstOp->getOperand(0));
case Module::AppendUnique: {
       for (auto I = DstValue->getNumOperands(); I < Elts.size(); I++)
         DstValue->push_back(Elts[I]);
       break;
     }
     }
 
   }
 
+  for (auto Idx : Mins) {
+    MDNode *Op = DstModFlags->getOperand(Idx);
+    MDString *ID = cast<MDString>(Op->getOperand(1));
+    if (!SeenMin.count(ID)) {
+      ConstantInt *V = mdconst::extract<ConstantInt>(Op->getOperand(2));
+      Metadata *FlagOps[] = {
+          Op->getOperand(0), ID,
+          ConstantAsMetadata::get(ConstantInt::get(V->getType(), 0))};
+      DstModFlags->setOperand(Idx, MDNode::get(DstM.getContext(), FlagOps));
+    }
+  }
+
   // Check all of the requirements.
   for (unsigned I = 0, E = Requirements.size(); I != E; ++I) {
     MDNode *Requirement = Requirements[I];
     MDString *Flag = cast<MDString>(Requirement->getOperand(0));
     Metadata *ReqValue = Requirement->getOperand(1);
 
     MDNode *Op = Flags[Flag].first;
     if (!Op || Op->getOperand(2) != ReqValue)

llvm/test/Linker/module-flags-min.ll

+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: llvm-link a.ll b.ll -S -o - 2>&1 | FileCheck %s
+
+; CHECK:      !0 = !{i32 8, !"foo", i32 2}
+; CHECK-NEXT: !1 = !{i32 8, !"bar", i64 3}
+; CHECK-NEXT: !2 = !{i32 8, !"only_in_a", i32 0}
+; CHECK-NEXT: !3 = !{i32 8, !"required_in_b", i32 3}
+; CHECK-NEXT: !4 = !{i32 8, !"only_in_b", i32 0}
+; CHECK-NEXT: !5 = !{i32 3, !"require", !6}
+; CHECK-NEXT: !6 = !{!"required_in_b", i32 3}
+
+;--- a.ll
+!0 = !{ i32 8, !"foo", i32 2 }
+!1 = !{ i32 8, !"bar", i64 4 }
+!2 = !{ i32 8, !"only_in_a", i32 4 }
+!3 = !{ i32 8, !"required_in_b", i32 3 }
+
+!llvm.module.flags = !{ !0, !1, !2, !3 }
+
+;--- b.ll
+!0 = !{ i32 8, !"foo", i32 3 }
+!1 = !{ i32 8, !"bar", i64 3 }
+!2 = !{ i32 8, !"only_in_b", i32 3 }
+!3 = !{ i32 8, !"required_in_b", i32 3 }
+!4 = !{ i32 3, !"require", !{!"required_in_b", i32 3} }
+
+!llvm.module.flags = !{ !0, !1, !2, !3, !4 }

llvm/test/Verifier/module-flags-1.ll

 ; CHECK-NOT: invalid value for 'append'-type module flag (expected a metadata node)
 !18 = !{i32 5, !"flag-4", !{i32 57}}
 
 ; Check that any 'max' module flags are valid.
 ; CHECK: invalid value for 'max' module flag (expected constant integer)
 !19 = !{i32 7, !"max", !"max"}
 
 ; Check that any 'min' module flags are valid.
-; CHECK: invalid value for 'min' module flag (expected constant integer)
+; CHECK: invalid value for 'min' module flag (expected constant non-negative integer)
 !20 = !{i32 8, !"min", !"min"}
+; CHECK: invalid value for 'min' module flag (expected constant non-negative integer)
+!21 = !{i32 8, !"min", i32 -1}
 
 ; Check that any 'require' module flags are valid.
 ; CHECK: invalid requirement on flag, flag is not present in module
 !11 = !{i32 3, !"bar", !{!"no-such-flag", i32 52}}
 ; CHECK: invalid requirement on flag, flag does not have the required value
 !12 = !{i32 1, !"flag-0", i32 53}
 !13 = !{i32 3, !"bar", !{!"flag-0", i32 54}}
 ; CHECK-NOT: invalid requirement on flag, flag is not present in module
 ; CHECK-NOT: invalid requirement on flag, flag does not have the required value
 !14 = !{i32 1, !"flag-1", i32 55}
 !15 = !{i32 3, !"bar", !{!"flag-1", i32 55}}
 
 !llvm.module.flags = !{
   !0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15,
-  !16, !17, !18, !19, !20 }
+  !16, !17, !18, !19, !20, !21 }