[clang][modulemap] Fix crashes loading pcms

[keith]

Previously in the case that the directory was missing, loading a pcm
would crash. Now it attempts to load absolute paths first, and
gracefully ignores missing directories.

Follow up to #181916

Fixes: #215931

Assisted By: codex

[keith]

@Bigcheese @jansvoboda11 from the related change

[llvmorg-github-actions]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Keith Smiley (keith)

Changes

Previously in the case that the directory was missing, loading a pcm
would crash. Now it attempts to load absolute paths first, and
gracefully ignores missing directories.

Follow up to #181916

Fixes: #215931

Assisted By: codex

---

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

2 Files Affected:

• (modified) clang/lib/Lex/ModuleMap.cpp (+12-10)
• (added) clang/test/Modules/pr215931.m (+38)

diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 6c07386f89010..9e0202409dfb2 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -175,10 +175,6 @@ static void appendSubframeworkPaths(Module *Mod,
 OptionalFileEntryRef ModuleMap::findHeader(
     Module *M, const Module::UnresolvedHeaderDirective &Header,
     SmallVectorImpl<char> &RelativePathName, bool &NeedsFramework) {
-  // Search for the header file within the module's home directory.
-  auto Directory = M->Directory;
-  SmallString<128> FullPathName(Directory->getName());
-
   auto GetFile = [&](StringRef Filename) -> OptionalFileEntryRef {
     auto File = SourceMgr.getFileManager().getOptionalFileRef(Filename);
     if (!File || (Header.Size && File->getSize() != *Header.Size) ||
@@ -187,6 +183,18 @@ OptionalFileEntryRef ModuleMap::findHeader(
     return *File;
   };
 
+  if (llvm::sys::path::is_absolute(Header.FileName)) {
+    RelativePathName.clear();
+    RelativePathName.append(Header.FileName.begin(), Header.FileName.end());
+    return GetFile(Header.FileName);
+  }
+
+  // Search for the header file within the module's home directory.
+  auto Directory = M->Directory;
+  if (!Directory)
+    return std::nullopt;
+  SmallString<128> FullPathName(Directory->getName());
+
   auto GetFrameworkFile = [&]() -> OptionalFileEntryRef {
     unsigned FullPathLength = FullPathName.size();
     appendSubframeworkPaths(M, RelativePathName);
@@ -215,12 +223,6 @@ OptionalFileEntryRef ModuleMap::findHeader(
     return GetFile(FullPathName);
   };
 
-  if (llvm::sys::path::is_absolute(Header.FileName)) {
-    RelativePathName.clear();
-    RelativePathName.append(Header.FileName.begin(), Header.FileName.end());
-    return GetFile(Header.FileName);
-  }
-
   if (M->isPartOfFramework())
     return GetFrameworkFile();
 
diff --git a/clang/test/Modules/pr215931.m b/clang/test/Modules/pr215931.m
new file mode 100644
index 0000000000000..7a8a5476e4a9a
--- /dev/null
+++ b/clang/test/Modules/pr215931.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// A PCM whose paths are relative to the consumer's working directory should
+// load successfully even though it does not contain a MODULE_DIRECTORY record.
+// RUN: %clang_cc1 -emit-module -x objective-c -fmodules \
+// RUN:   -fno-implicit-modules -fmodule-file-home-is-cwd \
+// RUN:   -fmodule-name=Repro mod/module.modulemap -o Repro-relative.pcm
+// RUN: %clang_cc1 -fsyntax-only -x objective-c -fmodules \
+// RUN:   -fno-implicit-modules -fmodule-file-home-is-cwd \
+// RUN:   -fmodule-file=Repro=Repro-relative.pcm use.m
+
+// An explicitly loaded PCM should also not crash if its absolute module home
+// directory has been removed.
+// RUN: %clang_cc1 -emit-module -x objective-c -fmodules \
+// RUN:   -fno-implicit-modules -fmodule-name=Repro \
+// RUN:   mod/module.modulemap -o Repro-absolute.pcm
+// RUN: rm -rf mod
+// RUN: %clang_cc1 -fsyntax-only -x objective-c -fmodules \
+// RUN:   -fno-implicit-modules -fmodule-file=Repro=Repro-absolute.pcm use.m
+
+//--- mod/module.modulemap
+module Repro {
+  umbrella header "Repro.h"
+  export *
+  module * { export * }
+}
+
+//--- mod/Repro.h
+#include "sub.h"
+
+//--- mod/sub.h
+static inline int repro_answer(void) { return 42; }
+
+//--- use.m
+@import Repro;
+int main(void) { return repro_answer(); }

[keith]

bump

[Bigcheese]

lgtm. Thanks for fixing this!

[Bigcheese]

/cherry-pick cd1abef

[llvmbot]

/cherry-pick cd1abef

Error: Command failed due to missing milestone.

[Bigcheese]

/cherry-pick cd1abef

[keith]

thanks, i'm happy to help shepard as needed too, i should have submitted this earlier!

[llvmbot]

/pull-request #224159