[clang] [llvm] Prefer std::getenv to ::getenv (PR #108529)

via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 13 03:42:22 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-clang-driver

Author: Vassil Vassilev (vgvassilev)

<details>
<summary>Changes</summary>

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

---
Full diff: https://github.com/llvm/llvm-project/pull/108529.diff


12 Files Affected:

- (modified) clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp (+1-1) 
- (modified) clang/lib/Driver/Driver.cpp (+1-1) 
- (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1-1) 
- (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+4-4) 
- (modified) clang/lib/Frontend/PrecompiledPreamble.cpp (+1-1) 
- (modified) clang/tools/clang-installapi/Options.cpp (+3-3) 
- (modified) clang/tools/driver/driver.cpp (+6-6) 
- (modified) clang/tools/libclang/ARCMigrate.cpp (+2-2) 
- (modified) clang/tools/libclang/CLog.h (+1-1) 
- (modified) llvm/lib/Support/Unix/Path.inc (+1-1) 
- (modified) llvm/lib/Support/Unix/Process.inc (+1-1) 
- (modified) llvm/unittests/Support/Path.cpp (+2-2) 


``````````diff
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.

``````````

</details>


https://github.com/llvm/llvm-project/pull/108529


More information about the cfe-commits mailing list