Skip to content

Commit bb26838

Browse files
committed
[clang driver] Move default module cache from system temporary directory
1) Shared writable directories like /tmp are a security problem. 2) Systems provide dedicated cache directories these days anyway. 3) This also refines LLVM's cache_directory() on Darwin platforms to use the Darwin per-user cache directory. Reviewers: compnerd, aprantl, jakehehrlich, espindola, respindola, ilya-biryukov, pcc, sammccall Reviewed By: compnerd, sammccall Subscribers: hiraditya, llvm-commits, cfe-commits Tags: #clang, #llvm Differential Revision: https://reviews.llvm.org/D82362
1 parent da2f852 commit bb26838

File tree

6 files changed

+45
-60
lines changed

6 files changed

+45
-60
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ New Compiler Flags
104104
simplify access to the many single purpose floating point options. The default
105105
setting is ``precise``.
106106

107+
- The default module cache has moved from /tmp to a per-user cache directory.
108+
By default, this is ~/.cache but on some platforms or installations, this
109+
might be elsewhere. The -fmodules-cache-path=... flag continues to work.
110+
107111
Deprecated Compiler Flags
108112
-------------------------
109113

clang/include/clang/Driver/Driver.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,8 @@ class Driver {
621621
static bool GetReleaseVersion(StringRef Str,
622622
MutableArrayRef<unsigned> Digits);
623623
/// Compute the default -fmodule-cache-path.
624-
static void getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
624+
/// \return True if the system provides a default cache directory.
625+
static bool getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
625626
};
626627

627628
/// \return True if the last defined optimization level is -Ofast.

clang/lib/Driver/ToolChains/Clang.cpp

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -721,38 +721,6 @@ static void addDashXForInput(const ArgList &Args, const InputInfo &Input,
721721
}
722722
}
723723

724-
static void appendUserToPath(SmallVectorImpl<char> &Result) {
725-
#ifdef LLVM_ON_UNIX
726-
const char *Username = getenv("LOGNAME");
727-
#else
728-
const char *Username = getenv("USERNAME");
729-
#endif
730-
if (Username) {
731-
// Validate that LoginName can be used in a path, and get its length.
732-
size_t Len = 0;
733-
for (const char *P = Username; *P; ++P, ++Len) {
734-
if (!clang::isAlphanumeric(*P) && *P != '_') {
735-
Username = nullptr;
736-
break;
737-
}
738-
}
739-
740-
if (Username && Len > 0) {
741-
Result.append(Username, Username + Len);
742-
return;
743-
}
744-
}
745-
746-
// Fallback to user id.
747-
#ifdef LLVM_ON_UNIX
748-
std::string UID = llvm::utostr(getuid());
749-
#else
750-
// FIXME: Windows seems to have an 'SID' that might work.
751-
std::string UID = "9999";
752-
#endif
753-
Result.append(UID.begin(), UID.end());
754-
}
755-
756724
static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
757725
const Driver &D, const InputInfo &Output,
758726
const ArgList &Args,
@@ -3201,11 +3169,13 @@ static void RenderBuiltinOptions(const ToolChain &TC, const llvm::Triple &T,
32013169
CmdArgs.push_back("-fno-math-builtin");
32023170
}
32033171

3204-
void Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
3205-
llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
3206-
llvm::sys::path::append(Result, "org.llvm.clang.");
3207-
appendUserToPath(Result);
3208-
llvm::sys::path::append(Result, "ModuleCache");
3172+
bool Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
3173+
if (llvm::sys::path::cache_directory(Result)) {
3174+
llvm::sys::path::append(Result, "clang");
3175+
llvm::sys::path::append(Result, "ModuleCache");
3176+
return true;
3177+
}
3178+
return false;
32093179
}
32103180

32113181
static void RenderModulesOptions(Compilation &C, const Driver &D,
@@ -3262,6 +3232,7 @@ static void RenderModulesOptions(Compilation &C, const Driver &D,
32623232
if (Arg *A = Args.getLastArg(options::OPT_fmodules_cache_path))
32633233
Path = A->getValue();
32643234

3235+
bool HasPath = true;
32653236
if (C.isForDiagnostics()) {
32663237
// When generating crash reports, we want to emit the modules along with
32673238
// the reproduction sources, so we ignore any provided module path.
@@ -3270,12 +3241,16 @@ static void RenderModulesOptions(Compilation &C, const Driver &D,
32703241
llvm::sys::path::append(Path, "modules");
32713242
} else if (Path.empty()) {
32723243
// No module path was provided: use the default.
3273-
Driver::getDefaultModuleCachePath(Path);
3244+
HasPath = Driver::getDefaultModuleCachePath(Path);
32743245
}
32753246

3276-
const char Arg[] = "-fmodules-cache-path=";
3277-
Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
3278-
CmdArgs.push_back(Args.MakeArgString(Path));
3247+
// `HasPath` will only be false if getDefaultModuleCachePath() fails.
3248+
// That being said, that failure is unlikely and not caching is harmless.
3249+
if (HasPath) {
3250+
const char Arg[] = "-fmodules-cache-path=";
3251+
Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
3252+
CmdArgs.push_back(Args.MakeArgString(Path));
3253+
}
32793254
}
32803255

32813256
if (HaveModules) {
Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,2 @@
1-
// RUN: env USERNAME=asdf LOGNAME=asdf %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-SET
2-
// CHECK-SET: -fmodules-cache-path={{.*}}org.llvm.clang.asdf{{[/\\]+}}ModuleCache
3-
41
// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
5-
// CHECK-DEFAULT: -fmodules-cache-path={{.*}}org.llvm.clang.{{[A-Za-z0-9_]*[/\\]+}}ModuleCache
2+
// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache

clang/unittests/Driver/ModuleCacheTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ TEST(ModuleCacheTest, GetTargetAndMode) {
2121
SmallString<128> Buf;
2222
Driver::getDefaultModuleCachePath(Buf);
2323
StringRef Path = Buf;
24-
EXPECT_TRUE(Path.find("org.llvm.clang") != Path.npos);
24+
EXPECT_TRUE(Path.find("clang") != Path.npos);
2525
EXPECT_TRUE(Path.endswith("ModuleCache"));
2626
}
2727
} // end anonymous namespace.

llvm/lib/Support/Unix/Path.inc

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,19 +1133,6 @@ bool home_directory(SmallVectorImpl<char> &result) {
11331133
return true;
11341134
}
11351135

1136-
bool cache_directory(SmallVectorImpl<char> &result) {
1137-
if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
1138-
result.clear();
1139-
result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
1140-
return true;
1141-
}
1142-
if (!home_directory(result)) {
1143-
return false;
1144-
}
1145-
append(result, ".cache");
1146-
return true;
1147-
}
1148-
11491136
static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
11501137
#if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
11511138
// On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.
@@ -1171,6 +1158,27 @@ static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
11711158
return false;
11721159
}
11731160

1161+
bool cache_directory(SmallVectorImpl<char> &result) {
1162+
#ifdef __APPLE__
1163+
if (getDarwinConfDir(false/*tempDir*/, result)) {
1164+
return true;
1165+
}
1166+
#else
1167+
// XDG_CACHE_HOME as defined in the XDG Base Directory Specification:
1168+
// http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
1169+
if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
1170+
result.clear();
1171+
result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
1172+
return true;
1173+
}
1174+
#endif
1175+
if (!home_directory(result)) {
1176+
return false;
1177+
}
1178+
append(result, ".cache");
1179+
return true;
1180+
}
1181+
11741182
static const char *getEnvTempDir() {
11751183
// Check whether the temporary directory is specified by an environment
11761184
// variable.

0 commit comments

Comments
 (0)