[llvm] r266238 - AsmParser: record "# line file" context to calculate location for diag

Kaylor, Andrew via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 20 17:36:04 PDT 2016


This change is causing lit test failures for me on Windows with MSVC 2013.  The following tests are failing:

  Failing Tests (10):
      LLVM :: MC/COFF/invalid-def.s
      LLVM :: MC/COFF/invalid-endef.s
      LLVM :: MC/COFF/invalid-scl-range.s
      LLVM :: MC/COFF/invalid-scl.s
      LLVM :: MC/COFF/invalid-type-range.s
      LLVM :: MC/COFF/invalid-type.s
      LLVM :: MC/COFF/secidx-diagnostic.s
      LLVM :: MC/ELF/common-error1.s
      LLVM :: MC/ELF/common-error2.s
      LLVM :: MC/ELF/undef-temp.s

I don't see these failures on Linux.  I have no idea why these tests are failing for me when they appear to pass on the Windows buildbots.  I'm using msbuild rather than ninja, so that could be related I guess.  I've tried both debug and release builds.  With assertions enabled, the failure looks like this:

Assertion failed: BufferID && "Invalid Location!", file <root>\llvm\lib\Support\SourceMgr.cpp, line 80

SourceMgr::getLineAndColumn() is being called with a null Loc argument.

With assertions turned off, the failure is an out of range index error because of the same problem.

Is anyone else seeing this?

-Andy

-----Original Message-----
From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf Of Tim Northover via llvm-commits
Sent: Wednesday, April 13, 2016 12:47 PM
To: llvm-commits at lists.llvm.org
Subject: [llvm] r266238 - AsmParser: record "# line file" context to calculate location for diag

Author: tnorthover
Date: Wed Apr 13 14:46:54 2016
New Revision: 266238

URL: http://llvm.org/viewvc/llvm-project?rev=266238&view=rev
Log:
AsmParser: record "# line file" context to calculate location for diag

Since we can't emit diagnostics for missing "jmp 1f" labels until the end of the file, we need to be able to restore the context used to calculate file/line. This is basically the "# line file" directive that's being used at the time the expression is seen.

rdar://25706972

Modified:
    llvm/trunk/lib/MC/MCParser/AsmParser.cpp
    llvm/trunk/test/MC/ELF/undefined-directional.s
    llvm/trunk/test/MC/MachO/undefined-directional.s

Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=266238&r1=266237&r2=266238&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
+++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Wed Apr 13 14:46:54 2016
@@ -146,9 +146,6 @@ private:
   /// \brief List of bodies of anonymous macros.
   std::deque<MCAsmMacro> MacroLikeBodies;
 
-  /// \brief List of forward directional labels for diagnosis at the end.
-  SmallVector<std::pair<SMLoc, MCSymbol *>, 4> DirectionalLabels;
-
   /// Boolean tracking whether macro substitution is enabled.
   unsigned MacrosEnabledFlag : 1;
 
@@ -159,10 +156,17 @@ private:
   unsigned HadError : 1;
 
   /// The values from the last parsed cpp hash file line comment if any.
-  StringRef CppHashFilename;
-  int64_t CppHashLineNumber;
-  SMLoc CppHashLoc;
-  unsigned CppHashBuf;
+  struct CppHashInfoTy {
+    StringRef Filename;
+    int64_t LineNumber;
+    SMLoc Loc;
+    unsigned Buf;
+  };
+  CppHashInfoTy CppHashInfo;
+
+  /// \brief List of forward directional labels for diagnosis at the end.
+  SmallVector<std::tuple<SMLoc, CppHashInfoTy, MCSymbol *>, 4> 
+ DirLabels;
+
   /// When generating dwarf for assembly source files we need to calculate the
   /// logical line number based on the last parsed cpp hash file line comment
   /// and current line. Since this is slow and messes up the SourceMgr's @@ -522,7 +526,7 @@ AsmParser::AsmParser(SourceMgr &SM, MCCo
                      const MCAsmInfo &MAI)
     : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM),
       PlatformParser(nullptr), CurBuffer(SM.getMainFileID()),
-      MacrosEnabledFlag(true), HadError(false), CppHashLineNumber(0),
+      MacrosEnabledFlag(true), HadError(false), CppHashInfo(),
       AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false) {
   // Save the old handler.
   SavedDiagHandler = SrcMgr.getDiagHandler(); @@ -718,9 +722,13 @@ bool AsmParser::Run(bool NoInitialTextSe
 
     // Temporary symbols like the ones for directional jumps don't go in the
     // symbol table. They also need to be diagnosed in all (final) cases.
-    for (std::pair<SMLoc, MCSymbol *> &LocSym : DirectionalLabels) {
-      if (LocSym.second->isUndefined())
-        HadError |= Error(LocSym.first, "directional label undefined");
+    for (std::tuple<SMLoc, CppHashInfoTy, MCSymbol *> &LocSym : DirLabels) {
+      if (std::get<2>(LocSym)->isUndefined()) {
+        // Reset the state of any "# line file" directives we've seen to the
+        // context as it was at the diagnostic site.
+        CppHashInfo = std::get<1>(LocSym);
+        HadError |= Error(std::get<0>(LocSym), "directional label undefined");
+      }
     }
   }
 
@@ -931,7 +939,7 @@ bool AsmParser::parsePrimaryExpr(const M
         Res = MCSymbolRefExpr::create(Sym, Variant, getContext());
         if (IDVal == "b" && Sym->isUndefined())
           return Error(Loc, "directional label undefined");
-        DirectionalLabels.push_back(std::make_pair(Loc, Sym));
+        DirLabels.push_back(std::make_tuple(Loc, CppHashInfo, Sym));
         EndLoc = Lexer.getTok().getEndLoc();
         Lex(); // Eat identifier.
       }
@@ -1796,24 +1804,26 @@ bool AsmParser::parseStatement(ParseStat
     // If we previously parsed a cpp hash file line comment then make sure the
     // current Dwarf File is for the CppHashFilename if not then emit the
     // Dwarf File table for it and adjust the line number for the .loc.
-    if (CppHashFilename.size()) {
+    if (CppHashInfo.Filename.size()) {
       unsigned FileNumber = getStreamer().EmitDwarfFileDirective(
-          0, StringRef(), CppHashFilename);
+          0, StringRef(), CppHashInfo.Filename);
       getContext().setGenDwarfFileNumber(FileNumber);
 
       // Since SrcMgr.FindLineNumber() is slow and messes up the SourceMgr's
       // cache with the different Loc from the call above we save the last
       // info we queried here with SrcMgr.FindLineNumber().
       unsigned CppHashLocLineNo;
-      if (LastQueryIDLoc == CppHashLoc && LastQueryBuffer == CppHashBuf)
+      if (LastQueryIDLoc == CppHashInfo.Loc &&
+          LastQueryBuffer == CppHashInfo.Buf)
         CppHashLocLineNo = LastQueryLine;
       else {
-        CppHashLocLineNo = SrcMgr.FindLineNumber(CppHashLoc, CppHashBuf);
+        CppHashLocLineNo =
+            SrcMgr.FindLineNumber(CppHashInfo.Loc, CppHashInfo.Buf);
         LastQueryLine = CppHashLocLineNo;
-        LastQueryIDLoc = CppHashLoc;
-        LastQueryBuffer = CppHashBuf;
+        LastQueryIDLoc = CppHashInfo.Loc;
+        LastQueryBuffer = CppHashInfo.Buf;
       }
-      Line = CppHashLineNumber - 1 + (Line - CppHashLocLineNo);
+      Line = CppHashInfo.LineNumber - 1 + (Line - CppHashLocLineNo);
     }
 
     getStreamer().EmitDwarfLocDirective(
@@ -1888,10 +1898,10 @@ bool AsmParser::parseCppHashLineFilename
   Filename = Filename.substr(1, Filename.size() - 2);
 
   // Save the SMLoc, Filename and LineNumber for later use by diagnostics.
-  CppHashLoc = L;
-  CppHashFilename = Filename;
-  CppHashLineNumber = LineNumber;
-  CppHashBuf = CurBuffer;
+  CppHashInfo.Loc = L;
+  CppHashInfo.Filename = Filename;
+  CppHashInfo.LineNumber = LineNumber;
+  CppHashInfo.Buf = CurBuffer;
 
   // Ignore any trailing characters, they're just comment.
   eatToEndOfLine();
@@ -1908,7 +1918,7 @@ void AsmParser::DiagHandler(const SMDiag
   SMLoc DiagLoc = Diag.getLoc();
   unsigned DiagBuf = DiagSrcMgr.FindBufferContainingLoc(DiagLoc);
   unsigned CppHashBuf =
-      Parser->SrcMgr.FindBufferContainingLoc(Parser->CppHashLoc);
+      Parser->SrcMgr.FindBufferContainingLoc(Parser->CppHashInfo.Loc);
 
   // Like SourceMgr::printMessage() we need to print the include stack if any
   // before printing the message.
@@ -1922,7 +1932,7 @@ void AsmParser::DiagHandler(const SMDiag
   // If we have not parsed a cpp hash line filename comment or the source
   // manager changed or buffer changed (like in a nested include) then just
   // print the normal diagnostic using its Filename and LineNo.
-  if (!Parser->CppHashLineNumber || &DiagSrcMgr != &Parser->SrcMgr ||
+  if (!Parser->CppHashInfo.LineNumber || &DiagSrcMgr != &Parser->SrcMgr 
+ ||
       DiagBuf != CppHashBuf) {
     if (Parser->SavedDiagHandler)
       Parser->SavedDiagHandler(Diag, Parser->SavedDiagContext); @@ -1932,15 +1942,15 @@ void AsmParser::DiagHandler(const SMDiag
   }
 
   // Use the CppHashFilename and calculate a line number based on the
-  // CppHashLoc and CppHashLineNumber relative to this Diag's SMLoc for
-  // the diagnostic.
-  const std::string &Filename = Parser->CppHashFilename;
+  // CppHashInfo.Loc and CppHashInfo.LineNumber relative to this Diag's 
+ SMLoc  // for the diagnostic.
+  const std::string &Filename = Parser->CppHashInfo.Filename;
 
   int DiagLocLineNo = DiagSrcMgr.FindLineNumber(DiagLoc, DiagBuf);
   int CppHashLocLineNo =
-      Parser->SrcMgr.FindLineNumber(Parser->CppHashLoc, CppHashBuf);
+      Parser->SrcMgr.FindLineNumber(Parser->CppHashInfo.Loc, 
+ CppHashBuf);
   int LineNo =
-      Parser->CppHashLineNumber - 1 + (DiagLocLineNo - CppHashLocLineNo);
+      Parser->CppHashInfo.LineNumber - 1 + (DiagLocLineNo - 
+ CppHashLocLineNo);
 
   SMDiagnostic NewDiag(*Diag.getSourceMgr(), Diag.getLoc(), Filename, LineNo,
                        Diag.getColumnNo(), Diag.getKind(), Diag.getMessage(),

Modified: llvm/trunk/test/MC/ELF/undefined-directional.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/undefined-directional.s?rev=266238&r1=266237&r2=266238&view=diff
==============================================================================
--- llvm/trunk/test/MC/ELF/undefined-directional.s (original)
+++ llvm/trunk/test/MC/ELF/undefined-directional.s Wed Apr 13 14:46:54 
+++ 2016
@@ -4,6 +4,9 @@
         jmp 1b
 // CHECK: [[@LINE+1]]:{{[0-9]+}}: error: directional label undefined
         jmp 1f
-// CHECK: [[@LINE+1]]:{{[0-9]+}}: error: directional label undefined
+# 10 "wibble.s"
+// CHECK: wibble.s:11:{{[0-9]+}}: error: directional label undefined
         jmp 2f
 
+# 42 "invalid.s"
+

Modified: llvm/trunk/test/MC/MachO/undefined-directional.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/MachO/undefined-directional.s?rev=266238&r1=266237&r2=266238&view=diff
==============================================================================
--- llvm/trunk/test/MC/MachO/undefined-directional.s (original)
+++ llvm/trunk/test/MC/MachO/undefined-directional.s Wed Apr 13 14:46:54 
+++ 2016
@@ -4,6 +4,8 @@
         jmp 1b
 // CHECK: [[@LINE+1]]:{{[0-9]+}}: error: directional label undefined
         jmp 1f
-// CHECK: [[@LINE+1]]:{{[0-9]+}}: error: directional label undefined
+# 10 "wibble.s"
+// CHECK: wibble.s:11:{{[0-9]+}}: error: directional label undefined
         jmp 2f
 
+# 42 "invalid.s"


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list