[clang-tools-extra] [clangd] Harden incomingCalls() against possible misinterpretation of a range as pertaining to the wrong file (PR #111616)

[Nathan Ridge via cfe-commits]

https://github.com/HighCommander4 created https://github.com/llvm/llvm-project/pull/111616

I don't have a concrete motivating scenario here, just something I noticed during code reading:

`CallHierarchyIncomingCall::fromRanges` are interpreted as ranges in the same file as the `CallHierarchyItem` representing the caller (`CallHierarchyIncomingCall::from`).

In the server implementation, we populate them based on `Ref::Location`, taking only the range and discarding the file, and associate them with a `CallHierarchyItem` representing `Ref::Container`.

Now, logically it **should** be the case that the definition location of the symbol referred to by `Ref::Container` is in the same file as the `Ref` itself... but I don't think anything guarantees this, and if for some reason this doesn't hold, we are effectively taking ranges from one file and interpreting them as being in another file.

The patch adds a check for this and discards ranges which are not in fact in the same file.

>From 88eed085ab89e53a4fe2bd66bbf108ed2c0f64a0 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Tue, 8 Oct 2024 21:43:55 -0400
Subject: [PATCH] [clangd] Harden incomingCalls() against possible
 misinterpretation of a range as pertaining to the wrong file

---
 clang-tools-extra/clangd/XRefs.cpp | 34 +++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index f94cadeffaa298..cca5035fb74bd4 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -63,6 +63,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <optional>
@@ -2272,18 +2273,14 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // Initially store the ranges in a map keyed by SymbolID of the caller.
   // This allows us to group different calls with the same caller
   // into the same CallHierarchyIncomingCall.
-  llvm::DenseMap<SymbolID, std::vector<Range>> CallsIn;
+  llvm::DenseMap<SymbolID, std::vector<SymbolLocation>> CallsIn;
   // We can populate the ranges based on a refs request only. As we do so, we
   // also accumulate the container IDs into a lookup request.
   LookupRequest ContainerLookup;
   Index->refs(Request, [&](const Ref &R) {
-    auto Loc = indexToLSPLocation(R.Location, Item.uri.file());
-    if (!Loc) {
-      elog("incomingCalls failed to convert location: {0}", Loc.takeError());
-      return;
-    }
-    auto It = CallsIn.try_emplace(R.Container, std::vector<Range>{}).first;
-    It->second.push_back(Loc->range);
+    auto It =
+        CallsIn.try_emplace(R.Container, std::vector<SymbolLocation>{}).first;
+    It->second.push_back(R.Location);
 
     ContainerLookup.IDs.insert(R.Container);
   });
@@ -2292,9 +2289,26 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   Index->lookup(ContainerLookup, [&](const Symbol &Caller) {
     auto It = CallsIn.find(Caller.ID);
     assert(It != CallsIn.end());
-    if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()))
+    if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) {
+      SymbolLocation CallerLoc =
+          Caller.Definition ? Caller.Definition : Caller.CanonicalDeclaration;
+      std::vector<Range> FromRanges;
+      for (const SymbolLocation &L : It->second) {
+        if (StringRef{L.FileURI} != StringRef{CallerLoc.FileURI}) {
+          elog("incomingCalls: call location not in same file as caller, this "
+               "is unexpected");
+          continue;
+        }
+        Range R;
+        R.start.line = L.Start.line();
+        R.start.character = L.Start.column();
+        R.end.line = L.End.line();
+        R.end.character = L.End.column();
+        FromRanges.push_back(R);
+      }
       Results.push_back(
-          CallHierarchyIncomingCall{std::move(*CHI), std::move(It->second)});
+          CallHierarchyIncomingCall{std::move(*CHI), std::move(FromRanges)});
+    }
   });
   // Sort results by name of container.
   llvm::sort(Results, [](const CallHierarchyIncomingCall &A,