[modules] Attempt to fix PR31905 - #include "stddef.h" breaks module map search paths; causes redefinitions.

[EricWF]

Summary

This patch attempts to fix llvm.org/PR31905.

The fix is to only allow the compiler to search the top-level include directories when looking a module.modulemap definition in ASTReader.cpp. This prevent loading arbitrary module maps from directories that would never normally be searched for a module map for _Builtin_stddef_max_align_t.

Perhaps it would be better to figure out what top-level search directory the module was originally built under, and then remove the prefix of the top-level directory. Then we could allow the search to look under matching subdirectories of other top-level search paths. However this seems like it could be incorrect as well.

[EricWF]

I've got a better, actually correct, solution incoming.

[EricWF]

• Only allow the top level header search directories to be search for module.modulemap files when loading modules in ReadAST.cpp.

Perhaps it would be better to figure out what top-level search directory the module was originally built under, and then remove the prefix of the top-level directory. Then we could allow the search to look under matching subdirectories of other top-level search paths. However this seems like it could be incorrect as well.

@zygoloid: Any thoughts?

[rsmith]

lib/Serialization/ASTReader.cpp

2554–2556 ↗

(On Diff #101307)

This is only trying to catch the case where we've already loaded a module map for the module and the two conflict. We should just set AllowSearch to false here. If we want to also perform validation on the case where we load the module map after importing another definition, that should be handled if/when we load the module map.

lib/Basic/Module.cpp

bool Module::directlyUses(const Module *Requested) const {
   // A top-level module implicitly uses itself.
   if (Requested->isSubModuleOf(Top))
     return true;
 
   for (auto *Use : Top->DirectUses)
     if (Requested->isSubModuleOf(Use))
       return true;
 
-  // Anyone is allowed to use our builtin stddef.h and its accompanying module.
-  if (!Requested->Parent && Requested->Name == "_Builtin_stddef_max_align_t")
-    return true;
-
   return false;
 }
 
 void Module::addRequirement(StringRef Feature, bool RequiredState,
                             const LangOptions &LangOpts,
                             const TargetInfo &Target) {
   Requirements.push_back(Requirement(Feature, RequiredState));
 

lib/Headers/module.modulemap

explicit module systemz {
 
     explicit module zvector {
       requires zvector, vx
       header "vecintrin.h"
     }
   }
 }
 
-module _Builtin_stddef_max_align_t [system] [extern_c] {
-  header "__stddef_max_align_t.h"
-}
-
 module opencl_c {
   requires opencl
   header "opencl-c.h"
 }

test/Modules/Inputs/PR31905/my-project/module.modulemap

+module my_project {
+  header "my-header.h"
+}

test/Modules/Inputs/PR31905/my-project/my-header.h

+#ifndef MY_HEADER_H
+#define MY_HEADER_H
+#include <stddef.h>
+#endif

test/Modules/pr31905.cpp

+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp -R %S/Inputs/PR31905 %t/other-include
+// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR31905/ -I%t/other-include -fmodules -fmodules-local-submodule-visibility \
+// RUN:    -fimplicit-module-maps -fmodules-cache-path=%t  -verify %s
+
+#include "my-project/my-header.h"
+
+int main() {} // expected-no-diagnostics