[llvm-bugs] [Bug 41431] New: SourceManager::getPresumedLoc() does the wrong thing for locations referring to a header that was present when a module was built but are no longer around when the module is used

via llvm-bugs llvm-bugs at lists.llvm.org
Mon Apr 8 10:46:06 PDT 2019


https://bugs.llvm.org/show_bug.cgi?id=41431

            Bug ID: 41431
           Summary: SourceManager::getPresumedLoc() does the wrong thing
                    for locations referring to a header that was present
                    when a module was built but are no longer around when
                    the module is used
           Product: clang
           Version: unspecified
          Hardware: PC
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Modules
          Assignee: unassignedclangbugs at nondot.org
          Reporter: nicolasweber at gmx.de
                CC: dgregor at apple.com, llvm-bugs at lists.llvm.org,
                    richard-llvm at metafoo.co.uk

While I was working on something else I noticed that
clang/test/Modules/explicit-build-missing-files.cpp passes by accident.

VerifyDiagnosticConsumer calls getPresumedLineNumber() for the expected and
actual diags, but both happen to return a line number of 0 for the include
line.

The output is correct when not using -verify, but with -verify two bugs happen
that cancel out:

1. -verify causes diagnostics to be emitted not when they're normally emitted,
but batched together "later" (this is what I'm working on changing). Were the
diagnostic emitted immediately it correctly would be emitted on line 45.

2. When VerifyDiagnositcHandler reads the //expected-error{{}} comment, it
constructs the correct ExpectedLoc from the correct ExpectedLine. However, when
that same ExpectedLoc is later converted back to a line number via
getPresumedLineNumber, it comes back as line 0. See the debug patch below [1]
which produces output:

expected roundtrip 2624 0x7fe9a8d01b90 46 46 0
while checking 2624 0x7fe9a8d01b90 0

The first line is the raw encoding of ExpectedLoc and the SourceManager
pointer, followed by the output of getSpellingLineNumber,
getPresumedLineNumber, and 0. The second line is again the raw encoding, the
SourceManager, and the output of getPresumedLineNumber later on when the line
number is computed for comparison with the actual diag.


The problem is that getPresumedLoc() calls

  std::pair<FileID, unsigned> LocInfo = getDecomposedExpansionLoc(Loc);

which calls

    const SrcMgr::SLocEntry *E = &getSLocEntry(FID, &Invalid);

which calls

    return getSLocEntryByID(FID.ID, Invalid);

which calls

    if (ID < 0)
      return getLoadedSLocEntryByID(ID, Invalid);

which calls

    return getLoadedSLocEntry(static_cast<unsigned>(-ID - 2), Invalid);

which calls

    return loadSLocEntry(Index, Invalid);

which does

   439    if (ExternalSLocEntries->ReadSLocEntry(-(static_cast<int>(Index) +
2))) {
   440      if (Invalid)
-> 441        *Invalid = true;


and returns `Invalid` here, which causes an empty SourceLocation to come back.


The root issue seems to be that LastFileIDLookup is set to -3 when loading the
module, which points to the .h file that no longer exists.
isOffsetInFileID() succeeds because:

(lldb) 
Process 90002 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x0000000100a3cb61
clang`clang::SourceManager::isOffsetInFileID(this=0x0000000110d10d00, FID=(ID =
-3), SLocOffset=2624) const at SourceManager.h:1767
   1764 
   1765     // Otherwise, the entry after it has to not include it. This works
for both
   1766     // local and loaded entries.
-> 1767     return SLocOffset < getSLocEntryByID(FID.ID+1).getOffset();
   1768   }
   1769 
   1770   /// Returns the previous in-order FileID or an invalid FileID if
there
Target 0: (clang) stopped.
(lldb) p getSLocEntryByID(FID.ID+1, nullptr).getOffset()
(unsigned int) $2 = 2147483628


(which looks bogus).



I tried changing isOffsetInFileID() to call `getLoadedSLocEntry(FID.ID+1)`
instead of getSLocEntry() and that changes the error message of that test in a
way that looks like a progression:


error: 'error' diagnostics seen but not expected: 
  (frontend): malformed or corrupted AST file: 'could not find file
'/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/explicit-build-missing-files.cpp.tmp/a.h'
referenced by AST file
'/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/explicit-build-missing-files.cpp.tmp/a.pcm''


(but lots of other tests start crashing so it's clearly not the right fix)



It looks like the 1-elt LastFileIDLookup cache isn't invalidated correctly,
probably need to figure out why.






1.

diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
index f78013b91b9..8ac302cd5d3 100644
--- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -465,7 +465,16 @@ static bool ParseDirective(StringRef S, ExpectedData *ED,
SourceManager &SM,
         if (!Invalid && PH.Next(Line) && (FoundPlus || Line < ExpectedLine)) {
           if (FoundPlus) ExpectedLine += Line;
           else ExpectedLine -= Line;
+
+          unsigned Pres = SM.getPresumedLineNumber(Pos);
+          fprintf(stderr, "expected %d %d\n", ExpectedLine, Pres);
+
           ExpectedLoc = SM.translateLineCol(SM.getFileID(Pos), ExpectedLine,
1);
+
+          bool b = false;
+          unsigned ExpectedLine2 = SM.getSpellingLineNumber(ExpectedLoc);
+          unsigned Pres2 = SM.getPresumedLineNumber(ExpectedLoc);
+          fprintf(stderr, "expected roundtrip %u %p %d %d %d\n",
ExpectedLoc.getRawEncoding(), &SM, ExpectedLine2, Pres2, b);
         }
       } else if (PH.Next(Line)) {
         // Absolute line number.
@@ -782,6 +791,7 @@ static unsigned CheckLists(DiagnosticsEngine &Diags,
SourceManager &SourceMgr,
   for (auto &Owner : Left) {
     Directive &D = *Owner;
     unsigned LineNo1 = SourceMgr.getPresumedLineNumber(D.DiagnosticLoc);
+fprintf(stderr, "while checking %u %p %d\n", D.DiagnosticLoc.getRawEncoding(),
&SourceMgr, LineNo1);

     for (unsigned i = 0; i < D.Max; ++i) {
       DiagList::iterator II, IE;

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190408/7c8cd80e/attachment.html>


More information about the llvm-bugs mailing list