[LLVM][Parser] Check invalid overload suffix for intrinsics

llvm/include/llvm/AsmParser/LLParser.h

@@ -171,6 +171,10 @@ namespace llvm {
     std::map<unsigned, std::vector<std::pair<GlobalValue::GUID *, LocTy>>>
         ForwardRefTypeIds;
 
+    // Locations for all call instructions that call an overloaded intrinsic
+    // (for accurate error reporting).
+    std::map<CallBase *, LocTy> OverloadedIntrinsicCallLocs;
+
     // Map of module ID to path.
     std::map<unsigned, StringRef> ModuleIdMap;
 

llvm/include/llvm/IR/Function.h

@@ -269,7 +269,12 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node<Function> {
   /// getIntrinsicID() returns Intrinsic::not_intrinsic.
   bool isConstrainedFPIntrinsic() const;
 
-  static Intrinsic::ID lookupIntrinsicID(StringRef Name);
+  static std::pair<Intrinsic::ID, StringRef>
+  lookupIntrinsicIDAndSuffix(StringRef Name);
+
+  static Intrinsic::ID lookupIntrinsicID(StringRef Name) {
+    return lookupIntrinsicIDAndSuffix(Name).first;
+  }
 
   /// Update internal caches that depend on the function name (such as the
   /// intrinsic ID and libcall cache).

llvm/lib/AsmParser/LLParser.cpp

@@ -338,7 +338,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
 
   for (const auto &[Name, Info] : make_early_inc_range(ForwardRefVals)) {
     if (StringRef(Name).starts_with("llvm.")) {
-      Intrinsic::ID IID = Function::lookupIntrinsicID(Name);
+      auto [IID, Suffix] = Function::lookupIntrinsicIDAndSuffix(Name);
       if (IID == Intrinsic::not_intrinsic)
         // Don't do anything for unknown intrinsics.
         continue;
@@ -349,18 +349,37 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
       //
       // Additionally, automatically add the required mangling suffix to the
       // intrinsic name. This means that we may replace a single forward
-      // declaration with multiple functions here.
+      // declaration with multiple functions here. If there is a suffix left
+      // over after matching the intrinsic name, it should match the mangling
+      // suffix.
       for (Use &U : make_early_inc_range(Info.first->uses())) {
         auto *CB = dyn_cast<CallBase>(U.getUser());
         if (!CB || !CB->isCallee(&U))
           return error(Info.second, "intrinsic can only be used as callee");
 
+        // Location for error reporting.
+        LocTy Loc;
+        auto II = OverloadedIntrinsicCallLocs.find(CB);
+        if (II != OverloadedIntrinsicCallLocs.end()) {
+          Loc = II->second;
+          OverloadedIntrinsicCallLocs.erase(II);
+        } else {
+          Loc = Info.second;
+        }
+
         SmallVector<Type *> OverloadTys;
         if (!Intrinsic::getIntrinsicSignature(IID, CB->getFunctionType(),
                                               OverloadTys))
-          return error(Info.second, "invalid intrinsic signature");
-
-        U.set(Intrinsic::getDeclaration(M, IID, OverloadTys));
+          return error(Loc, "invalid intrinsic signature");
+        Function *Intrinsic = Intrinsic::getDeclaration(M, IID, OverloadTys);
+        // Note: Suffix will be empty for non-overloaded intrinsics, so this
+        // check will always pass. For overloaded intrinsics that do not use
+        // a mangling suffix as well, the suffix will be empty and they will
+        // always pass this error check.
+        if (!Intrinsic->getName().ends_with(Suffix))
+          return error(Loc, "invalid intrinsic name, expected @" +
+                                Intrinsic->getName());
+        U.set(Intrinsic);
       }
 
       Info.first->eraseFromParent();
@@ -8079,16 +8098,24 @@ bool LLParser::parseCall(Instruction *&Inst, PerFunctionState &PFS,
     CI->setFastMathFlags(FMF);
   }
 
-  if (CalleeID.Kind == ValID::t_GlobalName &&
-      isOldDbgFormatIntrinsic(CalleeID.StrVal)) {
-    if (SeenNewDbgInfoFormat) {
-      CI->deleteValue();
-      return error(CallLoc, "llvm.dbg intrinsic should not appear in a module "
-                            "using non-intrinsic debug info");
+  if (CalleeID.Kind == ValID::t_GlobalName) {
+    if (StringRef(CalleeID.StrVal).starts_with("llvm.")) {
+      // If this is a call to an intrinsic, remember its location for better
+      // error reporting when overloaded intrinsic resolution fails.
+      OverloadedIntrinsicCallLocs[CI] = CalleeID.Loc;
+    }
+
+    if (isOldDbgFormatIntrinsic(CalleeID.StrVal)) {
+      if (SeenNewDbgInfoFormat) {
+        CI->deleteValue();
+        return error(CallLoc,
+                     "llvm.dbg intrinsic should not appear in a module "
+                     "using non-intrinsic debug info");
+      }
+      if (!SeenOldDbgInfoFormat)
+        M->setNewDbgInfoFormatFlag(false);
+      SeenOldDbgInfoFormat = true;
     }
-    if (!SeenOldDbgInfoFormat)
-      M->setNewDbgInfoFormatFlag(false);
-    SeenOldDbgInfoFormat = true;
   }
   CI->setAttributes(PAL);
   ForwardRefAttrGroups[CI] = FwdRefAttrGrps;

llvm/lib/IR/Function.cpp

@@ -974,13 +974,14 @@ static ArrayRef<const char *> findTargetSubtable(StringRef Name) {
   return ArrayRef(&IntrinsicNameTable[1] + TI.Offset, TI.Count);
 }
 
-/// This does the actual lookup of an intrinsic ID which
-/// matches the given function name.
-Intrinsic::ID Function::lookupIntrinsicID(StringRef Name) {
+/// This does the actual lookup of an intrinsic ID which matches the given
+/// function name.
+std::pair<Intrinsic::ID, StringRef>
+Function::lookupIntrinsicIDAndSuffix(StringRef Name) {
   ArrayRef<const char *> NameTable = findTargetSubtable(Name);
   int Idx = Intrinsic::lookupLLVMIntrinsicByName(NameTable, Name);
   if (Idx == -1)
-    return Intrinsic::not_intrinsic;
+    return {Intrinsic::not_intrinsic, ""};
 
   // Intrinsic IDs correspond to the location in IntrinsicNameTable, but we have
   // an index into a sub-table.
@@ -992,8 +993,11 @@ Intrinsic::ID Function::lookupIntrinsicID(StringRef Name) {
   const auto MatchSize = strlen(NameTable[Idx]);
   assert(Name.size() >= MatchSize && "Expected either exact or prefix match");
   bool IsExactMatch = Name.size() == MatchSize;
-  return IsExactMatch || Intrinsic::isOverloaded(ID) ? ID
-                                                     : Intrinsic::not_intrinsic;
+  if (IsExactMatch)
+    return {ID, ""};
+  if (Intrinsic::isOverloaded(ID))
+    return {ID, Name.drop_front(MatchSize)};
+  return {Intrinsic::not_intrinsic, ""};
 }
 
 void Function::updateAfterNameChange() {

llvm/test/Assembler/intrinsic-overload-error0.ll

@@ -0,0 +1,12 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; Check that intrinsic calls with mangling are error checked.
+; Mix good and bad calls to demonstrate that line number tracking is required
+; in the parser to report correct line number for the second (bad) call.
+define void @foo(float %a, i32 %b) {
+    %c = call i1 @llvm.is.constant.i32(i32 0)
+    ; CHECK: <stdin>:[[@LINE+1]]:18: error: invalid intrinsic name, expected @llvm.is.constant.f32
+    %d = call i1 @llvm.is.constant.i32(float %a)
+    %e = call i1 @llvm.is.constant.i1(i1 false)
+    ret void
+}

llvm/test/Assembler/intrinsic-overload-error1.ll

@@ -0,0 +1,8 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; Check that intrinsic calls with mangling are error checked.
+define void @foo(float %a, i32 %b) {
+    ; CHECK: <stdin>:[[@LINE+1]]:18: error: invalid intrinsic name, expected @llvm.is.constant.f32
+    %c = call i1 @llvm.is.constant.badsuffix(float %a)
+    ret void
+}

llvm/test/Assembler/intrinsic-overload.ll

@@ -0,0 +1,16 @@
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+; Check that intrinsic calls without any mangling are converted to correct
+; mangled forms. And mangled forms with correct mangling parse correctly.
+define void @foo(float %a, i32 %b) {
+    ; CHECK:  call i1 @llvm.is.constant.f32
+    %c = call i1 @llvm.is.constant(float %a)
+
+    ; CHECK: call i1 @llvm.is.constant.i32
+    %d = call i1 @llvm.is.constant(i32 %b)
+
+    ; CHECK: call i1 @llvm.is.constant.i1
+    %e = call i1 @llvm.is.constant.i1(i1 false)
+
+    ret void
+}