[PATCH] D88780: Allow interfaces to operate on in-memory buffers with no source location info.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

I think we could at least have a unittest that just calls these functions with an invalid SourceLocation. `unittests/Basic/SourceManagerTest.cpp` already takes care of creating a valid SourceManager object, so the unit test would just be a single call to each method. Of course it wouldn't be the exact same setup as whatever is Cling is doing, but it's better than nothing.

Also, I'm still trying to puzzle together what exactly the situation is that triggers this bug in Cling. From what I understand:

1. We call the ASTImporter::Import(Decl) with a Decl.
2. That triggers the importing of some SourceLocation. Given the ASTImporter is early-exiting on an invalid SourceLocation, this means you were importing a valid SourceLocation when hitting this crash.
3. We enter with a valid SourceLocation `isWrittenInBuiltinFile` from `ASTImporter::Import(SourceLocation)`. Now the function is computing the PresumedLoc via `getPresumedLoc` and that returns an invalid PresumedLoc.
4. We hit the assert due to the invalid PresumedLoc.

Do you know where exactly is getPresumedLoc returning the invalid error value? The review title just mentions a "in-memory buffer with no source location info", but it doesn't really explain what's going on. Are there SourceLocations pointing to that memory buffer but it's not registered with the SourceManager?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88780/new/

https://reviews.llvm.org/D88780