davezarzycki updated this revision to Diff 272434.
davezarzycki added a comment.
1. Respect Darwin and XDG cache directories.
2. Drop atypical reverse DNS in the path.
3. Make clang more robust if the default module path cannot be determined.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82259/new/
https://reviews.llvm.org/D82259
Files:
include/clang/Driver/Driver.h
include/llvm/Support/FileSystem.h
include/llvm/Support/Path.h
lib/Driver/ToolChains/Clang.cpp
lib/Support/Path.cpp
lib/Support/Unix/Path.inc
source/Core/ModuleList.cpp
test/Driver/modules-cache-path.m
unittests/Driver/ModuleCacheTest.cpp
Index: lib/Support/Unix/Path.inc
===================================================================
--- lib/Support/Unix/Path.inc
+++ lib/Support/Unix/Path.inc
@@ -1133,19 +1133,6 @@
return true;
}
-bool cache_directory(SmallVectorImpl<char> &result) {
- if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
- result.clear();
- result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
- return true;
- }
- if (!home_directory(result)) {
- return false;
- }
- append(result, ".cache");
- return true;
-}
-
static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
#if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
// On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.
@@ -1171,6 +1158,27 @@
return false;
}
+bool cache_directory(SmallVectorImpl<char> &result) {
+#ifdef __APPLE__
+ if (getDarwinConfDir(false/*tempDir*/, result)) {
+ return true;
+ }
+#else
+ // XDG_CACHE_HOME as defined in the XDG Base Directory Specification:
+ // http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
+ if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
+ result.clear();
+ result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
+ return true;
+ }
+#endif
+ if (!home_directory(result)) {
+ return false;
+ }
+ append(result, ".cache");
+ return true;
+}
+
static const char *getEnvTempDir() {
// Check whether the temporary directory is specified by an environment
// variable.
Index: lib/Support/Path.cpp
===================================================================
--- lib/Support/Path.cpp
+++ lib/Support/Path.cpp
@@ -164,6 +164,41 @@
FS_Name
};
+// Avoid a deprecation warning by moving the createUniquePath body here.
+static void _createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
+ bool MakeAbsolute) {
+ SmallString<128> ModelStorage;
+ Model.toVector(ModelStorage);
+
+ if (MakeAbsolute) {
+ // Make model absolute by prepending a temp directory if it's not already.
+ if (!sys::path::is_absolute(Twine(ModelStorage))) {
+ SmallString<128> TDir;
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#endif
+ sys::path::system_temp_directory(true, TDir);
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+ sys::path::append(TDir, Twine(ModelStorage));
+ ModelStorage.swap(TDir);
+ }
+ }
+
+ ResultPath = ModelStorage;
+ ResultPath.push_back(0);
+ ResultPath.pop_back();
+
+ // Replace '%' with random chars.
+ for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
+ if (ModelStorage[i] == '%')
+ ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
+ }
+}
+
+
static std::error_code
createUniqueEntity(const Twine &Model, int &ResultFD,
SmallVectorImpl<char> &ResultPath, bool MakeAbsolute,
@@ -176,7 +211,7 @@
// Checking which is racy, so we try a number of times, then give up.
std::error_code EC;
for (int Retries = 128; Retries > 0; --Retries) {
- sys::fs::createUniquePath(Model, ResultPath, MakeAbsolute);
+ _createUniquePath(Model, ResultPath, MakeAbsolute);
// Try to open + create the file.
switch (Type) {
case FS_File: {
@@ -777,29 +812,8 @@
}
void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
- bool MakeAbsolute) {
- SmallString<128> ModelStorage;
- Model.toVector(ModelStorage);
-
- if (MakeAbsolute) {
- // Make model absolute by prepending a temp directory if it's not already.
- if (!sys::path::is_absolute(Twine(ModelStorage))) {
- SmallString<128> TDir;
- sys::path::system_temp_directory(true, TDir);
- sys::path::append(TDir, Twine(ModelStorage));
- ModelStorage.swap(TDir);
- }
- }
-
- ResultPath = ModelStorage;
- ResultPath.push_back(0);
- ResultPath.pop_back();
-
- // Replace '%' with random chars.
- for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
- if (ModelStorage[i] == '%')
- ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
- }
+ bool MakeAbsolute) {
+ return _createUniquePath(Model, ResultPath, MakeAbsolute);
}
std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
Index: include/llvm/Support/Path.h
===================================================================
--- include/llvm/Support/Path.h
+++ include/llvm/Support/Path.h
@@ -363,7 +363,10 @@
/// (e.g., TEMP on Windows, TMPDIR on *nix) to specify a temporary directory.
///
/// @param result Holds the resulting path name.
-void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result);
+LLVM_ATTRIBUTE_DEPRECATED(
+void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result),
+ "Only meant for debugging due to trivial security problems"
+);
/// Get the user's home directory.
///
Index: include/llvm/Support/FileSystem.h
===================================================================
--- include/llvm/Support/FileSystem.h
+++ include/llvm/Support/FileSystem.h
@@ -800,8 +800,11 @@
/// @param Model Name to base unique path off of.
/// @param ResultPath Set to the file's path.
/// @param MakeAbsolute Whether to use the system temp directory.
-void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
- bool MakeAbsolute);
+LLVM_ATTRIBUTE_DEPRECATED(
+ void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
+ bool MakeAbsolute),
+ "Use createUniqueFile() or createUniqueDirectory()"
+);
/// Create a uniquely named file.
///
Index: source/Core/ModuleList.cpp
===================================================================
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -82,8 +82,8 @@
[this] { UpdateSymlinkMappings(); });
llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ if (clang::driver::Driver::getDefaultModuleCachePath(path))
+ SetClangModulesCachePath(path);
}
bool ModuleListProperties::GetEnableExternalLookup() const {
Index: unittests/Driver/ModuleCacheTest.cpp
===================================================================
--- unittests/Driver/ModuleCacheTest.cpp
+++ unittests/Driver/ModuleCacheTest.cpp
@@ -21,7 +21,7 @@
SmallString<128> Buf;
Driver::getDefaultModuleCachePath(Buf);
StringRef Path = Buf;
- EXPECT_TRUE(Path.find("org.llvm.clang") != Path.npos);
+ EXPECT_TRUE(Path.find("clang") != Path.npos);
EXPECT_TRUE(Path.endswith("ModuleCache"));
}
} // end anonymous namespace.
Index: test/Driver/modules-cache-path.m
===================================================================
--- test/Driver/modules-cache-path.m
+++ test/Driver/modules-cache-path.m
@@ -1,5 +1,2 @@
-// RUN: env USERNAME=asdf LOGNAME=asdf %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-SET
-// CHECK-SET: -fmodules-cache-path={{.*}}org.llvm.clang.asdf{{[/\\]+}}ModuleCache
-
// RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
-// CHECK-DEFAULT: -fmodules-cache-path={{.*}}org.llvm.clang.{{[A-Za-z0-9_]*[/\\]+}}ModuleCache
+// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -733,38 +733,6 @@
}
}
-static void appendUserToPath(SmallVectorImpl<char> &Result) {
-#ifdef LLVM_ON_UNIX
- const char *Username = getenv("LOGNAME");
-#else
- const char *Username = getenv("USERNAME");
-#endif
- if (Username) {
- // Validate that LoginName can be used in a path, and get its length.
- size_t Len = 0;
- for (const char *P = Username; *P; ++P, ++Len) {
- if (!clang::isAlphanumeric(*P) && *P != '_') {
- Username = nullptr;
- break;
- }
- }
-
- if (Username && Len > 0) {
- Result.append(Username, Username + Len);
- return;
- }
- }
-
-// Fallback to user id.
-#ifdef LLVM_ON_UNIX
- std::string UID = llvm::utostr(getuid());
-#else
- // FIXME: Windows seems to have an 'SID' that might work.
- std::string UID = "9999";
-#endif
- Result.append(UID.begin(), UID.end());
-}
-
static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
const Driver &D, const InputInfo &Output,
const ArgList &Args,
@@ -3203,11 +3171,13 @@
CmdArgs.push_back("-fno-math-builtin");
}
-void Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
- llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
- llvm::sys::path::append(Result, "org.llvm.clang.");
- appendUserToPath(Result);
- llvm::sys::path::append(Result, "ModuleCache");
+bool Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
+ if (llvm::sys::path::cache_directory(Result)) {
+ llvm::sys::path::append(Result, "clang");
+ llvm::sys::path::append(Result, "ModuleCache");
+ return true;
+ }
+ return false;
}
static void RenderModulesOptions(Compilation &C, const Driver &D,
@@ -3264,6 +3234,7 @@
if (Arg *A = Args.getLastArg(options::OPT_fmodules_cache_path))
Path = A->getValue();
+ bool HasPath = true;
if (C.isForDiagnostics()) {
// When generating crash reports, we want to emit the modules along with
// the reproduction sources, so we ignore any provided module path.
@@ -3272,12 +3243,14 @@
llvm::sys::path::append(Path, "modules");
} else if (Path.empty()) {
// No module path was provided: use the default.
- Driver::getDefaultModuleCachePath(Path);
+ HasPath = Driver::getDefaultModuleCachePath(Path);
}
- const char Arg[] = "-fmodules-cache-path=";
- Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
- CmdArgs.push_back(Args.MakeArgString(Path));
+ if (HasPath) {
+ const char Arg[] = "-fmodules-cache-path=";
+ Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
+ CmdArgs.push_back(Args.MakeArgString(Path));
+ }
}
if (HaveModules) {
Index: include/clang/Driver/Driver.h
===================================================================
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -618,7 +618,8 @@
static bool GetReleaseVersion(StringRef Str,
MutableArrayRef<unsigned> Digits);
/// Compute the default -fmodule-cache-path.
- static void getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
+ /// \return True if the system provides a default cache directory.
+ static bool getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
};
/// \return True if the last defined optimization level is -Ofast.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
