Skip to content

Commit

Permalink
Revert r361885 "[Driver] Fix -working-directory issues"
Browse files Browse the repository at this point in the history
This made clang unable to open files using relative paths on network shares on
Windows (PR43204). On the bug it was pointed out that createPhysicalFileSystem()
is not terribly mature, and using it is risky. Reverting for now until there's
a clear way forward.

> Currently the `-working-directory` option does not actually impact the working
> directory for all of the clang driver, it only impacts how files are looked up
> to make sure they exist.  This means that that clang passes the wrong paths
> to -fdebug-compilation-dir and -coverage-notes-file.
>
> This patch fixes that by changing all the places in the driver where we convert
> to absolute paths to use the VFS, and then calling setCurrentWorkingDirectory on
> the VFS.  This also changes the default VFS for `Driver` to use a virtualized
> working directory, instead of changing the process's working directory.
>
> Differential Revision: https://reviews.llvm.org/D62271

This also revertes the part of r369938 which checked that -working-directory works.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371027 91177308-0d34-0410-b5e6-96231b3b80d8
(cherry picked from commit 7912f72)
  • Loading branch information
zmodem authored and JDevlieghere committed Sep 27, 2019
1 parent 2597f47 commit 59d80e7
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 44 deletions.
2 changes: 0 additions & 2 deletions include/clang/Basic/DiagnosticDriverKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ def err_no_external_assembler : Error<
"there is no external assembler that can be used on this platform">;
def err_drv_unable_to_remove_file : Error<
"unable to remove file: %0">;
def err_drv_unable_to_set_working_directory : Error <
"unable to set working directory: %0">;
def err_drv_command_failure : Error<
"unable to execute command: %0">;
def err_drv_invalid_darwin_version : Error<
Expand Down
24 changes: 14 additions & 10 deletions lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,

// Provide a sane fallback if no VFS is specified.
if (!this->VFS)
this->VFS = llvm::vfs::createPhysicalFileSystem().release();
this->VFS = llvm::vfs::getRealFileSystem();

Name = llvm::sys::path::filename(ClangExecutable);
Dir = llvm::sys::path::parent_path(ClangExecutable);
Expand Down Expand Up @@ -1005,11 +1005,6 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
}
}

// Check for working directory option before accessing any files
if (Arg *WD = Args.getLastArg(options::OPT_working_directory))
if (VFS->setCurrentWorkingDirectory(WD->getValue()))
Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue();

// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;

Expand Down Expand Up @@ -1992,11 +1987,20 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
if (Value == "-")
return true;

if (getVFS().exists(Value))
SmallString<64> Path(Value);
if (Arg *WorkDir = Args.getLastArg(options::OPT_working_directory)) {
if (!llvm::sys::path::is_absolute(Path)) {
SmallString<64> Directory(WorkDir->getValue());
llvm::sys::path::append(Directory, Value);
Path.assign(Directory);
}
}

if (getVFS().exists(Path))
return true;

if (IsCLMode()) {
if (!llvm::sys::path::is_absolute(Twine(Value)) &&
if (!llvm::sys::path::is_absolute(Twine(Path)) &&
llvm::sys::Process::FindInEnvPath("LIB", Value))
return true;

Expand All @@ -2022,12 +2026,12 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask,
ExcludedFlagsBitmask) <= 1) {
Diag(clang::diag::err_drv_no_such_file_with_suggestion)
<< Value << Nearest;
<< Path << Nearest;
return false;
}
}

Diag(clang::diag::err_drv_no_such_file) << Value;
Diag(clang::diag::err_drv_no_such_file) << Path;
return false;
}

Expand Down
35 changes: 20 additions & 15 deletions lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,15 +610,16 @@ getFramePointerKind(const ArgList &Args, const llvm::Triple &Triple) {
}

/// Add a CC1 option to specify the debug compilation directory.
static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs,
const llvm::vfs::FileSystem &VFS) {
static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs) {
if (Arg *A = Args.getLastArg(options::OPT_fdebug_compilation_dir)) {
CmdArgs.push_back("-fdebug-compilation-dir");
CmdArgs.push_back(A->getValue());
} else if (llvm::ErrorOr<std::string> CWD =
VFS.getCurrentWorkingDirectory()) {
CmdArgs.push_back("-fdebug-compilation-dir");
CmdArgs.push_back(Args.MakeArgString(*CWD));
} else {
SmallString<128> cwd;
if (!llvm::sys::fs::current_path(cwd)) {
CmdArgs.push_back("-fdebug-compilation-dir");
CmdArgs.push_back(Args.MakeArgString(cwd));
}
}
}

Expand Down Expand Up @@ -884,8 +885,13 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
else
OutputFilename = llvm::sys::path::filename(Output.getBaseInput());
SmallString<128> CoverageFilename = OutputFilename;
if (llvm::sys::path::is_relative(CoverageFilename))
(void)D.getVFS().makeAbsolute(CoverageFilename);
if (llvm::sys::path::is_relative(CoverageFilename)) {
SmallString<128> Pwd;
if (!llvm::sys::fs::current_path(Pwd)) {
llvm::sys::path::append(Pwd, CoverageFilename);
CoverageFilename.swap(Pwd);
}
}
llvm::sys::path::replace_extension(CoverageFilename, "gcno");
CmdArgs.push_back(Args.MakeArgString(CoverageFilename));

Expand Down Expand Up @@ -2009,14 +2015,13 @@ void Clang::DumpCompilationDatabase(Compilation &C, StringRef Filename,
CompilationDatabase = std::move(File);
}
auto &CDB = *CompilationDatabase;
auto CWD = D.getVFS().getCurrentWorkingDirectory();
if (!CWD)
CWD = ".";
CDB << "{ \"directory\": \"" << escape(*CWD) << "\"";
SmallString<128> Buf;
if (!llvm::sys::fs::current_path(Buf))
Buf = ".";
CDB << "{ \"directory\": \"" << escape(Buf) << "\"";
CDB << ", \"file\": \"" << escape(Input.getFilename()) << "\"";
CDB << ", \"output\": \"" << escape(Output.getFilename()) << "\"";
CDB << ", \"arguments\": [\"" << escape(D.ClangExecutable) << "\"";
SmallString<128> Buf;
Buf = "-x";
Buf += types::getTypeName(Input.getType());
CDB << ", \"" << escape(Buf) << "\"";
Expand Down Expand Up @@ -4429,7 +4434,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fno-autolink");

// Add in -fdebug-compilation-dir if necessary.
addDebugCompDirArg(Args, CmdArgs, D.getVFS());
addDebugCompDirArg(Args, CmdArgs);

addDebugPrefixMapArg(D, Args, CmdArgs);

Expand Down Expand Up @@ -6177,7 +6182,7 @@ void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
DebugInfoKind = (WantDebug ? codegenoptions::LimitedDebugInfo
: codegenoptions::NoDebugInfo);
// Add the -fdebug-compilation-dir flag if needed.
addDebugCompDirArg(Args, CmdArgs, C.getDriver().getVFS());
addDebugCompDirArg(Args, CmdArgs);

addDebugPrefixMapArg(getToolChain().getDriver(), Args, CmdArgs);

Expand Down
8 changes: 0 additions & 8 deletions test/Driver/gen-cdb-fragment.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@
// RUN: %clang -target x86_64-apple-macos10.15 -S %s -o - -gen-cdb-fragment-path %t.cdb
// RUN: ls %t.cdb | FileCheck --check-prefix=CHECK-LS %s

// Working directory arg is respected.
// RUN: rm -rf %t.cdb
// RUN: mkdir %t.cdb
// RUN: %clang -target x86_64-apple-macos10.15 -working-directory %t.cdb -c %s -o - -gen-cdb-fragment-path "."
// RUN: ls %t.cdb | FileCheck --check-prefix=CHECK-LS %s
// RUN: cat %t.cdb/*.json | FileCheck --check-prefix=CHECK-CWD %s
// CHECK-CWD: "directory": "{{.*}}.cdb"

// -### does not emit the CDB fragment
// RUN: rm -rf %t.cdb
// RUN: mkdir %t.cdb
Expand Down
10 changes: 1 addition & 9 deletions test/Driver/working-directory.c
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
// RUN: %clang -### -working-directory /no/such/dir/ input 2>&1 | FileCheck %s
// RUN: %clang -### -working-directory %p/Inputs no_such_file.cpp -c 2>&1 | FileCheck %s --check-prefix=CHECK_NO_FILE
// RUN: %clang -### -working-directory %p/Inputs pchfile.cpp -c 2>&1 | FileCheck %s --check-prefix=CHECK_WORKS

// CHECK: unable to set working directory: /no/such/dir/

// CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp'

// CHECK_WORKS: "-coverage-notes-file" "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs{{/|\\\\}}pchfile.gcno"
// CHECK_WORKS: "-working-directory" "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs"
// CHECK_WORKS: "-fdebug-compilation-dir" "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs"
//CHECK: no such file or directory: '/no/such/dir/input'

0 comments on commit 59d80e7

Please sign in to comment.