[llvm-driver] Fix clang -fno-integrated-cc1 when invoked from the llvm driver

[abrachet]

Summary

Clang discovers the path to it's own executable and uses this
to re-invoke itself when -fno-integrated-cc1 is set, or if there are
multiple jobs, i.e. multiple source files specified. When clang is
invoked as part of the llvm driver, it's executable is going to be the
driver executable 'llvm', not the symlink 'clang', so the invocation
will look like 'llvm -cc1 ...'. The llvm driver will try to find an tool
called '-cc1' and will fail.

This patch changes clang to realize if the executable it found was
the llvm-driver, in which case it will prepend "clang" to the args.
Unconditionally doin'g "clang" is safe because it will only ever
re-invoke itself in cc1 mode, and never need to specify, clang++,
etc.

[phosek]

llvm/lib/Support/LLVMDriver.cpp

11

I'd prefer if this variable was a constant because there's no reason why should ever be changed at runtime.

Rather than defining it here, could we instead define it in llvm/tools/llvm-driver/llvm-driver.cpp and llvm/cmake/driver-template.cpp.in as constant that's initialized to true and false respectively.

[abrachet]

Make llvm::IsLLVMDriver const

[abrachet]

llvm/lib/Support/LLVMDriver.cpp

11

Nice, I like that, thanks

[phosek]

clang/lib/Driver/ToolChains/Clang.cpp

5005 ↗

(On Diff #467989)

Instead of storing a boolean and then duplicating this check and hardcoding the name "clang" in multiple places, we could store the name in the Driver (for example as DriverName) and default it to null. It'd be a responsibility of cc1gen_reproducer_main.cpp and driver.cpp to set it.

[abrachet]

clang/lib/Driver/ToolChains/Clang.cpp

5005 ↗

(On Diff #467989)

Sure

[dblaikie]

naive question, but... what if the llvm-driver special cased -cc1 as a tool? & then clang wouldn't have to do anything special?

[abrachet]

In D135108#3880245, @dblaikie wrote:

naive question, but... what if the llvm-driver special cased -cc1 as a tool? & then clang wouldn't have to do anything special?

Not a naive question at all. I had actually thought about this option too and listed it as one in this patch when it was a WIP, ultimately we thought that maybe it was too hacky, though so too is this patch. I've created D136895 so that we can compare and contrast.

It's certainly a lot cleaner to just touch the llvm-driver binary and it's nice to not have to touch clang's Job handling. What do folks think?

[abrachet]

I've also added D137800 which is very similar to this patch except it doesn't hardcode "clang" and relies on getting that from the driver

[abrachet]

Went a different direction

clang/test/Analysis/scan-build/lit.local.cfg

 # -*- Python -*-
 
 import lit.util
 import lit.formats
 import os
 
+# The os.path.realpath(config.clang) further down finds the
+# llvm executable and not clang.
+if config.is_llvm_driver:
+    config.unsupported = True
+
 use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")
 
 config.substitutions.append(('%scan-build',
                              '\'%s\' --use-analyzer=%s ' % (
                                  lit.util.which('scan-build',
                                                 os.path.join(
                                                     config.clang_src_dir,
                                                     'tools',
                                                     'scan-build',
                                                     'bin')),
                                  os.path.realpath(config.clang))))

clang/test/CMakeLists.txt

llvm_canonicalize_cmake_booleans(
   CLANG_PLUGIN_SUPPORT
   CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_ZSTD
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_THREADS
   LLVM_WITH_Z3
+  LLVM_TOOL_LLVM_DRIVER_BUILD
   )
 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
   MAIN_CONFIG
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   PATHS
list(APPEND CLANG_TEST_DEPS
   clang-import-test
   clang-rename
   clang-refactor
   clang-diff
   clang-scan-deps
   diagtool
   hmaptool
   )
-  
 if(CLANG_ENABLE_STATIC_ANALYZER)
   list(APPEND CLANG_TEST_DEPS
     clang-check
     clang-extdef-mapping
     )
 endif()
 
 if (CLANG_ENABLE_ARCMT)

clang/test/lit.site.cfg.py.in

 config.enable_threads = @LLVM_ENABLE_THREADS@
 config.host_arch = "@HOST_ARCH@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.use_z3_solver = lit_config.params.get('USE_Z3_SOLVER', "@USE_Z3_SOLVER@")
 config.has_plugins = @CLANG_PLUGIN_SUPPORT@
 config.clang_vendor_uti = "@CLANG_VENDOR_UTI@"
 config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
 config.standalone_build = @CLANG_BUILT_STANDALONE@
+config.is_llvm_driver = @LLVM_TOOL_LLVM_DRIVER_BUILD@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
 
 # Let the main config do the real work.
 lit_config.load_config(
     config, os.path.join(config.clang_src_dir, "test/lit.cfg.py"))

clang/tools/driver/driver.cpp

 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/LLVMDriver.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 #include <set>
 #include <system_error>
 using namespace clang;
 using namespace clang::driver;
 using namespace llvm::opt;
 
 std::string GetExecutablePath(const char *Argv0, bool CanonicalPrefixes) {
-  if (!CanonicalPrefixes) {
+  if (!CanonicalPrefixes || llvm::IsLLVMDriver()) {
     SmallString<128> ExecutablePath(Argv0);
     // Do a PATH lookup if Argv0 isn't a valid path.
     if (!llvm::sys::fs::exists(ExecutablePath))
       if (llvm::ErrorOr<std::string> P =
               llvm::sys::findProgramByName(ExecutablePath))
         ExecutablePath = *P;
     return std::string(ExecutablePath.str());
   }

llvm/include/llvm/Support/LLVMDriver.h

+//===- LLVMDriver.h ---------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// The llvm-driver is an executable which statically links in many llvm tools
+// into one. This file defines just a single function, llvm::IsLLVMDriver(),
+// This can be useful for tools which need to behave slightly differently under
+// that environment.
+//
+//===----------------------------------------------------------------------===//
+
+namespace llvm {
+
+namespace impl {
+
+extern bool IsLLVMDriver;
+
+}
+
+static inline bool IsLLVMDriver() {
+  return impl::IsLLVMDriver;
+}
+
+}

llvm/lib/Support/CMakeLists.txt

add_llvm_component_library(LLVMSupport
   InstructionCost.cpp
   IntEqClasses.cpp
   IntervalMap.cpp
   ItaniumManglingCanonicalizer.cpp
   JSON.cpp
   KnownBits.cpp
   LEB128.cpp
   LineIterator.cpp
+  LLVMDriver.cpp
   Locale.cpp
   LockFileManager.cpp
   LowLevelType.cpp
   ManagedStatic.cpp
   MathExtras.cpp
   MemAlloc.cpp
   MemoryBuffer.cpp
   MemoryBufferRef.cpp

llvm/lib/Support/LLVMDriver.cpp

+//===- LLVMDriver.cpp ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+
+namespace llvm::impl {
+
+bool IsLLVMDriver = false;
+
+}

llvm/lib/Support/Path.cpp

 // Include the truly platform-specific parts.
 #if defined(LLVM_ON_UNIX)
 #include "Unix/Path.inc"
 #endif
 #if defined(_WIN32)
 #include "Windows/Path.inc"
 #endif
 
-bool IsLLVMDriver = false;
-
 namespace llvm {
 namespace sys {
 namespace fs {
 
-std::string getMainExecutable(const char *Argv0, void *MainAddr) {
-  if (IsLLVMDriver)
-    return sys::path::stem(Argv0).str();
-  return getMainExecutableImpl(Argv0, MainAddr);
-}
-
 TempFile::TempFile(StringRef Name, int FD)
     : TmpName(std::string(Name)), FD(FD) {}
 TempFile::TempFile(TempFile &&Other) { *this = std::move(Other); }
 TempFile &TempFile::operator=(TempFile &&Other) {
   TmpName = std::move(Other.TmpName);
   FD = Other.FD;
   Other.Done = true;
   Other.FD = -1;

llvm/lib/Support/Unix/Path.inc

getprogpath(char ret[PATH_MAX], const char *bin)
   }
   free(s);
   return nullptr;
 }
 #endif // __FreeBSD__ || __NetBSD__ || __FreeBSD_kernel__
 
 /// GetMainExecutable - Return the path to the main executable, given the
 /// value of argv[0] from program startup.
-std::string getMainExecutableImpl(const char *argv0, void *MainAddr) {
+std::string getMainExecutable(const char *argv0, void *MainAddr) {
 #if defined(__APPLE__)
   // On OS X the executable path is saved to the stack by dyld. Reading it
   // from there is much faster than calling dladdr, especially for large
   // binaries with symbols.
   char exe_path[PATH_MAX];
   uint32_t size = sizeof(exe_path);
   if (_NSGetExecutablePath(exe_path, &size) == 0) {
     char link_path[PATH_MAX];

llvm/lib/Support/Windows/Path.inc

 }
 
 } // end namespace windows
 
 namespace fs {
 
 const file_t kInvalidFile = INVALID_HANDLE_VALUE;
 
-std::string getMainExecutableImpl(const char *argv0, void *MainExecAddr) {
+std::string getMainExecutable(const char *argv0, void *MainExecAddr) {
   SmallVector<wchar_t, MAX_PATH> PathName;
   PathName.resize_for_overwrite(PathName.capacity());
   DWORD Size = ::GetModuleFileNameW(NULL, PathName.data(), PathName.size());
 
   // A zero return value indicates a failure other than insufficient space.
   if (Size == 0)
     return "";
 

llvm/tools/llvm-driver/llvm-driver.cpp

 //===-- llvm-driver.cpp ---------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/LLVMDriver.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/WithColor.h"
 
 using namespace llvm;
 
 #define LLVM_DRIVER_TOOL(tool, entry) int entry##_main(int argc, char **argv);
 #include "LLVMDriverTools.def"
 
#include "LLVMDriverTools.def"
 
   if (Is("llvm"))
     return findTool(Argc - 1, Argv + 1);
 
   printHelpMessage();
   return 1;
 }
 
-extern bool IsLLVMDriver;
-
 int main(int Argc, char **Argv) {
-  IsLLVMDriver = true;
+  llvm::impl::IsLLVMDriver = true;
   return findTool(Argc, Argv);
 }

llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn

sources = [
     "InstructionCost.cpp",
     "IntEqClasses.cpp",
     "IntervalMap.cpp",
     "ItaniumManglingCanonicalizer.cpp",
     "JSON.cpp",
     "KnownBits.cpp",
     "LEB128.cpp",
     "LineIterator.cpp",
+    "LLVMDriver.cpp",
     "Locale.cpp",
     "LockFileManager.cpp",
     "LowLevelType.cpp",
     "MD5.cpp",
     "MSP430AttributeParser.cpp",
     "MSP430Attributes.cpp",
     "ManagedStatic.cpp",
     "MathExtras.cpp",