[clang] [clang][driver] Prevent clang picking the system headers when started via a symlink (PR #68091)

Liviu Ionescu via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 5 02:27:12 PDT 2023


https://github.com/ilg-ul updated https://github.com/llvm/llvm-project/pull/68091

>From f3812174546270051c4a2903b9a99408bf5b7ba0 Mon Sep 17 00:00:00 2001
From: Liviu Ionescu <ilg at livius.net>
Date: Tue, 3 Oct 2023 14:07:48 +0300
Subject: [PATCH 1/4] [clang][driver] Use platform specific calls to get the
 executable absolute path (#66704)

In clang/tools/driver/driver.cpp, the function SetInstallDir() tries to determine the
location where the toolchain is installed, basically by taking the parent folder of
the executable absolute path. This is not correct when the compiler is invoked
via a link, since it returns the parent of the link.

This leads to subtle errors, for example on macOS it silently picks the wrong
system headers.
---
 clang/tools/driver/driver.cpp | 61 +++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 531b5b4a61c1804..c8ad167cba0a423 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -57,6 +57,15 @@ using namespace clang;
 using namespace clang::driver;
 using namespace llvm::opt;
 
+#if defined(__linux__)
+#include <unistd.h>
+#elif defined(__APPLE__)
+#include <libproc.h>
+#include <unistd.h>
+#elif defined(__MINGW32__)
+#include <windows.h>
+#endif
+
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
   if (!CanonicalPrefixes) {
     SmallString<128> ExecutablePath(Argv0);
@@ -331,6 +340,56 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   // path being a symlink.
   SmallString<128> InstalledPath(argv[0]);
 
+#if defined(__linux__)
+
+  char ProcessAbsolutePath[PATH_MAX];
+
+  int len = readlink("/proc/self/exe", ProcessAbsolutePath,
+                     sizeof(ProcessAbsolutePath) - 1);
+  if ( len <= 0 ) {
+    llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with "
+                 << strerror(errno) << "\n";
+    exit(1);
+  }
+
+  ProcessAbsolutePath[len] = 0;
+  InstalledPath = ProcessAbsolutePath;
+
+#elif defined(__APPLE__)
+
+  // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call
+  // fails with ENOMEM (12) - Cannot allocate memory.
+  // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c
+  char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1];
+
+  int len = proc_pidpath(getpid(), ProcessAbsolutePath,
+                         sizeof(ProcessAbsolutePath) - 1);
+  if ( len <= 0 ) {
+    llvm::errs() << "Internal error: proc_pidpath() failed with "
+                 << strerror(errno) << "\n";
+    exit(1);
+  }
+
+  ProcessAbsolutePath[len] = 0;
+  InstalledPath = ProcessAbsolutePath;
+
+#elif defined(__MINGW32__)
+
+  char ProcessAbsolutePath[PATH_MAX];
+
+  len = GetModuleFileName(NULL, ProcessAbsolutePath,
+                          sizeof(ProcessAbsolutePath) - 1);
+  if ( len <= 0 ) {
+    llvm::errs() << "Internal error: GetModuleFileName() failed with "
+                 << strerror(errno) << "\n";
+    exit(1);
+  }
+
+  ProcessAbsolutePath[len] = 0;
+  InstalledPath = ProcessAbsolutePath;
+
+#else
+
   // Do a PATH lookup, if there are no directory components.
   if (llvm::sys::path::filename(InstalledPath) == InstalledPath)
     if (llvm::ErrorOr<std::string> Tmp = llvm::sys::findProgramByName(
@@ -341,6 +400,8 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   if (CanonicalPrefixes)
     llvm::sys::fs::make_absolute(InstalledPath);
 
+#endif
+
   StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath));
   if (llvm::sys::fs::exists(InstalledPathParent))
     TheDriver.setInstalledDir(InstalledPathParent);

>From eb7b122d5e6ce160d3f0880aac44a2df720d182f Mon Sep 17 00:00:00 2001
From: Liviu Ionescu <ilg at livius.net>
Date: Tue, 3 Oct 2023 14:16:30 +0300
Subject: [PATCH 2/4] clang-format driver.cpp patch

---
 clang/tools/driver/driver.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index c8ad167cba0a423..45e1469aafca95e 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -346,7 +346,7 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
 
   int len = readlink("/proc/self/exe", ProcessAbsolutePath,
                      sizeof(ProcessAbsolutePath) - 1);
-  if ( len <= 0 ) {
+  if (len <= 0) {
     llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with "
                  << strerror(errno) << "\n";
     exit(1);
@@ -360,11 +360,11 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call
   // fails with ENOMEM (12) - Cannot allocate memory.
   // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c
-  char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE+1];
+  char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1];
 
   int len = proc_pidpath(getpid(), ProcessAbsolutePath,
                          sizeof(ProcessAbsolutePath) - 1);
-  if ( len <= 0 ) {
+  if (len <= 0) {
     llvm::errs() << "Internal error: proc_pidpath() failed with "
                  << strerror(errno) << "\n";
     exit(1);
@@ -379,7 +379,7 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
 
   len = GetModuleFileName(NULL, ProcessAbsolutePath,
                           sizeof(ProcessAbsolutePath) - 1);
-  if ( len <= 0 ) {
+  if (len <= 0) {
     llvm::errs() << "Internal error: GetModuleFileName() failed with "
                  << strerror(errno) << "\n";
     exit(1);

>From f461d6bc9ea64fc4685b2020a958fb7730af3657 Mon Sep 17 00:00:00 2001
From: Liviu Ionescu <ilg at livius.net>
Date: Tue, 3 Oct 2023 22:54:39 +0300
Subject: [PATCH 3/4] driver.cpp: use ClangExecutable if canonical

---
 clang/tools/driver/driver.cpp | 64 ++---------------------------------
 1 file changed, 2 insertions(+), 62 deletions(-)

diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index 45e1469aafca95e..e170cc24fe046e2 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -57,15 +57,6 @@ using namespace clang;
 using namespace clang::driver;
 using namespace llvm::opt;
 
-#if defined(__linux__)
-#include <unistd.h>
-#elif defined(__APPLE__)
-#include <libproc.h>
-#include <unistd.h>
-#elif defined(__MINGW32__)
-#include <windows.h>
-#endif
-
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
   if (!CanonicalPrefixes) {
     SmallString<128> ExecutablePath(Argv0);
@@ -338,57 +329,8 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   // Attempt to find the original path used to invoke the driver, to determine
   // the installed path. We do this manually, because we want to support that
   // path being a symlink.
-  SmallString<128> InstalledPath(argv[0]);
-
-#if defined(__linux__)
-
-  char ProcessAbsolutePath[PATH_MAX];
-
-  int len = readlink("/proc/self/exe", ProcessAbsolutePath,
-                     sizeof(ProcessAbsolutePath) - 1);
-  if (len <= 0) {
-    llvm::errs() << "Internal error: readlink(\"/proc/self/exe\") failed with "
-                 << strerror(errno) << "\n";
-    exit(1);
-  }
-
-  ProcessAbsolutePath[len] = 0;
-  InstalledPath = ProcessAbsolutePath;
-
-#elif defined(__APPLE__)
-
-  // The size must be higher than PROC_PIDPATHINFO_SIZE, otherwise the call
-  // fails with ENOMEM (12) - Cannot allocate memory.
-  // https://opensource.apple.com/source/Libc/Libc-498/darwin/libproc.c
-  char ProcessAbsolutePath[PROC_PIDPATHINFO_SIZE + 1];
-
-  int len = proc_pidpath(getpid(), ProcessAbsolutePath,
-                         sizeof(ProcessAbsolutePath) - 1);
-  if (len <= 0) {
-    llvm::errs() << "Internal error: proc_pidpath() failed with "
-                 << strerror(errno) << "\n";
-    exit(1);
-  }
-
-  ProcessAbsolutePath[len] = 0;
-  InstalledPath = ProcessAbsolutePath;
-
-#elif defined(__MINGW32__)
-
-  char ProcessAbsolutePath[PATH_MAX];
-
-  len = GetModuleFileName(NULL, ProcessAbsolutePath,
-                          sizeof(ProcessAbsolutePath) - 1);
-  if (len <= 0) {
-    llvm::errs() << "Internal error: GetModuleFileName() failed with "
-                 << strerror(errno) << "\n";
-    exit(1);
-  }
-
-  ProcessAbsolutePath[len] = 0;
-  InstalledPath = ProcessAbsolutePath;
-
-#else
+  SmallString<128> InstalledPath(CanonicalPrefixes ? TheDriver.ClangExecutable
+                                                   : argv[0]);
 
   // Do a PATH lookup, if there are no directory components.
   if (llvm::sys::path::filename(InstalledPath) == InstalledPath)
@@ -400,8 +342,6 @@ static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   if (CanonicalPrefixes)
     llvm::sys::fs::make_absolute(InstalledPath);
 
-#endif
-
   StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath));
   if (llvm::sys::fs::exists(InstalledPathParent))
     TheDriver.setInstalledDir(InstalledPathParent);

>From ad49d7f3d43b38c838101b54e0d4458e2eaa22d5 Mon Sep 17 00:00:00 2001
From: Liviu Ionescu <ilg at livius.net>
Date: Thu, 5 Oct 2023 12:26:56 +0300
Subject: [PATCH 4/4] [clang][driver] add <executable>/../include/c++/v1 to
 include path

On macOS, when clang is invoked via a symlink, since the InstalledDir is
where the link is located, the C++ headers were not identified and the
default system headers were used.

The fix add a second check using the folder where the executable is
located.
---
 clang/lib/Driver/ToolChains/Darwin.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index c8700a7d159aa47..1b4e41939c48868 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2494,6 +2494,19 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
                    << "\"\n";
     }
 
+    // Check for the location where the executable is located, if different.
+    if (getDriver().getInstalledDir() != getDriver().Dir) {
+      InstallBin = llvm::StringRef(getDriver().Dir.c_str());
+      llvm::sys::path::append(InstallBin, "..", "include", "c++", "v1");
+      if (getVFS().exists(InstallBin)) {
+        addSystemInclude(DriverArgs, CC1Args, InstallBin);
+        return;
+      } else if (DriverArgs.hasArg(options::OPT_v)) {
+        llvm::errs() << "ignoring nonexistent directory \"" << InstallBin
+                     << "\"\n";
+      }
+    }
+
     // Otherwise, check for (2)
     llvm::SmallString<128> SysrootUsr = Sysroot;
     llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1");



More information about the cfe-commits mailing list