https://github.com/vgvassilev created https://github.com/llvm/llvm-project/pull/108529
Posix call to ::getenv is not guarenteed to be thread safe while C++11 made std::getenv thread safe. This resolves bugs when using llvm in multithreaded environment similar to cms-sw/cmssw#44659 >From 4ae2832068848ea84bae0dd16e6ad40cb5207f4f Mon Sep 17 00:00:00 2001 From: Vassil Vassilev <v.g.vassi...@gmail.com> Date: Fri, 13 Sep 2024 10:37:36 +0000 Subject: [PATCH] Prefer std::getenv to ::getenv Posix call to ::getenv is not guarenteed to be thread safe while C++11 made std::getenv thread safe. This resolves bugs when using llvm in multithreaded environment similar to cms-sw/cmssw#44659 --- clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp | 2 +- clang/lib/Driver/Driver.cpp | 2 +- clang/lib/Driver/ToolChains/CommonArgs.cpp | 2 +- clang/lib/Driver/ToolChains/Darwin.cpp | 8 ++++---- clang/lib/Frontend/PrecompiledPreamble.cpp | 2 +- clang/tools/clang-installapi/Options.cpp | 6 +++--- clang/tools/driver/driver.cpp | 12 ++++++------ clang/tools/libclang/ARCMigrate.cpp | 4 ++-- clang/tools/libclang/CLog.h | 2 +- llvm/lib/Support/Unix/Path.inc | 2 +- llvm/lib/Support/Unix/Process.inc | 2 +- llvm/unittests/Support/Path.cpp | 4 ++-- 12 files changed, 24 insertions(+), 24 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp index a36cb41a63dfb1..8d6590fab281c2 100644 --- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp +++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp @@ -522,7 +522,7 @@ class HTMLLogger : public Logger { // Nothing interesting here, just subprocess/temp-file plumbing. llvm::Expected<std::string> renderSVG(llvm::StringRef DotGraph) { std::string DotPath; - if (const auto *FromEnv = ::getenv("GRAPHVIZ_DOT")) + if (const auto *FromEnv = std::getenv("GRAPHVIZ_DOT")) DotPath = FromEnv; else { auto FromPath = llvm::sys::findProgramByName("dot"); diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e12416e51f8d24..4b65bff86393cd 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1124,7 +1124,7 @@ bool Driver::loadConfigFiles() { bool Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) { // Disable default config if CLANG_NO_DEFAULT_CONFIG is set to a non-empty // value. - if (const char *NoConfigEnv = ::getenv("CLANG_NO_DEFAULT_CONFIG")) { + if (const char *NoConfigEnv = std::getenv("CLANG_NO_DEFAULT_CONFIG")) { if (*NoConfigEnv) return false; } diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 320d2901da06ed..8c65c7d2d3d3e5 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -390,7 +390,7 @@ tools::unifyTargetFeatures(ArrayRef<StringRef> Features) { void tools::addDirectoryList(const ArgList &Args, ArgStringList &CmdArgs, const char *ArgName, const char *EnvVar) { - const char *DirList = ::getenv(EnvVar); + const char *DirList = std::getenv(EnvVar); bool CombinedArg = false; if (!DirList) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index 2550541a438481..0c7fcd54fb7d21 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1936,7 +1936,7 @@ getDeploymentTargetFromEnvironmentVariables(const Driver &TheDriver, static_assert(std::size(EnvVars) == Darwin::LastDarwinPlatform + 1, "Missing platform"); for (const auto &I : llvm::enumerate(llvm::ArrayRef(EnvVars))) { - if (char *Env = ::getenv(I.value())) + if (char *Env = std::getenv(I.value())) Targets[I.index()] = Env; } @@ -2217,7 +2217,7 @@ void Darwin::AddDeploymentTarget(DerivedArgList &Args) const { if (!getVFS().exists(A->getValue())) getDriver().Diag(clang::diag::warn_missing_sysroot) << A->getValue(); } else { - if (char *env = ::getenv("SDKROOT")) { + if (char *env = std::getenv("SDKROOT")) { // We only use this value as the default if it is an absolute path, // exists, and it is not the root path. if (llvm::sys::path::is_absolute(env) && getVFS().exists(env) && @@ -3217,13 +3217,13 @@ ToolChain::UnwindTableLevel MachO::getDefaultUnwindTableLevel(const ArgList &Arg } bool MachO::UseDwarfDebugFlags() const { - if (const char *S = ::getenv("RC_DEBUG_OPTIONS")) + if (const char *S = std::getenv("RC_DEBUG_OPTIONS")) return S[0] != '\0'; return false; } std::string MachO::GetGlobalDebugPathRemapping() const { - if (const char *S = ::getenv("RC_DEBUG_PREFIX_MAP")) + if (const char *S = std::getenv("RC_DEBUG_PREFIX_MAP")) return S; return {}; } diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp index cab5838fceb24d..cf3999341dc2d4 100644 --- a/clang/lib/Frontend/PrecompiledPreamble.cpp +++ b/clang/lib/Frontend/PrecompiledPreamble.cpp @@ -203,7 +203,7 @@ class TempPCHFile { // FIXME: This is a hack so that we can override the preamble file during // crash-recovery testing, which is the only case where the preamble files // are not necessarily cleaned up. - if (const char *TmpFile = ::getenv("CINDEXTEST_PREAMBLE_FILE")) + if (const char *TmpFile = std::getenv("CINDEXTEST_PREAMBLE_FILE")) return std::unique_ptr<TempPCHFile>(new TempPCHFile(TmpFile)); llvm::SmallString<128> File; diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp index 1ca1d583d5ccdf..f977275357830d 100644 --- a/clang/tools/clang-installapi/Options.cpp +++ b/clang/tools/clang-installapi/Options.cpp @@ -455,10 +455,10 @@ bool Options::processLinkerOptions(InputArgList &Args) { drv::OPT_fapplication_extension, drv::OPT_fno_application_extension, /*Default=*/LinkerOpts.AppExtensionSafe); - if (::getenv("LD_NO_ENCRYPT") != nullptr) + if (std::getenv("LD_NO_ENCRYPT") != nullptr) LinkerOpts.AppExtensionSafe = true; - if (::getenv("LD_APPLICATION_EXTENSION_SAFE") != nullptr) + if (std::getenv("LD_APPLICATION_EXTENSION_SAFE") != nullptr) LinkerOpts.AppExtensionSafe = true; // Capture library paths. @@ -511,7 +511,7 @@ bool Options::processFrontendOptions(InputArgList &Args) { } else if (FEOpts.ISysroot.empty()) { // Mirror CLANG and obtain the isysroot from the SDKROOT environment // variable, if it wasn't defined by the command line. - if (auto *Env = ::getenv("SDKROOT")) { + if (auto *Env = std::getenv("SDKROOT")) { if (StringRef(Env) != "/" && llvm::sys::path::is_absolute(Env) && FM->getOptionalFileRef(Env)) FEOpts.ISysroot = Env; diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 83b5bbb71f5212..4a7a72e0e539c9 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -121,12 +121,12 @@ static void getCLEnvVarOptions(std::string &EnvValue, llvm::StringSaver &Saver, template <class T> static T checkEnvVar(const char *EnvOptSet, const char *EnvOptFile, std::string &OptFile) { - const char *Str = ::getenv(EnvOptSet); + const char *Str = std::getenv(EnvOptSet); if (!Str) return T{}; T OptVal = Str; - if (const char *Var = ::getenv(EnvOptFile)) + if (const char *Var = std::getenv(EnvOptFile)) OptFile = Var; return OptVal; } @@ -152,7 +152,7 @@ static bool SetBackdoorDriverOutputsFromEnvVars(Driver &TheDriver) { return false; } - const char *FilteringStr = ::getenv("CC_PRINT_HEADERS_FILTERING"); + const char *FilteringStr = std::getenv("CC_PRINT_HEADERS_FILTERING"); HeaderIncludeFilteringKind Filtering; if (!stringToHeaderIncludeFiltering(FilteringStr, Filtering)) { TheDriver.Diag(clang::diag::err_drv_print_header_env_var) @@ -294,7 +294,7 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) { llvm::StringSet<> SavedStrings; // Handle CCC_OVERRIDE_OPTIONS, used for editing a command line behind the // scenes. - if (const char *OverrideStr = ::getenv("CCC_OVERRIDE_OPTIONS")) { + if (const char *OverrideStr = std::getenv("CCC_OVERRIDE_OPTIONS")) { // FIXME: Driver shouldn't take extra initial argument. driver::applyOverrideOptions(Args, OverrideStr, SavedStrings, &llvm::errs()); @@ -376,7 +376,7 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) { } ReproLevel = *Level; } - if (!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) + if (!!std::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) ReproLevel = Driver::ReproLevel::Always; int Res = 1; @@ -420,7 +420,7 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) { // Print the bug report message that would be printed if we did actually // crash, but only if we're crashing due to FORCE_CLANG_DIAGNOSTICS_CRASH. - if (::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) + if (std::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) llvm::dbgs() << llvm::getBugReportMsg(); if (FailingCommand != nullptr && TheDriver.maybeGenerateCompilationDiagnostics(CommandStatus, ReproLevel, diff --git a/clang/tools/libclang/ARCMigrate.cpp b/clang/tools/libclang/ARCMigrate.cpp index da8a7e4b9130b9..befaa7d98ec4be 100644 --- a/clang/tools/libclang/ARCMigrate.cpp +++ b/clang/tools/libclang/ARCMigrate.cpp @@ -37,7 +37,7 @@ CXRemapping clang_getRemappings(const char *migrate_dir_path) { llvm::errs() << "error: feature not enabled in this build\n"; return nullptr; #else - bool Logging = ::getenv("LIBCLANG_LOGGING"); + bool Logging = std::getenv("LIBCLANG_LOGGING"); if (!migrate_dir_path) { if (Logging) @@ -80,7 +80,7 @@ CXRemapping clang_getRemappingsFromFileList(const char **filePaths, llvm::errs() << "error: feature not enabled in this build\n"; return nullptr; #else - bool Logging = ::getenv("LIBCLANG_LOGGING"); + bool Logging = std::getenv("LIBCLANG_LOGGING"); std::unique_ptr<Remap> remap(new Remap()); diff --git a/clang/tools/libclang/CLog.h b/clang/tools/libclang/CLog.h index 6ce43a01ee8f2d..aeac62fe0dacff 100644 --- a/clang/tools/libclang/CLog.h +++ b/clang/tools/libclang/CLog.h @@ -43,7 +43,7 @@ class Logger : public RefCountedBase<Logger> { llvm::raw_svector_ostream LogOS; public: static const char *getEnvVar() { - static const char *sCachedVar = ::getenv("LIBCLANG_LOGGING"); + static const char *sCachedVar = std::getenv("LIBCLANG_LOGGING"); return sCachedVar; } static bool isLoggingEnabled() { return getEnvVar() != nullptr; } diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc index cf05db546e0214..3f29f966bf5f0c 100644 --- a/llvm/lib/Support/Unix/Path.inc +++ b/llvm/lib/Support/Unix/Path.inc @@ -369,7 +369,7 @@ ErrorOr<space_info> disk_space(const Twine &Path) { std::error_code current_path(SmallVectorImpl<char> &result) { result.clear(); - const char *pwd = ::getenv("PWD"); + const char *pwd = std::getenv("PWD"); llvm::sys::fs::file_status PWDStatus, DotStatus; if (pwd && llvm::sys::path::is_absolute(pwd) && !llvm::sys::fs::status(pwd, PWDStatus) && diff --git a/llvm/lib/Support/Unix/Process.inc b/llvm/lib/Support/Unix/Process.inc index 84b10ff5d1d08a..f24972f541f0f3 100644 --- a/llvm/lib/Support/Unix/Process.inc +++ b/llvm/lib/Support/Unix/Process.inc @@ -199,7 +199,7 @@ void Process::PreventCoreFiles() { std::optional<std::string> Process::GetEnv(StringRef Name) { std::string NameStr = Name.str(); - const char *Val = ::getenv(NameStr.c_str()); + const char *Val = std::getenv(NameStr.c_str()); if (!Val) return std::nullopt; return std::string(Val); diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 463a6991dac515..24b18865d03b0e 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -459,7 +459,7 @@ class WithEnv { public: WithEnv(const char *Var, const char *Value) : Var(Var) { - if (const char *V = ::getenv(Var)) + if (const char *V = std::getenv(Var)) OriginalValue.emplace(V); if (Value) ::setenv(Var, Value, 1); @@ -480,7 +480,7 @@ TEST(Support, HomeDirectory) { #ifdef _WIN32 expected = getEnvWin(L"USERPROFILE"); #else - if (char const *path = ::getenv("HOME")) + if (char const *path = std::getenv("HOME")) expected = path; #endif // Do not try to test it if we don't know what to expect. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits