Skip to content

Commit

Permalink
Windows: Avoid accidentally using GNU link.exe for linking (#4718)
Browse files Browse the repository at this point in the history
This happens in an environment where MSVC was set up first (so env
var VSINSTALLDIR defined), and later prepending a dir such as
`C:\Program Files\Git\usr\bin` to PATH, so that the first link.exe
on PATH isn't the Microsoft one.
  • Loading branch information
kinke authored Aug 7, 2024
1 parent 7486117 commit 6aabae4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 8 deletions.
7 changes: 5 additions & 2 deletions driver/linker-msvc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,11 @@ int linkObjToBinaryMSVC(llvm::StringRef outputPath,
std::string linker = opts::linker;
if (linker.empty()) {
#ifdef _WIN32
// default to lld-link.exe for LTO
linker = opts::isUsingLTO() ? "lld-link.exe" : "link.exe";
// Default to lld-link.exe for LTO, otherwise Microsoft's link.exe.
// Try not to accidentally use a GNU link.exe (if in PATH before the MSVC
// bin dir).
linker = opts::isUsingLTO() ? "lld-link.exe"
: msvcEnv.tryResolveToolPath("link.exe");
#else
linker = "lld-link";
#endif
Expand Down
27 changes: 21 additions & 6 deletions driver/tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,15 @@ int executeToolAndWait(const Loc &loc, const std::string &tool_,
namespace windows {

namespace {

// cached 'singleton', lazily initialized (as that can be very expensive)
const VSOptions &getVSOptions() {
static VSOptions vsOptions;
if (!vsOptions.VSInstallDir)
vsOptions.initialize();
return vsOptions;
}

bool setupMsvcEnvironmentImpl(
bool forPreprocessingOnly,
std::vector<std::pair<std::wstring, wchar_t *>> *rollback) {
Expand Down Expand Up @@ -318,12 +327,9 @@ bool setupMsvcEnvironmentImpl(
const bool x64 = triple.isArch64Bit();
const auto begin = std::chrono::steady_clock::now();

static VSOptions vsOptions; // cache, as this can be expensive
if (!vsOptions.VSInstallDir) {
vsOptions.initialize();
if (!vsOptions.VSInstallDir)
return false;
}
auto &vsOptions = getVSOptions();
if (!vsOptions.VSInstallDir)
return false;

// cache the environment variable prefixes too
static llvm::SmallVector<const char *, 2> binPaths;
Expand Down Expand Up @@ -446,6 +452,15 @@ MsvcEnvironmentScope::~MsvcEnvironmentScope() {
}
}

std::string
MsvcEnvironmentScope::tryResolveToolPath(const char *fileName) const {
const bool x64 = global.params.targetTriple->isArch64Bit();
const char *secondaryBindir = nullptr;
if (auto bindir = getVSOptions().getVCBinDir(x64, secondaryBindir))
return (llvm::Twine(bindir) + "\\" + fileName).str();
return fileName;
}

} // namespace windows

#endif // _WIN32
4 changes: 4 additions & 0 deletions driver/tool.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ struct MsvcEnvironmentScope {

~MsvcEnvironmentScope();

// Tries to return the absolute path to a VC tool, falling back to the file
// name.
std::string tryResolveToolPath(const char *fileName) const;

private:
// for each changed env var: name & original value
std::vector<std::pair<std::wstring, wchar_t *>> rollback;
Expand Down

0 comments on commit 6aabae4

Please sign in to comment.