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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to