[llvm] [LLVM][AsmParser] Make error reporting of lexer errors more precise (PR #111077)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 3 16:35:04 PDT 2024


https://github.com/jurahul created https://github.com/llvm/llvm-project/pull/111077

When the lexer hits an error during assembly parsing, it just logs the error in the `ErrorMsg` object, and its possible that error gets overwritten later in by the parser with a more generic error message. This makes some errors reported by the LLVM's asm parser less precise.

Address this by not having the parser overwrite the message logged by the lexer. In certain instances, where the parser error may be more precise, allow overriding that behavior with an `Overwrite` flag.

Update several Assembler unit test to now check the more precise error messaged reported by the LLVM's AsmParser.

>From 9afca06070587155fa6e42c53335e414d36227bf Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 3 Oct 2024 16:28:16 -0700
Subject: [PATCH] [LLVM][AsmParser] Make error reporting of lexer errors more
 precise

When the lexer hits an error during assembly parsing, it just logs
the error in the `ErrorMsg` object, and its possible that error gets
overwritten later in by the parser with a more generic error message.
This makes some errors reported by the LLVM's asm parser less precise.

Address this by not having the parser overwrite the message logged
by the lexer. In certain instances, where the parser error may be
more precise, allow overriding that behavior with an `Overwrite`
flag.

Update several Assembler unit test to now check the more precise
error messaged reported by the LLVM's AsmParser.
---
 llvm/include/llvm/AsmParser/LLLexer.h     |  11 +++++++++--
 llvm/include/llvm/AsmParser/LLParser.h    |  10 +++++++---
 llvm/lib/AsmParser/LLLexer.cpp            |   6 ++++--
 llvm/lib/AsmParser/LLParser.cpp           |   6 ++++--
 llvm/test/Assembler/allockind-missing.ll  |   4 ++--
 llvm/test/Assembler/invalid-inttype.ll    |   4 ++--
 llvm/test/Assembler/invalid-landingpad.ll |   4 ++--
 llvm/test/Assembler/invalid-name.ll       | Bin 142 -> 207 bytes
 llvm/test/Assembler/invalid-name2.ll      | Bin 120 -> 185 bytes
 9 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/llvm/include/llvm/AsmParser/LLLexer.h b/llvm/include/llvm/AsmParser/LLLexer.h
index bd929db33c4a2b..9564265c6cedeb 100644
--- a/llvm/include/llvm/AsmParser/LLLexer.h
+++ b/llvm/include/llvm/AsmParser/LLLexer.h
@@ -28,7 +28,12 @@ namespace llvm {
   class LLLexer {
     const char *CurPtr;
     StringRef CurBuf;
+
+    // Used to make sure an earlier error is not overwritten by a later one
+    // (e.g., a lexer error overwritten by a later parser one).
+    bool HasErrorInfo = false;
     SMDiagnostic &ErrorInfo;
+
     SourceMgr &SM;
     LLVMContext &Context;
 
@@ -66,8 +71,10 @@ namespace llvm {
       IgnoreColonInIdentifiers = val;
     }
 
-    bool Error(LocTy ErrorLoc, const Twine &Msg) const;
-    bool Error(const Twine &Msg) const { return Error(getLoc(), Msg); }
+    bool Error(LocTy ErrorLoc, const Twine &Msg, bool Overwrite = false);
+    bool Error(const Twine &Msg, bool Overwrite = false) {
+      return Error(getLoc(), Msg);
+    }
 
     void Warning(LocTy WarningLoc, const Twine &Msg) const;
     void Warning(const Twine &Msg) const { return Warning(getLoc(), Msg); }
diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h
index 9576b935198dd4..a41597c1c155f7 100644
--- a/llvm/include/llvm/AsmParser/LLParser.h
+++ b/llvm/include/llvm/AsmParser/LLParser.h
@@ -207,11 +207,15 @@ namespace llvm {
     LLVMContext &getContext() { return Context; }
 
   private:
-    bool error(LocTy L, const Twine &Msg) const { return Lex.Error(L, Msg); }
-    bool tokError(const Twine &Msg) const { return error(Lex.getLoc(), Msg); }
+    bool error(LocTy L, const Twine &Msg, bool Overwrite = false) {
+      return Lex.Error(L, Msg, Overwrite);
+    }
+    bool tokError(const Twine &Msg, bool Overwrite = false) {
+      return error(Lex.getLoc(), Msg, Overwrite);
+    }
 
     bool checkValueID(LocTy L, StringRef Kind, StringRef Prefix,
-                      unsigned NextID, unsigned ID) const;
+                      unsigned NextID, unsigned ID);
 
     /// Restore the internal name and slot mappings using the mappings that
     /// were created at an earlier parsing stage.
diff --git a/llvm/lib/AsmParser/LLLexer.cpp b/llvm/lib/AsmParser/LLLexer.cpp
index a3e47da77fe776..eb50b35d209c8c 100644
--- a/llvm/lib/AsmParser/LLLexer.cpp
+++ b/llvm/lib/AsmParser/LLLexer.cpp
@@ -25,8 +25,10 @@
 
 using namespace llvm;
 
-bool LLLexer::Error(LocTy ErrorLoc, const Twine &Msg) const {
-  ErrorInfo = SM.GetMessage(ErrorLoc, SourceMgr::DK_Error, Msg);
+bool LLLexer::Error(LocTy ErrorLoc, const Twine &Msg, bool Overwrite) {
+  if (!HasErrorInfo || Overwrite)
+    ErrorInfo = SM.GetMessage(ErrorLoc, SourceMgr::DK_Error, Msg);
+  HasErrorInfo = true;
   return true;
 }
 
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index d84521d2e6e10d..ae03bba72fb0c3 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -2631,7 +2631,9 @@ unsigned LLParser::parseNoFPClassAttr() {
 
       return Value;
     } else {
-      error(Lex.getLoc(), "expected nofpclass test mask");
+      // We enter here is parseUInt64() fails and log a generic error message.
+      // Overwrite it with a more precise error message.
+      error(Lex.getLoc(), "expected nofpclass test mask", /*Overwrite=*/true);
       return 0;
     }
 
@@ -3220,7 +3222,7 @@ bool LLParser::parseOptionalOperandBundles(
 }
 
 bool LLParser::checkValueID(LocTy Loc, StringRef Kind, StringRef Prefix,
-                            unsigned NextID, unsigned ID) const {
+                            unsigned NextID, unsigned ID) {
   if (ID < NextID)
     return error(Loc, Kind + " expected to be numbered '" + Prefix +
                           Twine(NextID) + "' or greater");
diff --git a/llvm/test/Assembler/allockind-missing.ll b/llvm/test/Assembler/allockind-missing.ll
index e8672fe9db032a..8445f5d6d16482 100644
--- a/llvm/test/Assembler/allockind-missing.ll
+++ b/llvm/test/Assembler/allockind-missing.ll
@@ -1,4 +1,4 @@
-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as --disable-output %s  2>&1 | FileCheck -DFILE=%s %s
 
+; CHECK: [[FILE]]:[[@LINE+1]]:30: error:  expected string constant
 declare void @f0() allockind()
-; CHECK: :[[#@LINE-1]]:30: error: expected allockind value
diff --git a/llvm/test/Assembler/invalid-inttype.ll b/llvm/test/Assembler/invalid-inttype.ll
index df3175540fe829..c8aa7c66b79e4d 100644
--- a/llvm/test/Assembler/invalid-inttype.ll
+++ b/llvm/test/Assembler/invalid-inttype.ll
@@ -1,5 +1,5 @@
-; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+; RUN: not llvm-as --disable-output %s 2>&1 | FileCheck -DFILE=%s %s
 
 ; i8388609 is the smallest integer type that can't be represented in LLVM IR
+; CHECK: [[FILE]]:[[@LINE+1]]:21: error: bitwidth for integer type out of range!
 @i2 = common global i8388609 0, align 4
-; CHECK: expected type
diff --git a/llvm/test/Assembler/invalid-landingpad.ll b/llvm/test/Assembler/invalid-landingpad.ll
index 306e94312dd552..805d3dbaa4d206 100644
--- a/llvm/test/Assembler/invalid-landingpad.ll
+++ b/llvm/test/Assembler/invalid-landingpad.ll
@@ -1,7 +1,7 @@
-; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+; RUN: not llvm-as --disable-output %s 2>&1 | FileCheck -DFILE=%s %s
 
-; CHECK: clause argument must be a constant
 
 define void @test(i32 %in) personality ptr null {
+; CHECK: [[FILE]]:[[@LINE+1]]:24: error: 'filter' clause has an invalid type
   landingpad {} filter i32 %in
 }
diff --git a/llvm/test/Assembler/invalid-name.ll b/llvm/test/Assembler/invalid-name.ll
index 0681ea528bf43017d803f26ccd9f8c0ad3b0523b..74133e60df54d595c68c50aea282cd90899f334f 100644
GIT binary patch
literal 207
zcmW-b%L>9U6hvL;D~5uiNWdz%NKp#4q9|Pmu1XiFy<#vnlB7OR|J^FgV&EK at iLl#e
z0=4nb`gY;cf%8f^S!&{@@z=(q>oA`8LcFn5nvxS8&ftsXdYjH#)P}K&j;bcyjH(bH
z(I4Q&A|$Ic*$CwM&n*g(=ka!(rNgl869KZ;Sb?n38s*cIgS7O&BDFSms-SAr@<NUQ
b-Jla-21gGM6;3gNH&Byti!Ay7GW)PEXMR1O

literal 142
zcmW;E!3u&v6h`6J_bGk^qZUCLw9txRD1x9x&;!KG6))M0I5!chcTd_L_)dxI?NY!8
zMbq at p#XVw!G$P+kGkkEAhE2`(F*YK{m3T%U#9?1yHpiQQG?^sWBJ!Y+Y>g!}Kvp at G
gFsFk#7vP~~tLz{?YWK1#!6URbI39h+`d`wQeoNdb&;S4c

diff --git a/llvm/test/Assembler/invalid-name2.ll b/llvm/test/Assembler/invalid-name2.ll
index 384dee6777d805466166edf9db309eb75c3de521..8a848798a54cafef9c3c151b08f70955e59fa62f 100644
GIT binary patch
literal 185
zcmW-bu?oU47=)ek6o;TFlA(?fMW`(nO9#OzIHdL$gE5gLt%&;WR^f(&@8G)y_J>Nq
zx(H*s8&?6GH#(@=5O;07w-KX&`D&ctja(Z_FLb(sZ}OtdmObjjSVL!R37yjo)@Mwn
zc(Dc2ZI<o?r2K7<M<M09EUIjl^gR(E at 0}N at T4PW@BLyf=|0~KE_n-#a!m1kuHXIBE
L13bmQgMHW+VOTf-

literal 120
zcmW;DK?=e!5Cu at zbBa$v(S-|LrXoUU5d;^42asfngE3Q at u?Xtj6}Jywjnnz)kkydV
zcwb%$HkcNyx3d-AXeF_~a=87BC~_y6F{(uDI?NY%Q8lR?)$seq9~}7Eb1;sOTx at s*
R6V1>*Xk-EgwtuD%eE~+FBr^a2




More information about the llvm-commits mailing list