[clang] e78c80e - [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Ivan Murashko via cfe-commits]

Author: Ivan Murashko
Date: 2022-08-10T16:37:42+01:00
New Revision: e78c80e3d622d455934102dabd765a60ac917739

URL: https://github.com/llvm/llvm-project/commit/e78c80e3d622d455934102dabd765a60ac917739
DIFF: https://github.com/llvm/llvm-project/commit/e78c80e3d622d455934102dabd765a60ac917739.diff

LOG: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

There is a fix for the search procedure at `SourceManager::getFileIDLoaded`. It might return an invalid (not loaded) entry. That might cause clang/clangd crashes. At my case the scenario was the following:
- `SourceManager::getFileIDLoaded` returned an invalid file id for a non loaded entry and incorrectly set `SourceManager::LastFileIDLookup` to the value
- `getSLocEntry` returned `SourceManager::FakeSLocEntryForRecovery` introduced at [D89748](https://reviews.llvm.org/D89748).
- The result entry is not tested at `SourceManager::isOffsetInFileID`and as result the call `return SLocOffset < getSLocEntryByID(FID.ID+1).getOffset();` returned `true` value because `FID.ID+1` pointed to a non-fake entry
- The tested offset was marked as one that belonged to the fake `SLockEntry`

Such behaviour might cause a weird clangd crash when preamble contains some header files that were removed just after the preamble created. Unfortunately it's not so easy to reproduce the crash in the form of a LIT test thus I provided the fix only.

Test Plan:
```
ninja check-clang
```

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D130847

Added: 
    

Modified: 
    clang/lib/Basic/SourceManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 98e731eb12e62..38545d1be918f 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -877,9 +877,12 @@ FileID SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
     I = (-LastID - 2) + 1;
 
   unsigned NumProbes;
+  bool Invalid = false;
   for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
     // Make sure the entry is loaded!
-    const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+    const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+    if (Invalid)
+      return FileID(); // invalid entry.
     if (E.getOffset() <= SLocOffset) {
       FileID Res = FileID::get(-int(I) - 2);
       LastFileIDLookup = Res;
@@ -897,8 +900,8 @@ FileID SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
   while (true) {
     ++NumProbes;
     unsigned MiddleIndex = (LessIndex - GreaterIndex) / 2 + GreaterIndex;
-    const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex);
-    if (E.getOffset() == 0)
+    const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex, &Invalid);
+    if (Invalid)
       return FileID(); // invalid entry.
 
     ++NumProbes;