Do not use CppHashInfo if reporting an error after asm file parsed

[pmatos]

Summary

If parsing has finished, do not allow DiagHandler to rely on last parsed CppHashInfo
Instead recalculate the hash line before the error location.

Fixes PR#30817

[pmatos]

To the reviewers:
I believe there's a better way to handle this. However, I would like to know if this approach is a good one or the reviewers find that there's a much better way to do this and I took it in the wrong direction.

For example:

1. The re-parsing of the hash line in FindCppHashLoc is not great. It would be useful if we could reuse the asm lexer/parser but as far as I know, we can't;
2. I am slightly concerned that this fix won't work if the hash line doesn't fit the usual '# <int> ...', or worse, there's no hash line in the file; While in normal operation mode this doesn't happen it is possible to create a situation when the fix can be broken;

[pmatos]

Can someone please take a look at this one?

[enderby]

Seem like a step forward. The initial implementation for the search backwards in SourceMgr::FindCppHashLoc() is not great but I suspect it will work well enough.

[pmatos]

Apologies for leaving this for so long. Are there any further comments?
How do we get this upstream given I don't have commit rights?

include/llvm/Support/SourceMgr.h

public:
   unsigned AddIncludeFile(const std::string &Filename, SMLoc IncludeLoc,
                           std::string &IncludedFile);
 
   /// Return the ID of the buffer containing the specified location.
   ///
   /// 0 is returned if the buffer is not found.
   unsigned FindBufferContainingLoc(SMLoc Loc) const;
 
+  /// Find the location for the Hash line in the buffer preceeding Loc
+  SMLoc FindCppHashLoc(SMLoc Loc, unsigned BufferID) const;
+  
   /// Find the line number for the specified location in the specified file.
   /// This is not a fast method.
   unsigned FindLineNumber(SMLoc Loc, unsigned BufferID = 0) const {
     return getLineAndColumn(Loc, BufferID).first;
   }
 
   /// Find the line and column number for the specified location in the
   /// specified file. This is not a fast method.

lib/MC/MCParser/AsmParser.cpp

private:
   unsigned AssemblerDialect;
 
   /// \brief is Darwin compatibility enabled?
   bool IsDarwin;
 
   /// \brief Are we parsing ms-style inline assembly?
   bool ParsingInlineAsm;
 
+  /// \brief have we finished parsing the file?
+  bool FinishedParsing;
+  
 public:
   AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
             const MCAsmInfo &MAI);
   ~AsmParser() override;
 
   bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
 
   void addDirectiveHandler(StringRef Directive,

 
 enum { DEFAULT_ADDRSPACE = 0 };
 
 AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
                      const MCAsmInfo &MAI)
     : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM),
       PlatformParser(nullptr), CurBuffer(SM.getMainFileID()),
       MacrosEnabledFlag(true), CppHashInfo(), AssemblerDialect(~0U),
-      IsDarwin(false), ParsingInlineAsm(false) {
+      IsDarwin(false), ParsingInlineAsm(false), FinishedParsing(false) {
   HadError = false;
   // Save the old handler.
   SavedDiagHandler = SrcMgr.getDiagHandler();
   SavedDiagContext = SrcMgr.getDiagContext();
   // Set our own handler which calls the saved handler.
   SrcMgr.setDiagHandler(DiagHandler, this);
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer());
 
for (std::tuple<SMLoc, CppHashInfoTy, MCSymbol *> &LocSym : DirLabels) {
         // 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);
         printError(std::get<0>(LocSym), "directional label undefined");
       }
     }
   }
 
+  // Mark this run as finished
+  FinishedParsing = true;
+  
   // Finalize the output stream if there are no errors and if the client wants
   // us to.
   if (!HadError && !NoFinalize)
     Out.Finish();
 
   return HadError || getContext().hadError();
 }
 
void AsmParser::DiagHandler(const SMDiagnostic &Diag, void *Context) {
   }
 
   // Use the CppHashFilename and calculate a line number based on the
   // 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);
+  
+  // If we finished parsing CppHashLoc is invalid, so we need to recompute it
+  // from DiagBuf and DiagLoc.
+  SMLoc CppHashLoc;
+  int CppHashLineNumber;
+  if (Parser->FinishedParsing) {
+    CppHashLoc = Parser->SrcMgr.FindCppHashLoc(DiagLoc, DiagBuf);
+    
+    // From CppHashLoc, find line number (that's the number following the '#'
+    const char *Ptr = CppHashLoc.getPointer();
+    const char *Start = ++Ptr;
+    for (Ptr = Start; *Start == ' '; Start++, Ptr++);
+    for (; *Ptr != ' '; Ptr++);
+    
+    // Start points to the beginning of the number and Ptr to the end
+    StringRef LineNoStr(Start, Ptr - Start);
+    bool failed = LineNoStr.getAsInteger(0, CppHashLineNumber);
+    assert (!failed);
+  }
+  else {
+    CppHashLoc = Parser->CppHashInfo.Loc;
+    CppHashLineNumber = Parser->CppHashInfo.LineNumber;
+  }
+ 
   int CppHashLocLineNo =
-      Parser->SrcMgr.FindLineNumber(Parser->CppHashInfo.Loc, CppHashBuf);
+      Parser->SrcMgr.FindLineNumber(CppHashLoc, CppHashBuf);
   int LineNo =
-      Parser->CppHashInfo.LineNumber - 1 + (DiagLocLineNo - CppHashLocLineNo);
+      CppHashLineNumber - 1 + (DiagLocLineNo - CppHashLocLineNo);
+
+  // Better to crash than to be silly and report negative line number
+  assert (LineNo > 0);
   
   SMDiagnostic NewDiag(*Diag.getSourceMgr(), Diag.getLoc(), Filename, LineNo,
                        Diag.getColumnNo(), Diag.getKind(), Diag.getMessage(),
                        Diag.getLineContents(), Diag.getRanges());
 
   if (Parser->SavedDiagHandler)
     Parser->SavedDiagHandler(NewDiag, Parser->SavedDiagContext);
   else
     NewDiag.print(nullptr, OS);

lib/Support/SourceMgr.cpp

for (unsigned i = 0, e = Buffers.size(); i != e; ++i)
     if (Loc.getPointer() >= Buffers[i].Buffer->getBufferStart() &&
         // Use <= here so that a pointer to the null at the end of the buffer
         // is included as part of the buffer.
         Loc.getPointer() <= Buffers[i].Buffer->getBufferEnd())
       return i + 1;
   return 0;
 }
 
+SMLoc
+SourceMgr::FindCppHashLoc(SMLoc Loc, unsigned BufferID) const {
+  unsigned CppHashBuf = FindBufferContainingLoc(Loc);
+
+  // Initial implementation searches only for the previous '#' character
+  // after a newline: '\n[ ]*#'
+  const MemoryBuffer *Buff = getMemoryBuffer(BufferID);
+  const char *BufStart = Buff->getBufferStart();
+  const char *Ptr = BufStart;
+
+  SMLoc CppHashLoc;
+  
+  for (; SMLoc::getFromPointer(Ptr) != Loc; ++Ptr)
+    if (*Ptr == '#')
+      CppHashLoc = SMLoc::getFromPointer(Ptr);
+
+  assert(CppHashLoc.isValid());
+
+  return CppHashLoc;
+}
+
 std::pair<unsigned, unsigned>
 SourceMgr::getLineAndColumn(SMLoc Loc, unsigned BufferID) const {
   if (!BufferID)
     BufferID = FindBufferContainingLoc(Loc);
   assert(BufferID && "Invalid Location!");
 
   const MemoryBuffer *Buff = getMemoryBuffer(BufferID);