Support for taking the max of module flags when linking, use for PIE/PIC

[tejohnson]

Summary

Add Max ModFlagBehavior, which can be used to take the max of two
module flag values when merging modules. Use it for the PIE and PIC
levels.

This avoids an error when we try to import from a module built -fpic
into a module built -fPIC, for example. For both PIE and PIC levels,
this will be legal, since the code generation gets more conservative
as the level is increased. Therefore we can take the max instead of
somehow trying to block importing between modules compiled with
different levels.

[pcc]

lib/IR/Verifier.cpp

1287

Please add test coverage for this verifier check.

[tejohnson]

Added test of new verifier error message

[pcc]

LGTM

include/llvm/IR/Module.h

enum ModFlagBehavior {
     /// Appends the two values, which are required to be metadata nodes.
     Append = 5,
 
     /// Appends the two values, which are required to be metadata
     /// nodes. However, duplicate entries in the second list are dropped
     /// during the append operation.
     AppendUnique = 6,
 
+    /// Takes the max of the two values, which are required to be integers.
+    Max = 7,
+
     // Markers:
     ModFlagBehaviorFirstVal = Error,
-    ModFlagBehaviorLastVal = AppendUnique
+    ModFlagBehaviorLastVal = Max
   };
 
   /// Checks if Metadata represents a valid ModFlagBehavior, and stores the
   /// converted result in MFB.
   static bool isValidModFlagBehavior(Metadata *MD, ModFlagBehavior &MFB);
 
   struct ModuleFlagEntry {
     ModFlagBehavior Behavior;

lib/IR/Module.cpp

PICLevel::Level Module::getPICLevel() const {
   if (!Val)
     return PICLevel::NotPIC;
 
   return static_cast<PICLevel::Level>(
       cast<ConstantInt>(Val->getValue())->getZExtValue());
 }
 
 void Module::setPICLevel(PICLevel::Level PL) {
-  addModuleFlag(ModFlagBehavior::Error, "PIC Level", PL);
+  addModuleFlag(ModFlagBehavior::Max, "PIC Level", PL);
 }
 
 PIELevel::Level Module::getPIELevel() const {
   auto *Val = cast_or_null<ConstantAsMetadata>(getModuleFlag("PIE Level"));
 
   if (!Val)
     return PIELevel::Default;
 
   return static_cast<PIELevel::Level>(
       cast<ConstantInt>(Val->getValue())->getZExtValue());
 }
 
 void Module::setPIELevel(PIELevel::Level PL) {
-  addModuleFlag(ModFlagBehavior::Error, "PIE Level", PL);
+  addModuleFlag(ModFlagBehavior::Max, "PIE Level", PL);
 }
 
 void Module::setProfileSummary(Metadata *M) {
   addModuleFlag(ModFlagBehavior::Error, "ProfileSummary", M);
 }
 
 Metadata *Module::getProfileSummary() {
   return getModuleFlag("ProfileSummary");

lib/IR/Verifier.cpp

Verifier::visitModuleFlag(const MDNode *Op,
   // Sanity check the values for behaviors with additional requirements.
   switch (MFB) {
   case Module::Error:
   case Module::Warning:
   case Module::Override:
     // These behavior types accept any value.
     break;
 
+  case Module::Max: {
+    Assert(mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2)),
+           "invalid value for 'max' module flag (expected constant integer)",
+           Op->getOperand(2));
+    break;
+  }
+
   case Module::Require: {
     // The value should itself be an MDNode with two operands, a flag ID (an
     // MDString), and a value.
     MDNode *Value = dyn_cast<MDNode>(Op->getOperand(2));
     Assert(Value && Value->getNumOperands() == 2,
            "invalid value for 'require' module flag (expected metadata pair)",
            Op->getOperand(2));
     Assert(isa<MDString>(Value->getOperand(0)),

lib/Linker/IRMover.cpp

if (!DstOp) {
       continue;
     }
 
     // Otherwise, perform a merge.
     ConstantInt *DstBehavior =
         mdconst::extract<ConstantInt>(DstOp->getOperand(0));
     unsigned DstBehaviorValue = DstBehavior->getZExtValue();
 
+    auto overrideDstValue = [&]() {
+      DstModFlags->setOperand(DstIndex, SrcOp);
+      Flags[ID].first = SrcOp;
+    };
+
     // If either flag has override behavior, handle it first.
     if (DstBehaviorValue == Module::Override) {
       // Diagnose inconsistent flags which both have override behavior.
       if (SrcBehaviorValue == Module::Override &&
           SrcOp->getOperand(2) != DstOp->getOperand(2))
         return stringErr("linking module flags '" + ID->getString() +
                          "': IDs have conflicting override values");
       continue;
     } else if (SrcBehaviorValue == Module::Override) {
       // Update the destination flag to that of the source.
-      DstModFlags->setOperand(DstIndex, SrcOp);
-      Flags[ID].first = SrcOp;
+      overrideDstValue();
       continue;
     }
 
     // Diagnose inconsistent merge behavior types.
     if (SrcBehaviorValue != DstBehaviorValue)
       return stringErr("linking module flags '" + ID->getString() +
                        "': IDs have conflicting behaviors");
 
for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I) {
     case Module::Warning: {
       // Emit a warning if the values differ.
       if (SrcOp->getOperand(2) != DstOp->getOperand(2)) {
         emitWarning("linking module flags '" + ID->getString() +
                     "': IDs have conflicting values");
       }
       continue;
     }
+    case Module::Max: {
+      ConstantInt *DstValue =
+          mdconst::extract<ConstantInt>(DstOp->getOperand(2));
+      ConstantInt *SrcValue =
+          mdconst::extract<ConstantInt>(SrcOp->getOperand(2));
+      if (SrcValue->getZExtValue() > DstValue->getZExtValue())
+        overrideDstValue();
+      break;
+    }
     case Module::Append: {
       MDNode *DstValue = cast<MDNode>(DstOp->getOperand(2));
       MDNode *SrcValue = cast<MDNode>(SrcOp->getOperand(2));
       SmallVector<Metadata *, 8> MDs;
       MDs.reserve(DstValue->getNumOperands() + SrcValue->getNumOperands());
       MDs.append(DstValue->op_begin(), DstValue->op_end());
       MDs.append(SrcValue->op_begin(), SrcValue->op_end());
 

test/Linker/Inputs/module-flags-pic-2-b.ll

-!0 = !{ i32 1, !"PIC Level", i32 2 }
+!0 = !{ i32 7, !"PIC Level", i32 2 }
+!1 = !{ i32 7, !"PIE Level", i32 2 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!0, !1}

test/Linker/module-flags-pic-2-a.ll

-; RUN: not llvm-link %s %p/Inputs/module-flags-pic-2-b.ll -S -o - 2> %t
-; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
+; RUN: llvm-link %s %p/Inputs/module-flags-pic-2-b.ll -S -o - | FileCheck %s
 
-; test linking modules with two different PIC levels
+; test linking modules with two different PIC and PIE levels
 
-!0 = !{ i32 1, !"PIC Level", i32 1 }
+!0 = !{ i32 7, !"PIC Level", i32 1 }
+!1 = !{ i32 7, !"PIE Level", i32 1 }
 
-!llvm.module.flags = !{!0}
+!llvm.module.flags = !{!0, !1}
 
-; CHECK-ERRORS: ERROR: linking module flags 'PIC Level': IDs have conflicting values
+; CHECK: !0 = !{i32 7, !"PIC Level", i32 2}
+; CHECK: !1 = !{i32 7, !"PIE Level", i32 2}

test/Verifier/module-flags-1.ll

 ; Check that any 'append'-type module flags are valid.
 ; CHECK: invalid value for 'append'-type module flag (expected a metadata node)
 !16 = !{i32 5, !"flag-2", i32 56}
 ; CHECK: invalid value for 'append'-type module flag (expected a metadata node)
 !17 = !{i32 5, !"flag-3", i32 57}
 ; 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 '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 }
+  !16, !17, !18, !19 }