[PATCH] D123229: [clang][deps] Ensure deterministic file names on case-insensitive filesystems

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman, akyrtzi.
Herald added a subscriber: mgrang.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The dependency scanner can reuse single FileManager instance across multiple translation units. This may lead to non-deterministic output depending on which TU gets processed first.

The root cause is the fact that Clang is using DirectoryEntry::getName in the header search algorithm. This function returns the path that was first used to construct the (shared) entry in FileManager. Using DirectoryEntryRef::getName instead preserves the case as it was spelled out for the current "get directory entry" request.

rdar://90647508


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123229

Files:
  clang/include/clang/Lex/DirectoryLookup.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/ClangScanDeps/header-search-case-sensitivity.c


Index: clang/test/ClangScanDeps/header-search-case-sensitivity.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/header-search-case-sensitivity.c
@@ -0,0 +1,50 @@
+// This test checks that
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- dir1/arm/lower.h
+//--- dir2/ARM/upper.h
+//--- t1.c
+#include "upper.h"
+//--- t2.c
+#include "arm/lower.h"
+
+//--- cdb.json.template
+[{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/t1.c -I DIR/dir2/ARM -I DIR/dir1",
+  "file": "DIR/t1.c"
+},{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/t2.c -I DIR/dir2     -I DIR/dir1",
+  "file": "DIR/t2.c"
+}]
+
+//--- cdb-rev.json.template
+[{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/t2.c -I DIR/dir2     -I DIR/dir1",
+  "file": "DIR/t2.c"
+},{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/t1.c -I DIR/dir2/ARM -I DIR/dir1",
+  "file": "DIR/t1.c"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template     > %t/cdb.json
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb-rev.json.template > %t/cdb-rev.json
+
+// RUN: clang-scan-deps -compilation-database=%t/cdb.json     -format make -j 1 | FileCheck %s
+
+// In the reversed case, Clang starts by scanning "t2.c". When looking up the "arm/lower.h" header,
+// the string is appended to "DIR/dir2". That file ("DIR/dir2/arm/lower.h") doesn't exist, but when
+// learning so, the FileManager stats and caches the parent directory ("DIR/dir2/arm"), using the
+// UID as the key.
+// When scanning "t1.c" later on, the "DIR/dir2/ARM" search directory is assigned the **same**
+// directory entry (with lowercase "arm"), since they share the UID on case-insensitive filesystems.
+// To preserve the correct case throughout the compiler for any file within that directory, it's
+// important to use the spelling actually used, not just the cached one.
+// RUN: clang-scan-deps -compilation-database=%t/cdb-rev.json -format make -j 1 | FileCheck %s
+
+// CHECK: ARM/upper.h
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -436,10 +436,10 @@
   SmallString<1024> TmpDir;
   if (isNormalDir()) {
     // Concatenate the requested file onto the directory.
-    TmpDir = getDir()->getName();
+    TmpDir = getDirRef()->getName();
     llvm::sys::path::append(TmpDir, Filename);
     if (SearchPath) {
-      StringRef SearchPathRef(getDir()->getName());
+      StringRef SearchPathRef(getDirRef()->getName());
       SearchPath->clear();
       SearchPath->append(SearchPathRef.begin(), SearchPathRef.end());
     }
Index: clang/include/clang/Lex/DirectoryLookup.h
===================================================================
--- clang/include/clang/Lex/DirectoryLookup.h
+++ clang/include/clang/Lex/DirectoryLookup.h
@@ -91,6 +91,10 @@
     return isNormalDir() ? &u.Dir.getDirEntry() : nullptr;
   }
 
+  Optional<DirectoryEntryRef> getDirRef() const {
+    return isNormalDir() ? Optional<DirectoryEntryRef>(u.Dir) : None;
+  }
+
   /// getFrameworkDir - Return the directory that this framework refers to.
   ///
   const DirectoryEntry *getFrameworkDir() const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123229.420904.patch
Type: text/x-patch
Size: 3261 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220406/f8e5e0b2/attachment-0001.bin>