From 90fdad2001b756b8c6fdef5262cb76a67fc6420c Mon Sep 17 00:00:00 2001 From: Michael Spencer Date: Thu, 5 Mar 2026 11:29:31 -0800 Subject: [PATCH] [clang][modules] Add warning for module maps with ".." paths (#184279) Implicitly discovered module maps that reference files outside their module directory cause order dependent behavior when using implicitly discovered module maps. This adds an off by default diagnostic about these cases with the long term goal of removing import order dependent behavior. Module maps found via `-fmodule-map-file=` are not a problem because they are all loaded at the start of translation. Assisted-by: claude-opus-4.6 --- .../modularize/ModularizeUtilities.cpp | 3 +- clang/docs/Modules.rst | 14 ++++ clang/docs/ReleaseNotes.rst | 6 ++ .../include/clang/Basic/DiagnosticLexKinds.td | 4 + clang/include/clang/Lex/HeaderSearch.h | 26 ++++-- clang/include/clang/Lex/ModuleMap.h | 7 +- clang/include/clang/Lex/ModuleMapFile.h | 4 +- clang/lib/Frontend/FrontendAction.cpp | 7 +- clang/lib/Lex/HeaderSearch.cpp | 73 ++++++++++------- clang/lib/Lex/ModuleMap.cpp | 48 ++++++++--- clang/lib/Lex/ModuleMapFile.cpp | 4 +- .../deprecated-upwards-relative-path.m | 81 +++++++++++++++++++ .../Clang/ClangModulesDeclVendor.cpp | 3 +- 13 files changed, 225 insertions(+), 55 deletions(-) create mode 100644 clang/test/Modules/deprecated-upwards-relative-path.m diff --git a/clang-tools-extra/modularize/ModularizeUtilities.cpp b/clang-tools-extra/modularize/ModularizeUtilities.cpp index 6978a6b2fe1b..1353904f1fe0 100644 --- a/clang-tools-extra/modularize/ModularizeUtilities.cpp +++ b/clang-tools-extra/modularize/ModularizeUtilities.cpp @@ -287,7 +287,8 @@ std::error_code ModularizeUtilities::loadModuleMap( Target.get(), *HeaderInfo)); // Parse module.modulemap file into module map. - if (ModMap->parseAndLoadModuleMapFile(ModuleMapEntry, false, Dir)) { + if (ModMap->parseAndLoadModuleMapFile(ModuleMapEntry, /*IsSystem=*/false, + /*ImplicitlyDiscovered=*/false, Dir)) { return std::error_code(1, std::generic_category()); } diff --git a/clang/docs/Modules.rst b/clang/docs/Modules.rst index bfdb05ea459c..379f62ebf548 100644 --- a/clang/docs/Modules.rst +++ b/clang/docs/Modules.rst @@ -668,6 +668,14 @@ token sequence within the prebuilt module representation. A header with the ``exclude`` specifier is excluded from the module. It will not be included when the module is built, nor will it be considered to be part of the module, even if an ``umbrella`` directory would otherwise make it part of the module. +.. note:: + + Header paths that use ``..`` to refer to files outside of the module + directory are deprecated in module maps that are found via implicit search + (``-fimplicit-module-maps``). Use ``-Wmodule-map-path-outside-directory`` + to warn on such paths. Future versions of Clang may reject these paths + in implicitly discovered module maps. + **Example:** A "X macro" header is an excellent candidate for a textual header, because it is can't be compiled standalone, and by itself does not contain any declarations. .. parsed-literal:: @@ -701,6 +709,12 @@ An umbrella directory declaration specifies that all of the headers in the speci The *string-literal* refers to a directory. When the module is built, all of the header files in that directory (and its subdirectories) are included in the module. +.. note:: + + Umbrella directory paths that use ``..`` to refer to directories outside of + the module directory are deprecated in implicitly discovered module maps. + See the note in `Header declaration`_ for details. + An *umbrella-dir-declaration* shall not refer to the same directory as the location of an umbrella *header-declaration*. In other words, only a single kind of umbrella can be specified for a given directory. .. note:: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 82b03341138d..863c7392f756 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -285,6 +285,12 @@ Improvements to Clang's diagnostics - ``-Wunsafe-buffer-usage`` now warns about unsafe two-parameter constructors of ``std::string_view`` (pointer and size), consistent with the existing warning for ``std::span``. +- Added ``-Wmodule-map-path-outside-directory`` (off by default) to warn on + header and umbrella directory paths that use ``..`` to refer outside the module + directory in module maps found via implicit search + (``-fimplicit-module-maps``). This does not affect module maps specified + explicitly via ``-fmodule-map-file=``. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 77feea9f869e..5eceeced311f 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -996,6 +996,10 @@ def warn_non_modular_include_in_module : Warning< def warn_module_conflict : Warning< "module '%0' conflicts with already-imported module '%1': %2">, InGroup; +def warn_mmap_path_outside_directory : Warning< + "path refers outside of the module directory; such paths in implicitly " + "discovered module maps are deprecated">, + InGroup>, DefaultIgnore; // C++20 modules def err_pp_module_name_is_macro : Error< diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 25d1ca2a247f..d7000da682c6 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -755,6 +755,8 @@ public: /// /// \param File The module map file. /// \param IsSystem Whether this file is in a system header directory. + /// \param ImplicitlyDiscovered Whether this file was found by module map + /// search. /// \param ID If the module map file is already mapped (perhaps as part of /// processing a preprocessed module), the ID of the file. /// \param Offset [inout] An offset within ID to start parsing. On exit, @@ -765,6 +767,7 @@ public: /// building the module from preprocessed source). /// \returns true if an error occurred, false otherwise. bool parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, FileID ID = FileID(), unsigned *Offset = nullptr, StringRef OriginalModuleMapFile = StringRef()); @@ -823,9 +826,12 @@ private: /// \param IsSystem Whether the framework directory is part of the system /// frameworks. /// + /// \param ImplicitlyDiscovered Whether the framework was discovered by module + /// map search. + /// /// \returns The module, if found; otherwise, null. Module *loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir, - bool IsSystem); + bool IsSystem, bool ImplicitlyDiscovered); /// Load all of the module maps within the immediate subdirectories /// of the given search directory. @@ -983,14 +989,13 @@ private: MMR_InvalidModuleMap }; - ModuleMapResult parseAndLoadModuleMapFileImpl(FileEntryRef File, - bool IsSystem, - DirectoryEntryRef Dir, - FileID ID = FileID(), - unsigned *Offset = nullptr, - bool DiagnosePrivMMap = false); + ModuleMapResult parseAndLoadModuleMapFileImpl( + FileEntryRef File, bool IsSystem, bool ImplicitlyDiscovered, + DirectoryEntryRef Dir, FileID ID = FileID(), unsigned *Offset = nullptr, + bool DiagnosePrivMMap = false); ModuleMapResult parseModuleMapFileImpl(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, DirectoryEntryRef Dir, FileID ID = FileID()); @@ -1004,6 +1009,7 @@ private: /// \returns The result of attempting to load the module map file from the /// named directory. ModuleMapResult parseAndLoadModuleMapFile(StringRef DirName, bool IsSystem, + bool ImplicitlyDiscovered, bool IsFramework); /// Try to load the module map file in the given directory. @@ -1015,11 +1021,15 @@ private: /// \returns The result of attempting to load the module map file from the /// named directory. ModuleMapResult parseAndLoadModuleMapFile(DirectoryEntryRef Dir, - bool IsSystem, bool IsFramework); + bool IsSystem, + bool ImplicitlyDiscovered, + bool IsFramework); ModuleMapResult parseModuleMapFile(StringRef DirName, bool IsSystem, + bool ImplicitlyDiscovered, bool IsFramework); ModuleMapResult parseModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, + bool ImplicitlyDiscovered, bool IsFramework); }; diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 9133422b2999..570a68c37fac 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -726,7 +726,8 @@ public: /// Parse a module map without creating `clang::Module` instances. bool parseModuleMapFile(FileEntryRef File, bool IsSystem, - DirectoryEntryRef Dir, FileID ID = FileID(), + bool ImplicitlyDiscovered, DirectoryEntryRef Dir, + FileID ID = FileID(), SourceLocation ExternModuleLoc = SourceLocation()); void loadAllParsedModules(); @@ -739,6 +740,9 @@ public: /// \param IsSystem Whether this module map file is in a system header /// directory, and therefore should be considered a system module. /// + /// \param ImplicitlyDiscovered Whether this module map file was found via + /// module map search. + /// /// \param HomeDir The directory in which relative paths within this module /// map file will be resolved. /// @@ -753,6 +757,7 @@ public: /// \returns true if an error occurred, false otherwise. bool parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, DirectoryEntryRef HomeDir, FileID ID = FileID(), unsigned *Offset = nullptr, SourceLocation ExternModuleLoc = SourceLocation()); diff --git a/clang/include/clang/Lex/ModuleMapFile.h b/clang/include/clang/Lex/ModuleMapFile.h index 7d0e36e9ab86..59389cd85a92 100644 --- a/clang/include/clang/Lex/ModuleMapFile.h +++ b/clang/include/clang/Lex/ModuleMapFile.h @@ -144,6 +144,7 @@ struct ModuleMapFile { SourceLocation Start; bool IsSystem; + bool ImplicitlyDiscovered; std::vector Decls; void dump(llvm::raw_ostream &out) const; @@ -163,7 +164,8 @@ struct ModuleMapFile { /// \returns The parsed ModuleMapFile if successful, std::nullopt otherwise. std::optional parseModuleMap(FileID ID, clang::DirectoryEntryRef Dir, SourceManager &SM, - DiagnosticsEngine &Diags, bool IsSystem, unsigned *Offset); + DiagnosticsEngine &Diags, bool IsSystem, + bool ImplicitlyDiscovered, unsigned *Offset); } // namespace modulemap } // namespace clang diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 79e862f01be1..73f092521546 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -706,8 +706,9 @@ static bool loadModuleMapForModuleBuild(CompilerInstance &CI, bool IsSystem, } // Load the module map file. - if (HS.parseAndLoadModuleMapFile(*ModuleMap, IsSystem, ModuleMapID, &Offset, - PresumedModuleMapFile)) + if (HS.parseAndLoadModuleMapFile(*ModuleMap, IsSystem, + /*ImplicitlyDiscovered=*/false, ModuleMapID, + &Offset, PresumedModuleMapFile)) return true; if (SrcMgr.getBufferOrFake(ModuleMapID).getBufferSize() == Offset) @@ -1176,7 +1177,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) { if (auto File = CI.getFileManager().getOptionalFileRef(Filename)) CI.getPreprocessor().getHeaderSearchInfo().parseAndLoadModuleMapFile( - *File, /*IsSystem*/ false); + *File, /*IsSystem*/ false, /*ImplicitlyDiscovered=*/false); else CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename; } diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 4435cb6f23d9..5aee19cd14c5 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -342,7 +342,8 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName, if (auto FrameworkDir = FileMgr.getOptionalDirectoryRef(FrameworkDirName)) { bool IsSystem = Dir.getDirCharacteristic() != SrcMgr::C_User; - Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem); + Module = loadFrameworkModule(ModuleName, *FrameworkDir, IsSystem, + /*ImplicitlyDiscovered=*/true); if (Module) break; } @@ -359,7 +360,7 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName, // checked DirectoryEntryRef NormalDir = *Dir.getDirRef(); // Search for a module map file in this directory. - if (parseModuleMapFile(NormalDir, IsSystem, + if (parseModuleMapFile(NormalDir, IsSystem, /*ImplicitlyDiscovered=*/true, /*IsFramework*/ false) == MMR_NewlyProcessed) { // We just parsed a module map file; check whether the module can be // loaded now. @@ -374,6 +375,7 @@ Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName, NestedModuleMapDirName = Dir.getDirRef()->getName(); llvm::sys::path::append(NestedModuleMapDirName, ModuleName); if (parseModuleMapFile(NestedModuleMapDirName, IsSystem, + /*ImplicitlyDiscovered=*/true, /*IsFramework*/ false) == MMR_NewlyProcessed) { // If we just parsed a module map file, look for the module again. Module = ModMap.findOrLoadModule(ModuleName); @@ -1753,7 +1755,8 @@ bool HeaderSearch::hasModuleMap(StringRef FileName, // Check if it's possible that the module map for this directory can resolve // this header. - parseModuleMapFile(*Dir, IsSystem, IsFramework); + parseModuleMapFile(*Dir, IsSystem, /*ImplicitlyDiscovered=*/true, + IsFramework); auto DirState = DirectoryModuleMap.find(*Dir); if (DirState == DirectoryModuleMap.end() || !DirState->second.ModuleMapFile) continue; @@ -1940,7 +1943,8 @@ bool HeaderSearch::findUsableModuleForFrameworkHeader( // Load this framework module. If that succeeds, find the suggested module // for this header, if any. - loadFrameworkModule(ModuleName, *TopFrameworkDir, IsSystemFramework); + loadFrameworkModule(ModuleName, *TopFrameworkDir, IsSystemFramework, + /*ImplicitlyDiscovered=*/true); // FIXME: This can find a module not part of ModuleName, which is // important so that we're consistent about whether this header @@ -1977,6 +1981,7 @@ static OptionalFileEntryRef getPrivateModuleMap(FileEntryRef File, } bool HeaderSearch::parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, FileID ID, unsigned *Offset, StringRef OriginalModuleMapFile) { // Find the directory for the module. For frameworks, that may require going @@ -2012,7 +2017,8 @@ bool HeaderSearch::parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, } assert(Dir && "module map home directory must exist"); - switch (parseAndLoadModuleMapFileImpl(File, IsSystem, *Dir, ID, Offset, + switch (parseAndLoadModuleMapFileImpl(File, IsSystem, ImplicitlyDiscovered, + *Dir, ID, Offset, /*DiagnosePrivMMap=*/true)) { case MMR_AlreadyProcessed: case MMR_NewlyProcessed: @@ -2025,8 +2031,8 @@ bool HeaderSearch::parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, } HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFileImpl( - FileEntryRef File, bool IsSystem, DirectoryEntryRef Dir, FileID ID, - unsigned *Offset, bool DiagnosePrivMMap) { + FileEntryRef File, bool IsSystem, bool ImplicitlyDiscovered, + DirectoryEntryRef Dir, FileID ID, unsigned *Offset, bool DiagnosePrivMMap) { // Check whether we've already loaded this module map, and mark it as being // loaded in case we recursively try to load it from itself. auto AddResult = LoadedModuleMaps.insert(std::make_pair(File, true)); @@ -2034,7 +2040,8 @@ HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFileImpl( return AddResult.first->second ? MMR_AlreadyProcessed : MMR_InvalidModuleMap; - if (ModMap.parseAndLoadModuleMapFile(File, IsSystem, Dir, ID, Offset)) { + if (ModMap.parseAndLoadModuleMapFile(File, IsSystem, ImplicitlyDiscovered, + Dir, ID, Offset)) { LoadedModuleMaps[File] = false; return MMR_InvalidModuleMap; } @@ -2042,7 +2049,8 @@ HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFileImpl( // Try to load a corresponding private module map. if (OptionalFileEntryRef PMMFile = getPrivateModuleMap(File, FileMgr, Diags, DiagnosePrivMMap)) { - if (ModMap.parseAndLoadModuleMapFile(*PMMFile, IsSystem, Dir)) { + if (ModMap.parseAndLoadModuleMapFile(*PMMFile, IsSystem, + ImplicitlyDiscovered, Dir)) { LoadedModuleMaps[File] = false; return MMR_InvalidModuleMap; } @@ -2054,6 +2062,7 @@ HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFileImpl( HeaderSearch::ModuleMapResult HeaderSearch::parseModuleMapFileImpl(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, DirectoryEntryRef Dir, FileID ID) { // Check whether we've already parsed this module map, and mark it as being // parsed in case we recursively try to parse it from itself. @@ -2062,7 +2071,8 @@ HeaderSearch::parseModuleMapFileImpl(FileEntryRef File, bool IsSystem, return AddResult.first->second ? MMR_AlreadyProcessed : MMR_InvalidModuleMap; - if (ModMap.parseModuleMapFile(File, IsSystem, Dir, ID)) { + if (ModMap.parseModuleMapFile(File, IsSystem, ImplicitlyDiscovered, Dir, + ID)) { ParsedModuleMaps[File] = false; return MMR_InvalidModuleMap; } @@ -2070,7 +2080,8 @@ HeaderSearch::parseModuleMapFileImpl(FileEntryRef File, bool IsSystem, // Try to parse a corresponding private module map. if (OptionalFileEntryRef PMMFile = getPrivateModuleMap(File, FileMgr, Diags, /*Diagnose=*/false)) { - if (ModMap.parseModuleMapFile(*PMMFile, IsSystem, Dir)) { + if (ModMap.parseModuleMapFile(*PMMFile, IsSystem, ImplicitlyDiscovered, + Dir)) { ParsedModuleMaps[File] = false; return MMR_InvalidModuleMap; } @@ -2115,9 +2126,11 @@ HeaderSearch::lookupModuleMapFile(DirectoryEntryRef Dir, bool IsFramework) { } Module *HeaderSearch::loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir, - bool IsSystem) { + bool IsSystem, + bool ImplicitlyDiscovered) { // Try to load a module map file. - switch (parseAndLoadModuleMapFile(Dir, IsSystem, /*IsFramework*/ true)) { + switch (parseAndLoadModuleMapFile(Dir, IsSystem, ImplicitlyDiscovered, + /*IsFramework*/ true)) { case MMR_InvalidModuleMap: // Try to infer a module map from the framework directory. if (HSOpts.ImplicitModuleMaps) @@ -2137,15 +2150,18 @@ Module *HeaderSearch::loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir, HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFile(StringRef DirName, bool IsSystem, + bool ImplicitlyDiscovered, bool IsFramework) { if (auto Dir = FileMgr.getOptionalDirectoryRef(DirName)) - return parseAndLoadModuleMapFile(*Dir, IsSystem, IsFramework); + return parseAndLoadModuleMapFile(*Dir, IsSystem, ImplicitlyDiscovered, + IsFramework); return MMR_NoDirectory; } HeaderSearch::ModuleMapResult HeaderSearch::parseAndLoadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, + bool ImplicitlyDiscovered, bool IsFramework) { auto InsertRes = DirectoryModuleMap.insert(std::pair{ Dir, ModuleMapDirectoryState{{}, {}, ModuleMapDirectoryState::Invalid}}); @@ -2169,8 +2185,8 @@ HeaderSearch::parseAndLoadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, } if (MMState.ModuleMapFile) { - ModuleMapResult Result = - parseAndLoadModuleMapFileImpl(*MMState.ModuleMapFile, IsSystem, Dir); + ModuleMapResult Result = parseAndLoadModuleMapFileImpl( + *MMState.ModuleMapFile, IsSystem, ImplicitlyDiscovered, Dir); // Add Dir explicitly in case ModuleMapFile is in a subdirectory. // E.g. Foo.framework/Modules/module.modulemap // ^Dir ^ModuleMapFile @@ -2185,18 +2201,20 @@ HeaderSearch::parseAndLoadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, HeaderSearch::ModuleMapResult HeaderSearch::parseModuleMapFile(StringRef DirName, bool IsSystem, - bool IsFramework) { + bool ImplicitlyDiscovered, bool IsFramework) { if (auto Dir = FileMgr.getOptionalDirectoryRef(DirName)) - return parseModuleMapFile(*Dir, IsSystem, IsFramework); + return parseModuleMapFile(*Dir, IsSystem, ImplicitlyDiscovered, + IsFramework); return MMR_NoDirectory; } HeaderSearch::ModuleMapResult HeaderSearch::parseModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, - bool IsFramework) { + bool ImplicitlyDiscovered, bool IsFramework) { if (!HSOpts.LazyLoadModuleMaps) - return parseAndLoadModuleMapFile(Dir, IsSystem, IsFramework); + return parseAndLoadModuleMapFile(Dir, IsSystem, ImplicitlyDiscovered, + IsFramework); auto InsertRes = DirectoryModuleMap.insert(std::pair{ Dir, ModuleMapDirectoryState{{}, {}, ModuleMapDirectoryState::Invalid}}); @@ -2219,8 +2237,8 @@ HeaderSearch::parseModuleMapFile(DirectoryEntryRef Dir, bool IsSystem, } if (MMState.ModuleMapFile) { - ModuleMapResult Result = - parseModuleMapFileImpl(*MMState.ModuleMapFile, IsSystem, Dir); + ModuleMapResult Result = parseModuleMapFileImpl( + *MMState.ModuleMapFile, IsSystem, ImplicitlyDiscovered, Dir); // Add Dir explicitly in case ModuleMapFile is in a subdirectory. // E.g. Foo.framework/Modules/module.modulemap // ^Dir ^ModuleMapFile @@ -2259,7 +2277,7 @@ void HeaderSearch::collectAllModules(SmallVectorImpl &Modules) { // Load this framework module. loadFrameworkModule(llvm::sys::path::stem(Dir->path()), *FrameworkDir, - IsSystem); + IsSystem, /*ImplicitlyDiscovered=*/true); } continue; } @@ -2270,6 +2288,7 @@ void HeaderSearch::collectAllModules(SmallVectorImpl &Modules) { // Try to load a module map file for the search directory. parseAndLoadModuleMapFile(*DL.getDirRef(), IsSystem, + /*ImplicitlyDiscovered=*/true, /*IsFramework*/ false); // Try to load module map files for immediate subdirectories of this @@ -2294,7 +2313,7 @@ void HeaderSearch::loadTopLevelSystemModules() { // Try to load a module map file for the search directory. parseAndLoadModuleMapFile(*DL.getDirRef(), DL.isSystemHeaderDirectory(), - DL.isFramework()); + /*ImplicitlyDiscovered=*/true, DL.isFramework()); } } @@ -2317,9 +2336,9 @@ void HeaderSearch::loadSubdirectoryModuleMaps(DirectoryLookup &SearchDir) { continue; bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework"; if (IsFramework == SearchDir.isFramework()) - parseAndLoadModuleMapFile(Dir->path(), - SearchDir.isSystemHeaderDirectory(), - SearchDir.isFramework()); + parseAndLoadModuleMapFile( + Dir->path(), SearchDir.isSystemHeaderDirectory(), + /*ImplicitlyDiscovered=*/true, SearchDir.isFramework()); } SearchDir.setSearchedAllModuleMaps(true); diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 1ed6a91e9245..6c991430cb08 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -1063,7 +1063,9 @@ Module *ModuleMap::inferFrameworkModule(DirectoryEntryRef FrameworkDir, HeaderInfo.lookupModuleMapFile(*ParentDir, IsFrameworkDir)) { // TODO: Parsing a module map should populate `InferredDirectories` // so we don't need to do a full load here. - parseAndLoadModuleMapFile(*ModMapFile, Attrs.IsSystem, *ParentDir); + parseAndLoadModuleMapFile(*ModMapFile, Attrs.IsSystem, + /*ImplicitlyDiscovered=*/true, + *ParentDir); inferred = InferredDirectories.find(*ParentDir); } @@ -1333,6 +1335,7 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header, } bool ModuleMap::parseModuleMapFile(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, DirectoryEntryRef Dir, FileID ID, SourceLocation ExternModuleLoc) { llvm::DenseMap::iterator @@ -1358,8 +1361,8 @@ bool ModuleMap::parseModuleMapFile(FileEntryRef File, bool IsSystem, } Diags.Report(diag::remark_mmap_parse) << File.getName(); - std::optional MaybeMMF = - modulemap::parseModuleMap(ID, Dir, SourceMgr, Diags, IsSystem, nullptr); + std::optional MaybeMMF = modulemap::parseModuleMap( + ID, Dir, SourceMgr, Diags, IsSystem, ImplicitlyDiscovered, nullptr); if (!MaybeMMF) { ParsedModuleMap[File] = nullptr; @@ -1418,8 +1421,8 @@ bool ModuleMap::parseModuleMapFile(FileEntryRef File, bool IsSystem, if (auto EFile = SourceMgr.getFileManager().getOptionalFileRef(FileNameRef)) { - parseModuleMapFile(*EFile, IsSystem, EFile->getDir(), FileID(), - ExternModuleLoc); + parseModuleMapFile(*EFile, IsSystem, ImplicitlyDiscovered, + EFile->getDir(), FileID(), ExternModuleLoc); } } @@ -1592,6 +1595,8 @@ class ModuleMapLoader { /// Whether this module map is in a system header directory. bool IsSystem; + bool ImplicitlyDiscovered; + /// Whether an error occurred. bool HadError = false; @@ -1632,9 +1637,11 @@ class ModuleMapLoader { public: ModuleMapLoader(SourceManager &SourceMgr, DiagnosticsEngine &Diags, ModuleMap &Map, FileID ModuleMapFID, - DirectoryEntryRef Directory, bool IsSystem) + DirectoryEntryRef Directory, bool IsSystem, + bool ImplicitlyDiscovered) : SourceMgr(SourceMgr), Diags(Diags), Map(Map), - ModuleMapFID(ModuleMapFID), Directory(Directory), IsSystem(IsSystem) {} + ModuleMapFID(ModuleMapFID), Directory(Directory), IsSystem(IsSystem), + ImplicitlyDiscovered(ImplicitlyDiscovered) {} bool loadModuleDecl(const modulemap::ModuleDecl &MD); bool loadExternModuleDecl(const modulemap::ExternModuleDecl &EMD); @@ -1895,7 +1902,7 @@ void ModuleMapLoader::handleExternModuleDecl( } if (auto File = SourceMgr.getFileManager().getOptionalFileRef(FileNameRef)) Map.parseAndLoadModuleMapFile( - *File, IsSystem, + *File, IsSystem, ImplicitlyDiscovered, Map.HeaderInfo.getHeaderSearchOpts().ModuleMapFileHomeIsCwd ? Directory : File->getDir(), @@ -1983,6 +1990,13 @@ void ModuleMapLoader::handleHeaderDecl(const modulemap::HeaderDecl &HD) { return; } + if (ImplicitlyDiscovered) { + SmallString<128> NormalizedPath(HD.Path); + llvm::sys::path::remove_dots(NormalizedPath, /*remove_dot_dot=*/true); + if (NormalizedPath.starts_with("..")) + Diags.Report(HD.PathLoc, diag::warn_mmap_path_outside_directory); + } + if (HD.Size) Header.Size = HD.Size; if (HD.MTime) @@ -2021,6 +2035,13 @@ void ModuleMapLoader::handleUmbrellaDirDecl( return; } + if (ImplicitlyDiscovered) { + SmallString<128> NormalizedPath(UDD.Path); + llvm::sys::path::remove_dots(NormalizedPath, /*remove_dot_dot=*/true); + if (NormalizedPath.starts_with("..")) + Diags.Report(UDD.Location, diag::warn_mmap_path_outside_directory); + } + // Look for this file. OptionalDirectoryEntryRef Dir; if (llvm::sys::path::is_absolute(DirName)) { @@ -2264,7 +2285,8 @@ Module *ModuleMap::findOrLoadModule(StringRef Name) { for (const auto &ModuleDecl : ParsedMod->second) { const modulemap::ModuleMapFile &MMF = *ModuleDecl.first; ModuleMapLoader Loader(SourceMgr, Diags, const_cast(*this), - MMF.ID, *MMF.Dir, MMF.IsSystem); + MMF.ID, *MMF.Dir, MMF.IsSystem, + MMF.ImplicitlyDiscovered); if (Loader.loadModuleDecl(*ModuleDecl.second)) return nullptr; } @@ -2273,6 +2295,7 @@ Module *ModuleMap::findOrLoadModule(StringRef Name) { } bool ModuleMap::parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, + bool ImplicitlyDiscovered, DirectoryEntryRef Dir, FileID ID, unsigned *Offset, SourceLocation ExternModuleLoc) { @@ -2304,12 +2327,13 @@ bool ModuleMap::parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem, assert((!Offset || *Offset <= Buffer->getBufferSize()) && "invalid buffer offset"); - std::optional MMF = - modulemap::parseModuleMap(ID, Dir, SourceMgr, Diags, IsSystem, Offset); + std::optional MMF = modulemap::parseModuleMap( + ID, Dir, SourceMgr, Diags, IsSystem, ImplicitlyDiscovered, Offset); bool Result = false; if (MMF) { Diags.Report(diag::remark_mmap_load) << File.getName(); - ModuleMapLoader Loader(SourceMgr, Diags, *this, ID, Dir, IsSystem); + ModuleMapLoader Loader(SourceMgr, Diags, *this, ID, Dir, IsSystem, + ImplicitlyDiscovered); Result = Loader.parseAndLoadModuleMapFile(*MMF); } LoadedModuleMap[File] = Result; diff --git a/clang/lib/Lex/ModuleMapFile.cpp b/clang/lib/Lex/ModuleMapFile.cpp index 183e919d14c2..4ca33cb86ddd 100644 --- a/clang/lib/Lex/ModuleMapFile.cpp +++ b/clang/lib/Lex/ModuleMapFile.cpp @@ -147,7 +147,8 @@ std::string formatModuleId(const ModuleId &Id) { std::optional modulemap::parseModuleMap(FileID ID, clang::DirectoryEntryRef Dir, SourceManager &SM, DiagnosticsEngine &Diags, - bool IsSystem, unsigned *Offset) { + bool IsSystem, bool ImplicitlyDiscovered, + unsigned *Offset) { std::optional Buffer = SM.getBufferOrNone(ID); LangOptions LOpts; LOpts.LangStd = clang::LangStandard::lang_c99; @@ -171,6 +172,7 @@ modulemap::parseModuleMap(FileID ID, clang::DirectoryEntryRef Dir, Parser.MMF.Dir = Dir; Parser.MMF.Start = Start; Parser.MMF.IsSystem = IsSystem; + Parser.MMF.ImplicitlyDiscovered = ImplicitlyDiscovered; return std::move(Parser.MMF); } diff --git a/clang/test/Modules/deprecated-upwards-relative-path.m b/clang/test/Modules/deprecated-upwards-relative-path.m new file mode 100644 index 000000000000..1075be55f4c7 --- /dev/null +++ b/clang/test/Modules/deprecated-upwards-relative-path.m @@ -0,0 +1,81 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// Test that we warn on upwards relative paths in implicitly discovered module maps. +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -I%t/sub \ +// RUN: -fmodules-cache-path=%t/cache %t/tu.m -fsyntax-only \ +// RUN: -Wmodule-map-path-outside-directory -verify + +// Test that we don't warn when the module map is explicitly specified. +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/sub/module.modulemap \ +// RUN: -fmodules-cache-path=%t/cache2 %t/tu-no-warn.m -fsyntax-only \ +// RUN: -Wmodule-map-path-outside-directory -verify + +// Test umbrella header with upwards relative path. +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -I%t/umbrella-header/sub \ +// RUN: -fmodules-cache-path=%t/cache3 %t/umbrella-header/tu.m -fsyntax-only \ +// RUN: -Wmodule-map-path-outside-directory -verify + +// Test umbrella directory with upwards relative path. +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -I%t/umbrella-dir/sub \ +// RUN: -fmodules-cache-path=%t/cache4 %t/umbrella-dir/tu.m -fsyntax-only \ +// RUN: -Wmodule-map-path-outside-directory -verify + +// Test that interior .. that escapes the directory is caught. +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -I%t/interior/sub \ +// RUN: -fmodules-cache-path=%t/cache5 %t/interior/tu.m -fsyntax-only \ +// RUN: -Wmodule-map-path-outside-directory -verify + +//--- a.h +void a(void); + +//--- sub/module.modulemap +module A { + header "../a.h" +} + +//--- tu.m +@import A; +// expected-warning@*{{path refers outside of the module directory; such paths in implicitly discovered module maps are deprecated}} + +//--- tu-no-warn.m +// expected-no-diagnostics +@import A; + +//--- umbrella-header/inc/b.h +void b(void); + +//--- umbrella-header/sub/module.modulemap +module B { + umbrella header "../inc/b.h" +} + +//--- umbrella-header/tu.m +@import B; +// expected-warning@*{{path refers outside of the module directory; such paths in implicitly discovered module maps are deprecated}} + +//--- umbrella-dir/inc/c.h +void c(void); + +//--- umbrella-dir/sub/module.modulemap +module C { + umbrella "../inc" +} + +//--- umbrella-dir/tu.m +@import C; +// expected-warning@*{{path refers outside of the module directory; such paths in implicitly discovered module maps are deprecated}} + +//--- interior/d.h +void d(void); + +//--- interior/sub/inner/placeholder.h + +//--- interior/sub/module.modulemap +module D { + header "inner/../../d.h" +} + +//--- interior/tu.m +@import D; +// expected-warning@*{{path refers outside of the module directory; such paths in implicitly discovered module maps are deprecated}} diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp index 1276abe09c63..2c520f75f84b 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp @@ -329,7 +329,8 @@ ClangModulesDeclVendorImpl::AddModule(const SourceModule &module, return llvm::createStringError("couldn't find modulemap file in %s", module.search_path.GetCString()); - if (HS.parseAndLoadModuleMapFile(*file, is_system)) + if (HS.parseAndLoadModuleMapFile(*file, is_system, + /*ImplicitlyDiscovered=*/false)) return llvm::createStringError( "failed to parse and load modulemap file in %s", module.search_path.GetCString());