Skip to content

Commit

Permalink
[clang] use relative paths for builtin headers during module compilat…
Browse files Browse the repository at this point in the history
…ion (#68023)

When including builtin headers as part of a system module, ensure we use
relative paths to those headers. Otherwise the module will fail to compile 
when specifying relative resource directories without extra search paths.
  • Loading branch information
rmaz committed Oct 27, 2023
1 parent e9478b1 commit 396b562
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 3 deletions.
5 changes: 4 additions & 1 deletion clang/include/clang/Lex/ModuleMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,16 @@ class ModuleMap {
}

/// Get the directory that contains Clang-supplied include files.
const DirectoryEntry *getBuiltinDir() const {
OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr getBuiltinDir() const {
return BuiltinIncludeDir;
}

/// Is this a compiler builtin header?
bool isBuiltinHeader(FileEntryRef File);

bool shouldImportRelativeToBuiltinIncludeDir(StringRef FileName,
Module *Module) const;

/// Add a module map callback.
void addModuleMapCallbacks(std::unique_ptr<ModuleMapCallbacks> Callback) {
Callbacks.push_back(std::move(Callback));
Expand Down
9 changes: 8 additions & 1 deletion clang/lib/Lex/ModuleMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ bool ModuleMap::resolveAsBuiltinHeader(
if (!File)
return false;

Module::Header H = {Header.FileName, Header.FileName, *File};
auto Role = headerKindToRole(Header.Kind);
Module::Header H = {Header.FileName, std::string(Path.str()), *File};
addHeader(Mod, H, Role);
return true;
}
Expand Down Expand Up @@ -417,6 +417,13 @@ bool ModuleMap::isBuiltinHeader(FileEntryRef File) {
isBuiltinHeaderName(llvm::sys::path::filename(File.getName()));
}

bool ModuleMap::shouldImportRelativeToBuiltinIncludeDir(StringRef FileName,
Module *Module) const {
return LangOpts.BuiltinHeadersInSystemModules && BuiltinIncludeDir &&
Module->IsSystem && !Module->isPartOfFramework() &&
isBuiltinHeaderName(FileName);
}

ModuleMap::HeadersMap::iterator ModuleMap::findKnownHeader(FileEntryRef File) {
resolveHeaderDirectives(File);
HeadersMap::iterator Known = Headers.find(File);
Expand Down
9 changes: 8 additions & 1 deletion clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//

#include "clang/Basic/CharInfo.h"
#include "clang/Basic/DirectoryEntry.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
Expand All @@ -21,6 +22,7 @@
#include "clang/Basic/TokenKinds.h"
#include "clang/Lex/CodeCompletionHandler.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/HeaderSearchOptions.h"
#include "clang/Lex/LexDiagnostic.h"
#include "clang/Lex/LiteralSupport.h"
#include "clang/Lex/MacroInfo.h"
Expand Down Expand Up @@ -982,7 +984,12 @@ OptionalFileEntryRef Preprocessor::LookupFile(
// map file.
if (!FileEnt) {
if (FID == SourceMgr.getMainFileID() && MainFileDir) {
Includers.push_back(std::make_pair(std::nullopt, *MainFileDir));
auto IncludeDir =
HeaderInfo.getModuleMap().shouldImportRelativeToBuiltinIncludeDir(
Filename, getCurrentModule())
? HeaderInfo.getModuleMap().getBuiltinDir()
: MainFileDir;
Includers.push_back(std::make_pair(std::nullopt, *IncludeDir));
BuildSystemModule = getCurrentModule()->IsSystem;
} else if ((FileEnt = SourceMgr.getFileEntryRefForID(
SourceMgr.getMainFileID()))) {
Expand Down
3 changes: 3 additions & 0 deletions clang/test/Modules/Inputs/builtin-headers/module.modulemap
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module ModuleWithBuiltinHeader [system] {
header "float.h"
}
11 changes: 11 additions & 0 deletions clang/test/Modules/relative-resource-dir.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// REQUIRES: shell

// RUN: EXPECTED_RESOURCE_DIR=`%clang -print-resource-dir` && \
// RUN: mkdir -p %t && rm -rf %t/resource-dir && \
// RUN: cp -R $EXPECTED_RESOURCE_DIR %t/resource-dir
// RUN: cd %t && %clang -cc1 -x objective-c -fmodules -fmodule-format=obj \
// RUN: -fimplicit-module-maps -fmodules-cache-path=%t.mcp \
// RUN: -fbuiltin-headers-in-system-modules \
// RUN: -resource-dir resource-dir \
// RUN: -emit-module %S/Inputs/builtin-headers/module.modulemap \
// RUN: -fmodule-name=ModuleWithBuiltinHeader -o %t.pcm

1 comment on commit 396b562

@amy-kwan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!
I believe this patch is breaking the clang-ppc64-aix bot in https://lab.llvm.org/buildbot/#/builders/214/builds/10023:

******************** TEST 'Clang :: Modules/relative-resource-dir.m' FAILED ********************
Exit Code: 70
Command Output (stderr):
--
RUN: at line 3: EXPECTED_RESOURCE_DIR=`/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/clang -print-resource-dir` &&   mkdir -p /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp && rm -rf /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp/resource-dir &&   cp -R $EXPECTED_RESOURCE_DIR /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp/resource-dir
++ /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/clang -print-resource-dir
+ EXPECTED_RESOURCE_DIR=/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/lib/clang/18
+ mkdir -p /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp
+ rm -rf /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp/resource-dir
+ cp -R /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/lib/clang/18 /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp/resource-dir
RUN: at line 6: cd /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp && /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/clang -cc1 -x objective-c -fmodules -fmodule-format=obj    -fimplicit-module-maps -fmodules-cache-path=/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp.mcp    -fbuiltin-headers-in-system-modules    -resource-dir resource-dir    -emit-module /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/test/Modules/Inputs/builtin-headers/module.modulemap    -fmodule-name=ModuleWithBuiltinHeader -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp.pcm
+ cd /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp
+ /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/bin/clang -cc1 -x objective-c -fmodules -fmodule-format=obj -fimplicit-module-maps -fmodules-cache-path=/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp.mcp -fbuiltin-headers-in-system-modules -resource-dir resource-dir -emit-module /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/test/Modules/Inputs/builtin-headers/module.modulemap -fmodule-name=ModuleWithBuiltinHeader -o /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/test/Modules/Output/relative-resource-dir.m.tmp.pcm
fatal error: error in backend: Objective-C support is unimplemented for object file format
--
********************

Would you be able to investigate this?

Please sign in to comment.