[clangd] Allow --query-driver to match a dot-normalized form of the path

[sam-mccall]

(In addition to the un-normalized form, so this is back-compatible)

[sam-mccall]

@kadircet this is internal b/294113850

[llvmbot]

@llvm/pr-subscribers-clangd

Changes

(In addition to the un-normalized form, so this is back-compatible)

---

Full diff: https://github.com/llvm/llvm-project/pull/66757.diff

1 Files Affected:

• (modified) clang-tools-extra/clangd/SystemIncludeExtractor.cpp (+7-1)

diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index 88df5b04ccb09f3..3f93927434dbfa8 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -343,7 +343,13 @@ extractSystemIncludesAndTarget(const DriverArgs &InputArgs,
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", InputArgs.Lang);
 
-  if (!QueryDriverRegex.match(Driver)) {
+  // If driver was "../foo" then having to allowlist "/path/a/../foo" rather
+  // than "/path/foo" is absurd.
+  // Allow either to match the whitelist, then proceed with "/path/a/../foo".
+  // This was our historical behavior, and it *could* resolve to something else.
+  llvm::SmallString<256> NoDots(Driver);
+  llvm::sys::path::remove_dots(NoDots, /*remove_dot_dot=*/true);
+  if (!QueryDriverRegex.match(Driver) && !QueryDriverRegex.match(NoDots)) {
     vlog("System include extraction: not allowed driver {0}", Driver);
     return std::nullopt;
   }

[sam-mccall]

I didn't add a test here because it seems fairly fiddly and fragile (would be a lit test) and unlikely to actually regress in practice. But I'm just making excuses, happy to add one if you think it's worthwhile.

[kadircet]

thanks, no need for tests, the logic is rather trivial (might be worth mentioning in change notes though, so that we can build them up more naturally for the future and encourage others to do so as well)

[HighCommander4]

See also some existing reports we have on file about other types of normalization:

• --query-driver validation check does not understand shorthands like PROGRA~1 clangd/clangd#1518 (Windows shorthands like PROGRA~1)
• --query-driver does not follow symlinks clangd/clangd#1605 (symlinks)
• clangd --query-driver doesn't seem to work with double slashes #62506 (double slashes)

[sam-mccall]

Thanks! This will fix the double-slash issue (despite the name, add_dots normalizes those).

symlinks and progra~1 mean normalization based on actual filesystem access, and choosing between multiple forms that are in some sense equally valid.

I don't think I'm opposed if someone wants to take a shot at solving that (we can afford to do IO here), but it's definitely a more complicated idea.