[llvm] [IR] Strengthen COFF IR verifier check (PR #159203)

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 17 14:50:53 PDT 2025


https://github.com/rnk updated https://github.com/llvm/llvm-project/pull/159203

>From d2396b5d6a390d294b1ce239a27bbe1dc241eda8 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at google.com>
Date: Tue, 16 Sep 2025 15:01:28 -0700
Subject: [PATCH 1/4] [IR] Strengthen COFF IR verifier check

Find issues like the whole-program-vtables one earlier.
---
 llvm/lib/IR/Verifier.cpp     | 145 ++++++++++++++++++++---------------
 llvm/test/Verifier/comdat.ll |  40 +++++++++-
 2 files changed, 120 insertions(+), 65 deletions(-)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index c06b60fd2d9a9..0f47049a6457d 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -271,7 +271,7 @@ struct VerifierSupport {
   }
 
   template <typename T1, typename... Ts>
-  void WriteTs(const T1 &V1, const Ts &... Vs) {
+  void WriteTs(const T1 &V1, const Ts &...Vs) {
     Write(V1);
     WriteTs(Vs...);
   }
@@ -294,7 +294,7 @@ struct VerifierSupport {
   /// This calls the Message-only version so that the above is easier to set a
   /// breakpoint on.
   template <typename T1, typename... Ts>
-  void CheckFailed(const Twine &Message, const T1 &V1, const Ts &... Vs) {
+  void CheckFailed(const Twine &Message, const T1 &V1, const Ts &...Vs) {
     CheckFailed(Message);
     if (OS)
       WriteTs(V1, Vs...);
@@ -311,7 +311,7 @@ struct VerifierSupport {
   /// A debug info check failed (with values to print).
   template <typename T1, typename... Ts>
   void DebugInfoCheckFailed(const Twine &Message, const T1 &V1,
-                            const Ts &... Vs) {
+                            const Ts &...Vs) {
     DebugInfoCheckFailed(Message);
     if (OS)
       WriteTs(V1, Vs...);
@@ -715,7 +715,8 @@ void Verifier::visit(Instruction &I) {
   InstVisitor<Verifier>::visit(I);
 }
 
-// Helper to iterate over indirect users. By returning false, the callback can ask to stop traversing further.
+// Helper to iterate over indirect users. By returning false, the callback can
+// ask to stop traversing further.
 static void forEachUser(const Value *User,
                         SmallPtrSet<const Value *, 32> &Visited,
                         llvm::function_ref<bool(const Value *)> Callback) {
@@ -724,7 +725,7 @@ static void forEachUser(const Value *User,
 
   SmallVector<const Value *> WorkList(User->materialized_users());
   while (!WorkList.empty()) {
-   const Value *Cur = WorkList.pop_back_val();
+    const Value *Cur = WorkList.pop_back_val();
     if (!Visited.insert(Cur).second)
       continue;
     if (Callback(Cur))
@@ -876,8 +877,8 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
     }
   }
 
-  if (GV.hasName() && (GV.getName() == "llvm.used" ||
-                       GV.getName() == "llvm.compiler.used")) {
+  if (GV.hasName() &&
+      (GV.getName() == "llvm.used" || GV.getName() == "llvm.compiler.used")) {
     Check(!GV.hasInitializer() || GV.hasAppendingLinkage(),
           "invalid linkage for intrinsic global variable", &GV);
     Check(GV.materialized_use_empty(),
@@ -936,13 +937,14 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
 }
 
 void Verifier::visitAliaseeSubExpr(const GlobalAlias &GA, const Constant &C) {
-  SmallPtrSet<const GlobalAlias*, 4> Visited;
+  SmallPtrSet<const GlobalAlias *, 4> Visited;
   Visited.insert(&GA);
   visitAliaseeSubExpr(Visited, GA, C);
 }
 
-void Verifier::visitAliaseeSubExpr(SmallPtrSetImpl<const GlobalAlias*> &Visited,
-                                   const GlobalAlias &GA, const Constant &C) {
+void Verifier::visitAliaseeSubExpr(
+    SmallPtrSetImpl<const GlobalAlias *> &Visited, const GlobalAlias &GA,
+    const Constant &C) {
   if (GA.hasAvailableExternallyLinkage()) {
     Check(isa<GlobalValue>(C) &&
               cast<GlobalValue>(C).hasAvailableExternallyLinkage(),
@@ -1760,12 +1762,20 @@ void Verifier::visitDIImportedEntity(const DIImportedEntity &N) {
 }
 
 void Verifier::visitComdat(const Comdat &C) {
-  // In COFF the Module is invalid if the GlobalValue has private linkage.
-  // Entities with private linkage don't have entries in the symbol table.
-  if (TT.isOSBinFormatCOFF())
-    if (const GlobalValue *GV = M.getNamedValue(C.getName()))
+  // Comdats in COFF modules must have a corresponding global that is not
+  // private. If the global is private, there will be no symbol table entry.
+  // We consider unused or dead comdat entries to be valid, since they won't
+  // make it into the COFF object file.
+  if (TT.isOSBinFormatCOFF()) {
+    const GlobalValue *GV = M.getNamedValue(C.getName());
+    bool IsDefined = GV != nullptr && !GV->isDeclarationForLinker();
+    Check(IsDefined || C.getUsers().empty(),
+          "COFF comdats must have a defined global value with the same name",
+          GV);
+    if (IsDefined)
       Check(!GV->hasPrivateLinkage(), "comdat global value has private linkage",
             GV);
+  }
 }
 
 void Verifier::visitModuleIdents() {
@@ -1805,11 +1815,12 @@ void Verifier::visitModuleCommandLines() {
 
 void Verifier::visitModuleFlags() {
   const NamedMDNode *Flags = M.getModuleFlagsMetadata();
-  if (!Flags) return;
+  if (!Flags)
+    return;
 
   // Scan each flag, and track the flags and requirements.
-  DenseMap<const MDString*, const MDNode*> SeenIDs;
-  SmallVector<const MDNode*, 16> Requirements;
+  DenseMap<const MDString *, const MDNode *> SeenIDs;
+  SmallVector<const MDNode *, 16> Requirements;
   uint64_t PAuthABIPlatform = -1;
   uint64_t PAuthABIVersion = -1;
   for (const MDNode *MDN : Flags->operands()) {
@@ -1854,10 +1865,9 @@ void Verifier::visitModuleFlags() {
   }
 }
 
-void
-Verifier::visitModuleFlag(const MDNode *Op,
-                          DenseMap<const MDString *, const MDNode *> &SeenIDs,
-                          SmallVectorImpl<const MDNode *> &Requirements) {
+void Verifier::visitModuleFlag(
+    const MDNode *Op, DenseMap<const MDString *, const MDNode *> &SeenIDs,
+    SmallVectorImpl<const MDNode *> &Requirements) {
   // Each module flag should have three arguments, the merge behavior (a
   // constant int), the flag ID (an MDString), and the value.
   Check(Op->getNumOperands() == 3,
@@ -1936,8 +1946,8 @@ Verifier::visitModuleFlag(const MDNode *Op,
   }
 
   if (ID->getString() == "wchar_size") {
-    ConstantInt *Value
-      = mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
+    ConstantInt *Value =
+        mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
     Check(Value, "wchar_size metadata requires constant integer argument");
   }
 
@@ -2099,7 +2109,8 @@ void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
     if (!Attr.isStringAttribute() &&
         IncompatibleAttrs.contains(Attr.getKindAsEnum())) {
       CheckFailed("Attribute '" + Attr.getAsString() +
-                  "' applied to incompatible type!", V);
+                      "' applied to incompatible type!",
+                  V);
       return;
     }
   }
@@ -2335,15 +2346,15 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
   }
 
   Check(!Attrs.hasAttrSomewhere(Attribute::Writable) ||
-        isModSet(Attrs.getMemoryEffects().getModRef(IRMemLocation::ArgMem)),
+            isModSet(Attrs.getMemoryEffects().getModRef(IRMemLocation::ArgMem)),
         "Attribute writable and memory without argmem: write are incompatible!",
         V);
 
   if (Attrs.hasFnAttr("aarch64_pstate_sm_enabled")) {
     Check(!Attrs.hasFnAttr("aarch64_pstate_sm_compatible"),
-           "Attributes 'aarch64_pstate_sm_enabled and "
-           "aarch64_pstate_sm_compatible' are incompatible!",
-           V);
+          "Attributes 'aarch64_pstate_sm_enabled and "
+          "aarch64_pstate_sm_compatible' are incompatible!",
+          V);
   }
 
   Check((Attrs.hasFnAttr("aarch64_new_za") + Attrs.hasFnAttr("aarch64_in_za") +
@@ -2724,7 +2735,8 @@ void Verifier::verifyStatepoint(const CallBase &Call) {
   Check(TargetFuncType,
         "gc.statepoint callee elementtype must be function type", Call);
 
-  const int NumCallArgs = cast<ConstantInt>(Call.getArgOperand(3))->getZExtValue();
+  const int NumCallArgs =
+      cast<ConstantInt>(Call.getArgOperand(3))->getZExtValue();
   Check(NumCallArgs >= 0,
         "gc.statepoint number of arguments to underlying call "
         "must be positive",
@@ -2743,8 +2755,8 @@ void Verifier::verifyStatepoint(const CallBase &Call) {
     Check(NumCallArgs == NumParams,
           "gc.statepoint mismatch in number of call args", Call);
 
-  const uint64_t Flags
-    = cast<ConstantInt>(Call.getArgOperand(4))->getZExtValue();
+  const uint64_t Flags =
+      cast<ConstantInt>(Call.getArgOperand(4))->getZExtValue();
   Check((Flags & ~(uint64_t)StatepointFlags::MaskAll) == 0,
         "unknown flag used in gc.statepoint flags argument", Call);
 
@@ -3241,7 +3253,7 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
   // it.
   if (isa<PHINode>(BB.front())) {
     SmallVector<BasicBlock *, 8> Preds(predecessors(&BB));
-    SmallVector<std::pair<BasicBlock*, Value*>, 8> Values;
+    SmallVector<std::pair<BasicBlock *, Value *>, 8> Values;
     llvm::sort(Preds);
     for (const PHINode &PN : BB.phis()) {
       Check(PN.getNumIncomingValues() == Preds.size(),
@@ -3278,8 +3290,7 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
   }
 
   // Check that all instructions have their parent pointers set up correctly.
-  for (auto &I : BB)
-  {
+  for (auto &I : BB) {
     Check(I.getParent() == &BB, "Instruction has bogus parent pointer!");
   }
 
@@ -3327,7 +3338,7 @@ void Verifier::visitSwitchInst(SwitchInst &SI) {
   // Check to make sure that all of the constants in the switch instruction
   // have the same type as the switched-on value.
   Type *SwitchTy = SI.getCondition()->getType();
-  SmallPtrSet<ConstantInt*, 32> Constants;
+  SmallPtrSet<ConstantInt *, 32> Constants;
   for (auto &Case : SI.cases()) {
     Check(isa<ConstantInt>(SI.getOperand(Case.getCaseIndex() * 2 + 2)),
           "Case value is not a constant integer.", &SI);
@@ -3754,7 +3765,8 @@ void Verifier::visitCallBase(CallBase &Call) {
   for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
     if (Call.paramHasAttr(i, Attribute::SwiftError)) {
       Value *SwiftErrorArg = Call.getArgOperand(i);
-      if (auto AI = dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
+      if (auto AI =
+              dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
         Check(AI->isSwiftError(),
               "swifterror argument for call has mismatched alloca", AI, Call);
         continue;
@@ -3999,7 +4011,8 @@ static bool isTypeCongruent(Type *L, Type *R) {
   return PL->getAddressSpace() == PR->getAddressSpace();
 }
 
-static AttrBuilder getParameterABIAttributes(LLVMContext& C, unsigned I, AttributeList Attrs) {
+static AttrBuilder getParameterABIAttributes(LLVMContext &C, unsigned I,
+                                             AttributeList Attrs) {
   static const Attribute::AttrKind ABIAttrs[] = {
       Attribute::StructRet,  Attribute::ByVal,          Attribute::InAlloca,
       Attribute::InReg,      Attribute::StackAlignment, Attribute::SwiftSelf,
@@ -4067,12 +4080,14 @@ void Verifier::verifyMustTailCall(CallInst &CI) {
     // - Only sret, byval, swiftself, and swiftasync ABI-impacting attributes
     //   are allowed in swifttailcc call
     for (unsigned I = 0, E = CallerTy->getNumParams(); I != E; ++I) {
-      AttrBuilder ABIAttrs = getParameterABIAttributes(F->getContext(), I, CallerAttrs);
+      AttrBuilder ABIAttrs =
+          getParameterABIAttributes(F->getContext(), I, CallerAttrs);
       SmallString<32> Context{CCName, StringRef(" musttail caller")};
       verifyTailCCMustTailAttrs(ABIAttrs, Context);
     }
     for (unsigned I = 0, E = CalleeTy->getNumParams(); I != E; ++I) {
-      AttrBuilder ABIAttrs = getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
+      AttrBuilder ABIAttrs =
+          getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
       SmallString<32> Context{CCName, StringRef(" musttail callee")};
       verifyTailCCMustTailAttrs(ABIAttrs, Context);
     }
@@ -4098,8 +4113,10 @@ void Verifier::verifyMustTailCall(CallInst &CI) {
   // - All ABI-impacting function attributes, such as sret, byval, inreg,
   //   returned, preallocated, and inalloca, must match.
   for (unsigned I = 0, E = CallerTy->getNumParams(); I != E; ++I) {
-    AttrBuilder CallerABIAttrs = getParameterABIAttributes(F->getContext(), I, CallerAttrs);
-    AttrBuilder CalleeABIAttrs = getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
+    AttrBuilder CallerABIAttrs =
+        getParameterABIAttributes(F->getContext(), I, CallerAttrs);
+    AttrBuilder CalleeABIAttrs =
+        getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
     Check(CallerABIAttrs == CalleeABIAttrs,
           "cannot guarantee tail call due to mismatched ABI impacting "
           "function attributes",
@@ -4491,7 +4508,7 @@ void Verifier::verifySwiftErrorValue(const Value *SwiftErrorVal) {
 
 void Verifier::visitAllocaInst(AllocaInst &AI) {
   Type *Ty = AI.getAllocatedType();
-  SmallPtrSet<Type*, 4> Visited;
+  SmallPtrSet<Type *, 4> Visited;
   Check(Ty->isSized(&Visited), "Cannot allocate unsized type", &AI);
   // Check if it's a target extension type that disallows being used on the
   // stack.
@@ -4541,7 +4558,8 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
   } else if (AtomicRMWInst::isFPOperation(Op)) {
     Check(ElTy->isFPOrFPVectorTy() && !isa<ScalableVectorType>(ElTy),
           "atomicrmw " + AtomicRMWInst::getOperationName(Op) +
-              " operand must have floating-point or fixed vector of floating-point "
+              " operand must have floating-point or fixed vector of "
+              "floating-point "
               "type!",
           &RMWI, ElTy);
   } else {
@@ -5020,7 +5038,7 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
   Check(DT.dominates(Op, U), "Instruction does not dominate all uses!", Op, &I);
 }
 
-void Verifier::visitDereferenceableMetadata(Instruction& I, MDNode* MD) {
+void Verifier::visitDereferenceableMetadata(Instruction &I, MDNode *MD) {
   Check(I.getType()->isPointerTy(),
         "dereferenceable, dereferenceable_or_null "
         "apply only to pointer types",
@@ -5353,7 +5371,7 @@ void Verifier::visitInstruction(Instruction &I) {
   BasicBlock *BB = I.getParent();
   Check(BB, "Instruction not embedded in basic block!", &I);
 
-  if (!isa<PHINode>(I)) {   // Check that non-phi nodes are not self referential
+  if (!isa<PHINode>(I)) { // Check that non-phi nodes are not self referential
     for (User *U : I.users()) {
       Check(U != (User *)&I || !DT.isReachableFromEntry(BB),
             "Only PHI nodes may reference their own value!", &I);
@@ -7097,7 +7115,8 @@ void Verifier::visitVPIntrinsic(VPIntrinsic &VPI) {
     case Intrinsic::vp_llrint:
       Check(
           RetTy->isIntOrIntVectorTy() && ValTy->isFPOrFPVectorTy(),
-          "llvm.vp.fptoui, llvm.vp.fptosi, llvm.vp.lrint or llvm.vp.llrint" "intrinsic first argument element "
+          "llvm.vp.fptoui, llvm.vp.fptosi, llvm.vp.lrint or llvm.vp.llrint"
+          "intrinsic first argument element "
           "type must be floating-point and result element type must be integer",
           *VPCast);
       break;
@@ -7587,8 +7606,7 @@ struct VerifierLegacyPass : public FunctionPass {
     initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
   }
   explicit VerifierLegacyPass(bool FatalErrors)
-      : FunctionPass(ID),
-        FatalErrors(FatalErrors) {
+      : FunctionPass(ID), FatalErrors(FatalErrors) {
     initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
   }
 
@@ -7626,7 +7644,7 @@ struct VerifierLegacyPass : public FunctionPass {
 } // end anonymous namespace
 
 /// Helper to issue failure from the TBAA verification
-template <typename... Tys> void TBAAVerifier::CheckFailed(Tys &&... Args) {
+template <typename... Tys> void TBAAVerifier::CheckFailed(Tys &&...Args) {
   if (Diagnostic)
     return Diagnostic->CheckFailed(Args...);
 }
@@ -7676,7 +7694,8 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
   if (IsNewFormat) {
     if (BaseNode->getNumOperands() % 3 != 0) {
       CheckFailed("Access tag nodes must have the number of operands that is a "
-                  "multiple of 3!", BaseNode);
+                  "multiple of 3!",
+                  BaseNode);
       return InvalidNode;
     }
   } else {
@@ -7689,8 +7708,8 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
 
   // Check the type size field.
   if (IsNewFormat) {
-    auto *TypeSizeNode = mdconst::dyn_extract_or_null<ConstantInt>(
-        BaseNode->getOperand(1));
+    auto *TypeSizeNode =
+        mdconst::dyn_extract_or_null<ConstantInt>(BaseNode->getOperand(1));
     if (!TypeSizeNode) {
       CheckFailed("Type size nodes must be constants!", &I, BaseNode);
       return InvalidNode;
@@ -7714,7 +7733,7 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
   unsigned FirstFieldOpNo = IsNewFormat ? 3 : 1;
   unsigned NumOpsPerField = IsNewFormat ? 3 : 2;
   for (unsigned Idx = FirstFieldOpNo; Idx < BaseNode->getNumOperands();
-           Idx += NumOpsPerField) {
+       Idx += NumOpsPerField) {
     const MDOperand &FieldTy = BaseNode->getOperand(Idx);
     const MDOperand &FieldOffset = BaseNode->getOperand(Idx + 1);
     if (!isa<MDNode>(FieldTy)) {
@@ -7828,7 +7847,7 @@ MDNode *TBAAVerifier::getFieldNodeFromTBAABaseNode(Instruction &I,
   unsigned FirstFieldOpNo = IsNewFormat ? 3 : 1;
   unsigned NumOpsPerField = IsNewFormat ? 3 : 2;
   for (unsigned Idx = FirstFieldOpNo; Idx < BaseNode->getNumOperands();
-           Idx += NumOpsPerField) {
+       Idx += NumOpsPerField) {
     auto *OffsetEntryCI =
         mdconst::extract<ConstantInt>(BaseNode->getOperand(Idx + 1));
     if (OffsetEntryCI->getValue().ugt(Offset)) {
@@ -7847,8 +7866,8 @@ MDNode *TBAAVerifier::getFieldNodeFromTBAABaseNode(Instruction &I,
   }
 
   unsigned LastIdx = BaseNode->getNumOperands() - NumOpsPerField;
-  auto *LastOffsetEntryCI = mdconst::extract<ConstantInt>(
-      BaseNode->getOperand(LastIdx + 1));
+  auto *LastOffsetEntryCI =
+      mdconst::extract<ConstantInt>(BaseNode->getOperand(LastIdx + 1));
   Offset -= LastOffsetEntryCI->getValue();
   return cast<MDNode>(BaseNode->getOperand(LastIdx));
 }
@@ -7893,8 +7912,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
 
   // Check the access size field.
   if (IsNewFormat) {
-    auto *AccessSizeNode = mdconst::dyn_extract_or_null<ConstantInt>(
-        MD->getOperand(3));
+    auto *AccessSizeNode =
+        mdconst::dyn_extract_or_null<ConstantInt>(MD->getOperand(3));
     CheckTBAA(AccessSizeNode, "Access size field must be a constant", &I, MD);
   }
 
@@ -7932,8 +7951,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
   SmallPtrSet<MDNode *, 4> StructPath;
 
   for (/* empty */; BaseNode && !IsRootTBAANode(BaseNode);
-       BaseNode = getFieldNodeFromTBAABaseNode(I, BaseNode, Offset,
-                                               IsNewFormat)) {
+       BaseNode =
+           getFieldNodeFromTBAABaseNode(I, BaseNode, Offset, IsNewFormat)) {
     if (!StructPath.insert(BaseNode).second) {
       CheckFailed("Cycle detected in struct path", &I, MD);
       return false;
@@ -7941,8 +7960,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
 
     bool Invalid;
     unsigned BaseNodeBitWidth;
-    std::tie(Invalid, BaseNodeBitWidth) = verifyTBAABaseNode(I, BaseNode,
-                                                             IsNewFormat);
+    std::tie(Invalid, BaseNodeBitWidth) =
+        verifyTBAABaseNode(I, BaseNode, IsNewFormat);
 
     // If the base node is invalid in itself, then we've already printed all the
     // errors we wanted to print.
@@ -7987,7 +8006,7 @@ VerifierAnalysis::Result VerifierAnalysis::run(Module &M,
 
 VerifierAnalysis::Result VerifierAnalysis::run(Function &F,
                                                FunctionAnalysisManager &) {
-  return { llvm::verifyFunction(F, &dbgs()), false };
+  return {llvm::verifyFunction(F, &dbgs()), false};
 }
 
 PreservedAnalyses VerifierPass::run(Module &M, ModuleAnalysisManager &AM) {
diff --git a/llvm/test/Verifier/comdat.ll b/llvm/test/Verifier/comdat.ll
index dcf67d89f8d7d..84f401a321480 100644
--- a/llvm/test/Verifier/comdat.ll
+++ b/llvm/test/Verifier/comdat.ll
@@ -1,5 +1,41 @@
-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: split-file %s %t
 
+;--- common.ll
+; RUN: not llvm-as %t/common.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-COMMON
 $v = comdat any
 @v = common global i32 0, comdat($v)
-; CHECK: 'common' global may not be in a Comdat!
+; CHECK-COMMON: 'common' global may not be in a Comdat!
+
+;--- private.ll
+; RUN: llvm-as %t/private.ll -o /dev/null
+; RUN: opt -mtriple=x86_64-unknown-linux %t/private.ll -o /dev/null
+; RUN: not opt -mtriple=x86_64-pc-win32 %t/private.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-PRIVATE
+$v = comdat any
+ at v = private global i32 0, comdat($v)
+; CHECK-PRIVATE: comdat global value has private linkage
+
+;--- noleader.ll
+; RUN: llvm-as %t/noleader.ll -o /dev/null
+; RUN: opt -mtriple=x86_64-unknown-linux %t/noleader.ll -o /dev/null
+; RUN: not opt -mtriple=x86_64-pc-win32 %t/noleader.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-NOLEADER
+
+$v = comdat any
+ at unrelated = internal global i32 0, comdat($v)
+; CHECK-NOLEADER: COFF comdats must have a defined global value with the same name
+
+;--- undefined.ll
+; RUN: llvm-as %t/undefined.ll -o /dev/null
+; RUN: opt -mtriple=x86_64-unknown-linux %t/undefined.ll -o /dev/null
+; RUN: not opt -mtriple=x86_64-pc-win32 %t/undefined.ll -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED
+
+$v = comdat any
+ at v = external global i32
+ at unrelated = internal global i32 0, comdat($v)
+; CHECK-UNDEFINED: COFF comdats must have a defined global value with the same name
+
+;--- largest.ll
+; RUN: llvm-as %t/largest.ll -o /dev/null
+; This used to be invalid, but now it's valid.  Ensure the verifier
+; doesn't reject it.
+
+$v = comdat largest

>From 7e22cd2850ad15234f3868ca5e85edb4ab9a1a41 Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at google.com>
Date: Wed, 17 Sep 2025 13:06:35 -0700
Subject: [PATCH 2/4] Revert unintended verifier format changes and delete
 stale test cases that were folded into comdat.ll

---
 llvm/lib/IR/Verifier.cpp      | 129 ++++++++++++++++------------------
 llvm/test/Verifier/comdat2.ll |   7 --
 llvm/test/Verifier/comdat3.ll |   5 --
 3 files changed, 59 insertions(+), 82 deletions(-)
 delete mode 100644 llvm/test/Verifier/comdat2.ll
 delete mode 100644 llvm/test/Verifier/comdat3.ll

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 0f47049a6457d..3c176b42ad3ae 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -271,7 +271,7 @@ struct VerifierSupport {
   }
 
   template <typename T1, typename... Ts>
-  void WriteTs(const T1 &V1, const Ts &...Vs) {
+  void WriteTs(const T1 &V1, const Ts &... Vs) {
     Write(V1);
     WriteTs(Vs...);
   }
@@ -294,7 +294,7 @@ struct VerifierSupport {
   /// This calls the Message-only version so that the above is easier to set a
   /// breakpoint on.
   template <typename T1, typename... Ts>
-  void CheckFailed(const Twine &Message, const T1 &V1, const Ts &...Vs) {
+  void CheckFailed(const Twine &Message, const T1 &V1, const Ts &... Vs) {
     CheckFailed(Message);
     if (OS)
       WriteTs(V1, Vs...);
@@ -311,7 +311,7 @@ struct VerifierSupport {
   /// A debug info check failed (with values to print).
   template <typename T1, typename... Ts>
   void DebugInfoCheckFailed(const Twine &Message, const T1 &V1,
-                            const Ts &...Vs) {
+                            const Ts &... Vs) {
     DebugInfoCheckFailed(Message);
     if (OS)
       WriteTs(V1, Vs...);
@@ -715,8 +715,7 @@ void Verifier::visit(Instruction &I) {
   InstVisitor<Verifier>::visit(I);
 }
 
-// Helper to iterate over indirect users. By returning false, the callback can
-// ask to stop traversing further.
+// Helper to iterate over indirect users. By returning false, the callback can ask to stop traversing further.
 static void forEachUser(const Value *User,
                         SmallPtrSet<const Value *, 32> &Visited,
                         llvm::function_ref<bool(const Value *)> Callback) {
@@ -725,7 +724,7 @@ static void forEachUser(const Value *User,
 
   SmallVector<const Value *> WorkList(User->materialized_users());
   while (!WorkList.empty()) {
-    const Value *Cur = WorkList.pop_back_val();
+   const Value *Cur = WorkList.pop_back_val();
     if (!Visited.insert(Cur).second)
       continue;
     if (Callback(Cur))
@@ -877,8 +876,8 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
     }
   }
 
-  if (GV.hasName() &&
-      (GV.getName() == "llvm.used" || GV.getName() == "llvm.compiler.used")) {
+  if (GV.hasName() && (GV.getName() == "llvm.used" ||
+                       GV.getName() == "llvm.compiler.used")) {
     Check(!GV.hasInitializer() || GV.hasAppendingLinkage(),
           "invalid linkage for intrinsic global variable", &GV);
     Check(GV.materialized_use_empty(),
@@ -937,14 +936,13 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
 }
 
 void Verifier::visitAliaseeSubExpr(const GlobalAlias &GA, const Constant &C) {
-  SmallPtrSet<const GlobalAlias *, 4> Visited;
+  SmallPtrSet<const GlobalAlias*, 4> Visited;
   Visited.insert(&GA);
   visitAliaseeSubExpr(Visited, GA, C);
 }
 
-void Verifier::visitAliaseeSubExpr(
-    SmallPtrSetImpl<const GlobalAlias *> &Visited, const GlobalAlias &GA,
-    const Constant &C) {
+void Verifier::visitAliaseeSubExpr(SmallPtrSetImpl<const GlobalAlias*> &Visited,
+                                   const GlobalAlias &GA, const Constant &C) {
   if (GA.hasAvailableExternallyLinkage()) {
     Check(isa<GlobalValue>(C) &&
               cast<GlobalValue>(C).hasAvailableExternallyLinkage(),
@@ -1815,12 +1813,11 @@ void Verifier::visitModuleCommandLines() {
 
 void Verifier::visitModuleFlags() {
   const NamedMDNode *Flags = M.getModuleFlagsMetadata();
-  if (!Flags)
-    return;
+  if (!Flags) return;
 
   // Scan each flag, and track the flags and requirements.
-  DenseMap<const MDString *, const MDNode *> SeenIDs;
-  SmallVector<const MDNode *, 16> Requirements;
+  DenseMap<const MDString*, const MDNode*> SeenIDs;
+  SmallVector<const MDNode*, 16> Requirements;
   uint64_t PAuthABIPlatform = -1;
   uint64_t PAuthABIVersion = -1;
   for (const MDNode *MDN : Flags->operands()) {
@@ -1865,9 +1862,10 @@ void Verifier::visitModuleFlags() {
   }
 }
 
-void Verifier::visitModuleFlag(
-    const MDNode *Op, DenseMap<const MDString *, const MDNode *> &SeenIDs,
-    SmallVectorImpl<const MDNode *> &Requirements) {
+void
+Verifier::visitModuleFlag(const MDNode *Op,
+                          DenseMap<const MDString *, const MDNode *> &SeenIDs,
+                          SmallVectorImpl<const MDNode *> &Requirements) {
   // Each module flag should have three arguments, the merge behavior (a
   // constant int), the flag ID (an MDString), and the value.
   Check(Op->getNumOperands() == 3,
@@ -1946,8 +1944,8 @@ void Verifier::visitModuleFlag(
   }
 
   if (ID->getString() == "wchar_size") {
-    ConstantInt *Value =
-        mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
+    ConstantInt *Value
+      = mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2));
     Check(Value, "wchar_size metadata requires constant integer argument");
   }
 
@@ -2109,8 +2107,7 @@ void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
     if (!Attr.isStringAttribute() &&
         IncompatibleAttrs.contains(Attr.getKindAsEnum())) {
       CheckFailed("Attribute '" + Attr.getAsString() +
-                      "' applied to incompatible type!",
-                  V);
+                  "' applied to incompatible type!", V);
       return;
     }
   }
@@ -2346,15 +2343,15 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
   }
 
   Check(!Attrs.hasAttrSomewhere(Attribute::Writable) ||
-            isModSet(Attrs.getMemoryEffects().getModRef(IRMemLocation::ArgMem)),
+        isModSet(Attrs.getMemoryEffects().getModRef(IRMemLocation::ArgMem)),
         "Attribute writable and memory without argmem: write are incompatible!",
         V);
 
   if (Attrs.hasFnAttr("aarch64_pstate_sm_enabled")) {
     Check(!Attrs.hasFnAttr("aarch64_pstate_sm_compatible"),
-          "Attributes 'aarch64_pstate_sm_enabled and "
-          "aarch64_pstate_sm_compatible' are incompatible!",
-          V);
+           "Attributes 'aarch64_pstate_sm_enabled and "
+           "aarch64_pstate_sm_compatible' are incompatible!",
+           V);
   }
 
   Check((Attrs.hasFnAttr("aarch64_new_za") + Attrs.hasFnAttr("aarch64_in_za") +
@@ -2735,8 +2732,7 @@ void Verifier::verifyStatepoint(const CallBase &Call) {
   Check(TargetFuncType,
         "gc.statepoint callee elementtype must be function type", Call);
 
-  const int NumCallArgs =
-      cast<ConstantInt>(Call.getArgOperand(3))->getZExtValue();
+  const int NumCallArgs = cast<ConstantInt>(Call.getArgOperand(3))->getZExtValue();
   Check(NumCallArgs >= 0,
         "gc.statepoint number of arguments to underlying call "
         "must be positive",
@@ -2755,8 +2751,8 @@ void Verifier::verifyStatepoint(const CallBase &Call) {
     Check(NumCallArgs == NumParams,
           "gc.statepoint mismatch in number of call args", Call);
 
-  const uint64_t Flags =
-      cast<ConstantInt>(Call.getArgOperand(4))->getZExtValue();
+  const uint64_t Flags
+    = cast<ConstantInt>(Call.getArgOperand(4))->getZExtValue();
   Check((Flags & ~(uint64_t)StatepointFlags::MaskAll) == 0,
         "unknown flag used in gc.statepoint flags argument", Call);
 
@@ -3253,7 +3249,7 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
   // it.
   if (isa<PHINode>(BB.front())) {
     SmallVector<BasicBlock *, 8> Preds(predecessors(&BB));
-    SmallVector<std::pair<BasicBlock *, Value *>, 8> Values;
+    SmallVector<std::pair<BasicBlock*, Value*>, 8> Values;
     llvm::sort(Preds);
     for (const PHINode &PN : BB.phis()) {
       Check(PN.getNumIncomingValues() == Preds.size(),
@@ -3290,7 +3286,8 @@ void Verifier::visitBasicBlock(BasicBlock &BB) {
   }
 
   // Check that all instructions have their parent pointers set up correctly.
-  for (auto &I : BB) {
+  for (auto &I : BB)
+  {
     Check(I.getParent() == &BB, "Instruction has bogus parent pointer!");
   }
 
@@ -3338,7 +3335,7 @@ void Verifier::visitSwitchInst(SwitchInst &SI) {
   // Check to make sure that all of the constants in the switch instruction
   // have the same type as the switched-on value.
   Type *SwitchTy = SI.getCondition()->getType();
-  SmallPtrSet<ConstantInt *, 32> Constants;
+  SmallPtrSet<ConstantInt*, 32> Constants;
   for (auto &Case : SI.cases()) {
     Check(isa<ConstantInt>(SI.getOperand(Case.getCaseIndex() * 2 + 2)),
           "Case value is not a constant integer.", &SI);
@@ -3765,8 +3762,7 @@ void Verifier::visitCallBase(CallBase &Call) {
   for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
     if (Call.paramHasAttr(i, Attribute::SwiftError)) {
       Value *SwiftErrorArg = Call.getArgOperand(i);
-      if (auto AI =
-              dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
+      if (auto AI = dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
         Check(AI->isSwiftError(),
               "swifterror argument for call has mismatched alloca", AI, Call);
         continue;
@@ -4011,8 +4007,7 @@ static bool isTypeCongruent(Type *L, Type *R) {
   return PL->getAddressSpace() == PR->getAddressSpace();
 }
 
-static AttrBuilder getParameterABIAttributes(LLVMContext &C, unsigned I,
-                                             AttributeList Attrs) {
+static AttrBuilder getParameterABIAttributes(LLVMContext& C, unsigned I, AttributeList Attrs) {
   static const Attribute::AttrKind ABIAttrs[] = {
       Attribute::StructRet,  Attribute::ByVal,          Attribute::InAlloca,
       Attribute::InReg,      Attribute::StackAlignment, Attribute::SwiftSelf,
@@ -4080,14 +4075,12 @@ void Verifier::verifyMustTailCall(CallInst &CI) {
     // - Only sret, byval, swiftself, and swiftasync ABI-impacting attributes
     //   are allowed in swifttailcc call
     for (unsigned I = 0, E = CallerTy->getNumParams(); I != E; ++I) {
-      AttrBuilder ABIAttrs =
-          getParameterABIAttributes(F->getContext(), I, CallerAttrs);
+      AttrBuilder ABIAttrs = getParameterABIAttributes(F->getContext(), I, CallerAttrs);
       SmallString<32> Context{CCName, StringRef(" musttail caller")};
       verifyTailCCMustTailAttrs(ABIAttrs, Context);
     }
     for (unsigned I = 0, E = CalleeTy->getNumParams(); I != E; ++I) {
-      AttrBuilder ABIAttrs =
-          getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
+      AttrBuilder ABIAttrs = getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
       SmallString<32> Context{CCName, StringRef(" musttail callee")};
       verifyTailCCMustTailAttrs(ABIAttrs, Context);
     }
@@ -4113,10 +4106,8 @@ void Verifier::verifyMustTailCall(CallInst &CI) {
   // - All ABI-impacting function attributes, such as sret, byval, inreg,
   //   returned, preallocated, and inalloca, must match.
   for (unsigned I = 0, E = CallerTy->getNumParams(); I != E; ++I) {
-    AttrBuilder CallerABIAttrs =
-        getParameterABIAttributes(F->getContext(), I, CallerAttrs);
-    AttrBuilder CalleeABIAttrs =
-        getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
+    AttrBuilder CallerABIAttrs = getParameterABIAttributes(F->getContext(), I, CallerAttrs);
+    AttrBuilder CalleeABIAttrs = getParameterABIAttributes(F->getContext(), I, CalleeAttrs);
     Check(CallerABIAttrs == CalleeABIAttrs,
           "cannot guarantee tail call due to mismatched ABI impacting "
           "function attributes",
@@ -4508,7 +4499,7 @@ void Verifier::verifySwiftErrorValue(const Value *SwiftErrorVal) {
 
 void Verifier::visitAllocaInst(AllocaInst &AI) {
   Type *Ty = AI.getAllocatedType();
-  SmallPtrSet<Type *, 4> Visited;
+  SmallPtrSet<Type*, 4> Visited;
   Check(Ty->isSized(&Visited), "Cannot allocate unsized type", &AI);
   // Check if it's a target extension type that disallows being used on the
   // stack.
@@ -4558,8 +4549,7 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
   } else if (AtomicRMWInst::isFPOperation(Op)) {
     Check(ElTy->isFPOrFPVectorTy() && !isa<ScalableVectorType>(ElTy),
           "atomicrmw " + AtomicRMWInst::getOperationName(Op) +
-              " operand must have floating-point or fixed vector of "
-              "floating-point "
+              " operand must have floating-point or fixed vector of floating-point "
               "type!",
           &RMWI, ElTy);
   } else {
@@ -5038,7 +5028,7 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
   Check(DT.dominates(Op, U), "Instruction does not dominate all uses!", Op, &I);
 }
 
-void Verifier::visitDereferenceableMetadata(Instruction &I, MDNode *MD) {
+void Verifier::visitDereferenceableMetadata(Instruction& I, MDNode* MD) {
   Check(I.getType()->isPointerTy(),
         "dereferenceable, dereferenceable_or_null "
         "apply only to pointer types",
@@ -5371,7 +5361,7 @@ void Verifier::visitInstruction(Instruction &I) {
   BasicBlock *BB = I.getParent();
   Check(BB, "Instruction not embedded in basic block!", &I);
 
-  if (!isa<PHINode>(I)) { // Check that non-phi nodes are not self referential
+  if (!isa<PHINode>(I)) {   // Check that non-phi nodes are not self referential
     for (User *U : I.users()) {
       Check(U != (User *)&I || !DT.isReachableFromEntry(BB),
             "Only PHI nodes may reference their own value!", &I);
@@ -7115,8 +7105,7 @@ void Verifier::visitVPIntrinsic(VPIntrinsic &VPI) {
     case Intrinsic::vp_llrint:
       Check(
           RetTy->isIntOrIntVectorTy() && ValTy->isFPOrFPVectorTy(),
-          "llvm.vp.fptoui, llvm.vp.fptosi, llvm.vp.lrint or llvm.vp.llrint"
-          "intrinsic first argument element "
+          "llvm.vp.fptoui, llvm.vp.fptosi, llvm.vp.lrint or llvm.vp.llrint" "intrinsic first argument element "
           "type must be floating-point and result element type must be integer",
           *VPCast);
       break;
@@ -7606,7 +7595,8 @@ struct VerifierLegacyPass : public FunctionPass {
     initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
   }
   explicit VerifierLegacyPass(bool FatalErrors)
-      : FunctionPass(ID), FatalErrors(FatalErrors) {
+      : FunctionPass(ID),
+        FatalErrors(FatalErrors) {
     initializeVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
   }
 
@@ -7644,7 +7634,7 @@ struct VerifierLegacyPass : public FunctionPass {
 } // end anonymous namespace
 
 /// Helper to issue failure from the TBAA verification
-template <typename... Tys> void TBAAVerifier::CheckFailed(Tys &&...Args) {
+template <typename... Tys> void TBAAVerifier::CheckFailed(Tys &&... Args) {
   if (Diagnostic)
     return Diagnostic->CheckFailed(Args...);
 }
@@ -7694,8 +7684,7 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
   if (IsNewFormat) {
     if (BaseNode->getNumOperands() % 3 != 0) {
       CheckFailed("Access tag nodes must have the number of operands that is a "
-                  "multiple of 3!",
-                  BaseNode);
+                  "multiple of 3!", BaseNode);
       return InvalidNode;
     }
   } else {
@@ -7708,8 +7697,8 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
 
   // Check the type size field.
   if (IsNewFormat) {
-    auto *TypeSizeNode =
-        mdconst::dyn_extract_or_null<ConstantInt>(BaseNode->getOperand(1));
+    auto *TypeSizeNode = mdconst::dyn_extract_or_null<ConstantInt>(
+        BaseNode->getOperand(1));
     if (!TypeSizeNode) {
       CheckFailed("Type size nodes must be constants!", &I, BaseNode);
       return InvalidNode;
@@ -7733,7 +7722,7 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode,
   unsigned FirstFieldOpNo = IsNewFormat ? 3 : 1;
   unsigned NumOpsPerField = IsNewFormat ? 3 : 2;
   for (unsigned Idx = FirstFieldOpNo; Idx < BaseNode->getNumOperands();
-       Idx += NumOpsPerField) {
+           Idx += NumOpsPerField) {
     const MDOperand &FieldTy = BaseNode->getOperand(Idx);
     const MDOperand &FieldOffset = BaseNode->getOperand(Idx + 1);
     if (!isa<MDNode>(FieldTy)) {
@@ -7847,7 +7836,7 @@ MDNode *TBAAVerifier::getFieldNodeFromTBAABaseNode(Instruction &I,
   unsigned FirstFieldOpNo = IsNewFormat ? 3 : 1;
   unsigned NumOpsPerField = IsNewFormat ? 3 : 2;
   for (unsigned Idx = FirstFieldOpNo; Idx < BaseNode->getNumOperands();
-       Idx += NumOpsPerField) {
+           Idx += NumOpsPerField) {
     auto *OffsetEntryCI =
         mdconst::extract<ConstantInt>(BaseNode->getOperand(Idx + 1));
     if (OffsetEntryCI->getValue().ugt(Offset)) {
@@ -7866,8 +7855,8 @@ MDNode *TBAAVerifier::getFieldNodeFromTBAABaseNode(Instruction &I,
   }
 
   unsigned LastIdx = BaseNode->getNumOperands() - NumOpsPerField;
-  auto *LastOffsetEntryCI =
-      mdconst::extract<ConstantInt>(BaseNode->getOperand(LastIdx + 1));
+  auto *LastOffsetEntryCI = mdconst::extract<ConstantInt>(
+      BaseNode->getOperand(LastIdx + 1));
   Offset -= LastOffsetEntryCI->getValue();
   return cast<MDNode>(BaseNode->getOperand(LastIdx));
 }
@@ -7912,8 +7901,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
 
   // Check the access size field.
   if (IsNewFormat) {
-    auto *AccessSizeNode =
-        mdconst::dyn_extract_or_null<ConstantInt>(MD->getOperand(3));
+    auto *AccessSizeNode = mdconst::dyn_extract_or_null<ConstantInt>(
+        MD->getOperand(3));
     CheckTBAA(AccessSizeNode, "Access size field must be a constant", &I, MD);
   }
 
@@ -7951,8 +7940,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
   SmallPtrSet<MDNode *, 4> StructPath;
 
   for (/* empty */; BaseNode && !IsRootTBAANode(BaseNode);
-       BaseNode =
-           getFieldNodeFromTBAABaseNode(I, BaseNode, Offset, IsNewFormat)) {
+       BaseNode = getFieldNodeFromTBAABaseNode(I, BaseNode, Offset,
+                                               IsNewFormat)) {
     if (!StructPath.insert(BaseNode).second) {
       CheckFailed("Cycle detected in struct path", &I, MD);
       return false;
@@ -7960,8 +7949,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
 
     bool Invalid;
     unsigned BaseNodeBitWidth;
-    std::tie(Invalid, BaseNodeBitWidth) =
-        verifyTBAABaseNode(I, BaseNode, IsNewFormat);
+    std::tie(Invalid, BaseNodeBitWidth) = verifyTBAABaseNode(I, BaseNode,
+                                                             IsNewFormat);
 
     // If the base node is invalid in itself, then we've already printed all the
     // errors we wanted to print.
@@ -8006,7 +7995,7 @@ VerifierAnalysis::Result VerifierAnalysis::run(Module &M,
 
 VerifierAnalysis::Result VerifierAnalysis::run(Function &F,
                                                FunctionAnalysisManager &) {
-  return {llvm::verifyFunction(F, &dbgs()), false};
+  return { llvm::verifyFunction(F, &dbgs()), false };
 }
 
 PreservedAnalyses VerifierPass::run(Module &M, ModuleAnalysisManager &AM) {
diff --git a/llvm/test/Verifier/comdat2.ll b/llvm/test/Verifier/comdat2.ll
deleted file mode 100644
index b461d5846f9ca..0000000000000
--- a/llvm/test/Verifier/comdat2.ll
+++ /dev/null
@@ -1,7 +0,0 @@
-; RUN: llvm-as %s -o /dev/null
-; RUN: opt -mtriple=x86_64-unknown-linux -o /dev/null
-; RUN: not opt -mtriple=x86_64-pc-win32 %s -o /dev/null 2>&1 | FileCheck %s
-
-$v = comdat any
- at v = private global i32 0, comdat($v)
-; CHECK: comdat global value has private linkage
diff --git a/llvm/test/Verifier/comdat3.ll b/llvm/test/Verifier/comdat3.ll
deleted file mode 100644
index 28df930d9dddf..0000000000000
--- a/llvm/test/Verifier/comdat3.ll
+++ /dev/null
@@ -1,5 +0,0 @@
-; This used to be invalid, but now it's valid.  Ensure the verifier
-; doesn't reject it.
-; RUN: llvm-as %s -o /dev/null
-
-$v = comdat largest

>From 72af173cd9eac3f05c73566b52de06e91092e47c Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at google.com>
Date: Wed, 17 Sep 2025 14:40:05 -0700
Subject: [PATCH 3/4] Update llvm/lib/IR/Verifier.cpp

Co-authored-by: Matheus Izvekov <mizvekov at gmail.com>
---
 llvm/lib/IR/Verifier.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3c176b42ad3ae..109373fbc68e2 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1767,12 +1767,13 @@ void Verifier::visitComdat(const Comdat &C) {
   if (TT.isOSBinFormatCOFF()) {
     const GlobalValue *GV = M.getNamedValue(C.getName());
     bool IsDefined = GV != nullptr && !GV->isDeclarationForLinker();
-    Check(IsDefined || C.getUsers().empty(),
-          "COFF comdats must have a defined global value with the same name",
-          GV);
     if (IsDefined)
       Check(!GV->hasPrivateLinkage(), "comdat global value has private linkage",
             GV);
+    else
+      Check(C.getUsers().empty(),
+          "COFF comdats must have a defined global value with the same name",
+          GV);
   }
 }
 

>From dc7944742c8724a09a220dfd4070841eb65382ce Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at google.com>
Date: Wed, 17 Sep 2025 14:50:23 -0700
Subject: [PATCH 4/4] Format the new code

---
 llvm/lib/IR/Verifier.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 109373fbc68e2..adc9a4c17b88b 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1772,8 +1772,8 @@ void Verifier::visitComdat(const Comdat &C) {
             GV);
     else
       Check(C.getUsers().empty(),
-          "COFF comdats must have a defined global value with the same name",
-          GV);
+            "COFF comdats must have a defined global value with the same name",
+            GV);
   }
 }
 



More information about the llvm-commits mailing list