[CFI] Add a module flag to allow custom section on globals with !type metadata when not doing CFI

[kubamracek]

Summary

See the added testcase -- the problem happens when trying to apply VFE during LTO, but without CFI at all, and when having a custom section on a vtable global. In that case LowerTypeTestsPass rejects the IR because it enforces that all !type-having globals must not specify a custom section. However, in a non-CFI compilation, this requirement is too strict, using !type metadata for VFE does not have any such requirement.

This diff adds a module flag, "No CFI Section Enforcement", that allows custom sections on globals with !type metadata, and basically is a way of informing LowerTypeTestsPass that the compilation is not doing CFI. Suggestions for alternative solutions welcome!

[fhahn]

has this always been broken?

[fhahn]

In D111757#3064812, @fhahn wrote:

has this always been broken?

I guess it might be helpful to elaborate why it should be safe to ignore custom sections and/or when it would be safe for frontends to emit the new flag.

[kubamracek]

has this always been broken?

I guess it might be helpful to elaborate why it should be safe to ignore custom sections and/or when it would be safe for frontends to emit the new flag.

I think this "confusion" between CFI and VFE has always been there, as far as I can tell -- when doing VFE with no CFI. But vtables emitted by Clang never have a custom section specified, so I believe this problem can not be triggered from Clang.

would be safe for frontends to emit the new flag

My understanding is that if the frontend is not trying to enable CFI at all, *then* it's safe to emit the flag. Where would you like to see this explanation in source code? LangRef doesn't really try to document module flags... A comment instead?

[fhahn]

llvm/test/Transforms/GlobalDCE/virtual-functions-sections.ll

1

Shouldn't this test be in lvm/test/Transforms/LowerTypeTests/ and independent of -globaldce?

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

class LowerTypeTestsModule {
 
   void buildBitSetsFromGlobalVariables(ArrayRef<Metadata *> TypeIds,
                                        ArrayRef<GlobalTypeMember *> Globals);
   unsigned getJumpTableEntrySize();
   Type *getJumpTableEntryType();
   void createJumpTableEntry(raw_ostream &AsmOS, raw_ostream &ConstraintOS,
                             Triple::ArchType JumpTableArch,
                             SmallVectorImpl<Value *> &AsmArgs, Function *Dest);
-  void verifyTypeMDNode(GlobalObject *GO, MDNode *Type);
+  void verifyTypeMDNode(GlobalObject *GO, MDNode *Type,
+                        bool NoCfiSectionEnforcement);
   void buildBitSetsFromFunctions(ArrayRef<Metadata *> TypeIds,
                                  ArrayRef<GlobalTypeMember *> Functions);
   void buildBitSetsFromFunctionsNative(ArrayRef<Metadata *> TypeIds,
                                        ArrayRef<GlobalTypeMember *> Functions);
   void buildBitSetsFromFunctionsWASM(ArrayRef<Metadata *> TypeIds,
                                      ArrayRef<GlobalTypeMember *> Functions);
   void
   buildBitSetsFromDisjointSet(ArrayRef<Metadata *> TypeIds,
for (CallInst *CI : TIUI.CallSites) {
       if (Lowered) {
         CI->replaceAllUsesWith(Lowered);
         CI->eraseFromParent();
       }
     }
   }
 }
 
-void LowerTypeTestsModule::verifyTypeMDNode(GlobalObject *GO, MDNode *Type) {
+void LowerTypeTestsModule::verifyTypeMDNode(GlobalObject *GO, MDNode *Type,
+                                            bool NoCfiSectionEnforcement) {
   if (Type->getNumOperands() != 2)
     report_fatal_error("All operands of type metadata must have 2 elements");
 
   if (GO->isThreadLocal())
     report_fatal_error("Bit set element may not be thread-local");
-  if (isa<GlobalVariable>(GO) && GO->hasSection())
+  if (isa<GlobalVariable>(GO) && GO->hasSection() && !NoCfiSectionEnforcement)
     report_fatal_error(
         "A member of a type identifier may not have an explicit section");
 
   // FIXME: We previously checked that global var member of a type identifier
   // must be a definition, but the IR linker may leave type metadata on
   // declarations. We should restore this check after fixing PR31759.
 
   auto OffsetConstMD = dyn_cast<ConstantAsMetadata>(Type->getOperand(0));
if (CfiFunctionsMD) {
           for (unsigned I = 2; I < FuncMD->getNumOperands(); ++I)
             F->addMetadata(LLVMContext::MD_type,
                            *cast<MDNode>(FuncMD->getOperand(I).get()));
         }
       }
     }
   }
 
+  const bool NoCfiSectionEnforcement =
+      M.getModuleFlag("No CFI Section Enforcement") != nullptr;
+
   DenseMap<GlobalObject *, GlobalTypeMember *> GlobalTypeMembers;
   for (GlobalObject &GO : M.global_objects()) {
     if (isa<GlobalVariable>(GO) && GO.isDeclarationForLinker())
       continue;
 
     Types.clear();
     GO.getMetadata(LLVMContext::MD_type, Types);
 
if (Function *F = dyn_cast<Function>(&GO)) {
           continue;
       }
     }
 
     auto *GTM = GlobalTypeMember::create(Alloc, &GO, IsJumpTableCanonical,
                                          IsExported, Types);
     GlobalTypeMembers[&GO] = GTM;
     for (MDNode *Type : Types) {
-      verifyTypeMDNode(&GO, Type);
+      verifyTypeMDNode(&GO, Type, NoCfiSectionEnforcement);
       auto &Info = TypeIdInfo[Type->getOperand(1)];
       Info.UniqueId = ++CurUniqueId;
       Info.RefGlobals.push_back(GTM);
     }
   }
 
   auto AddTypeIdUse = [&](Metadata *TypeId) -> TypeIdUserInfo & {
     // Add the call site to the list of call sites for this type identifier. We

llvm/test/Transforms/GlobalDCE/virtual-functions-sections.ll

+; RUN: opt < %s -globaldce -lowertypetests -lowertypetests-summary-action=export -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+declare { i8*, i1 } @llvm.type.checked.load(i8*, i32, metadata)
+
+@vtable = internal unnamed_addr constant { [2 x i8*] } { [2 x i8*] [
+  i8* bitcast (void ()* @vfunc1_live to i8*),
+  i8* bitcast (void ()* @vfunc2_dead to i8*)
+]}, align 8, !type !0, !type !1, !vcall_visibility !{i64 2}, section "my_custom_section"
+!0 = !{i64 0, !"vfunc1.type"}
+!1 = !{i64 8, !"vfunc2.type"}
+
+define internal void @vfunc1_live() {
+  ; CHECK: define internal void @vfunc1_live(
+  ret void
+}
+
+define internal void @vfunc2_dead() {
+  ; CHECK-NOT: define internal void @vfunc2_dead(
+  ret void
+}
+
+define void @main() {
+  %1 = ptrtoint { [2 x i8*] }* @vtable to i64 ; to keep @vtable alive
+  %2 = tail call { i8*, i1 } @llvm.type.checked.load(i8* null, i32 0, metadata !"vfunc1.type")
+  ret void
+}
+
+!998 = !{i32 1, !"Virtual Function Elim", i32 1}
+!999 = !{i32 1, !"No CFI Section Enforcement", i32 1}
+!llvm.module.flags = !{!998, !999}