llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Bill Wendling (isanbard) <details> <summary>Changes</summary> With -save-temps, an intermediate file named after the input can be the input itself, e.g. the preprocessed output of a .s file on Darwin. The driver catches this, but only checked for it when the output's name was the input's file name. It missed two cases, and overwrote the input with the preprocessed output: - With -save-temps=obj, the output is in the output's directory, so "clang -c -save-temps=obj sub/t.s -o sub/t.o" wrote "sub/t.s". - On a case-insensitive file system, such as macOS's default, the preprocessed output "t.s" of an input "t.S" is the same file. Check whether the output is the same file as the input whenever temps are saved. Fixes #<!-- -->7002. Tested with the new tests (the case-insensitive one only runs where lit sets `case-insensitive-filesystem`), `clang/test/Driver`, and the other Clang tests that use `-save-temps`. I also checked both cases end to end, the `.S` one in a case-insensitive directory on Windows (WSL `/mnt/c`): before the fix the input was replaced by the preprocessed output, and after it's unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- Full diff: https://github.com/llvm/llvm-project/pull/235413.diff 3 Files Affected: - (modified) clang/lib/Driver/Driver.cpp (+7-8) - (added) clang/test/Driver/save-temps-input-overwrite-case-insensitive.c (+11) - (added) clang/test/Driver/save-temps-input-overwrite.c (+26) ``````````diff diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 707d6847caf446..1c693e792b9da5 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -5857,15 +5857,14 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA, NamedOutput = C.getArgs().MakeArgString(TempPath.c_str()); } - // If we're saving temps and the temp file conflicts with the input file, - // then avoid overwriting input file. - if (!AtTopLevel && isSaveTempsEnabled() && NamedOutput == BaseName) { + // If we're saving temps and the temp file is the input file, then avoid + // overwriting the input file. Compare the files, not their names: the output + // can name the input by another path (e.g. "sub/t.s" from -save-temps=obj), + // or with different case on a case-insensitive file system ("t.s" for an + // input "t.S"). + if (!AtTopLevel && isSaveTempsEnabled()) { bool SameFile = false; - SmallString<256> Result; - llvm::sys::fs::current_path(Result); - llvm::sys::path::append(Result, BaseName); - llvm::sys::fs::equivalent(BaseInput, Result.c_str(), SameFile); - // Must share the same path to conflict. + llvm::sys::fs::equivalent(BaseInput, NamedOutput, SameFile); if (SameFile) { StringRef Name = llvm::sys::path::filename(BaseInput); std::pair<StringRef, StringRef> Split = Name.split('.'); diff --git a/clang/test/Driver/save-temps-input-overwrite-case-insensitive.c b/clang/test/Driver/save-temps-input-overwrite-case-insensitive.c new file mode 100644 index 00000000000000..751ed2cd81c58f --- /dev/null +++ b/clang/test/Driver/save-temps-input-overwrite-case-insensitive.c @@ -0,0 +1,11 @@ +// Check that -save-temps doesn't overwrite a .S input with its preprocessed +// output, whose name ends in .s, on a case-insensitive file system. + +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && mkdir -p %t && touch %t/t.S +// RUN: cd %t && %clang --target=x86_64-linux-gnu -c -save-temps t.S -### 2>&1 \ +// RUN: | FileCheck %s +// CHECK: "-E" +// CHECK-NOT: "-o" "t.s" +// CHECK-SAME: "-o" "{{[^"]*}}t-{{[^"]*}}.s" diff --git a/clang/test/Driver/save-temps-input-overwrite.c b/clang/test/Driver/save-temps-input-overwrite.c new file mode 100644 index 00000000000000..3d0739265ad47e --- /dev/null +++ b/clang/test/Driver/save-temps-input-overwrite.c @@ -0,0 +1,26 @@ +// Check that -save-temps doesn't name an intermediate file after the input +// file, which would overwrite it. On Darwin, a .s file is preprocessed, and +// the preprocessed output's name ends in .s. + +// RUN: rm -rf %t && mkdir -p %t/sub %t/obj +// RUN: touch %t/t.s %t/sub/t.s + +// The output would be the input in the current directory. +// RUN: cd %t && %clang --target=x86_64-apple-darwin -c -save-temps t.s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=CWD +// CWD: "-E" +// CWD-NOT: "-o" "t.s" +// CWD-SAME: "-o" "{{[^"]*}}t-{{[^"]*}}.s" + +// With -save-temps=obj, the output would be the input in the output directory. +// RUN: cd %t && %clang --target=x86_64-apple-darwin -c -save-temps=obj sub/t.s -o sub/t.o -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=OBJ +// OBJ: "-E" +// OBJ-NOT: "-o" "sub{{/|\\\\}}t.s" +// OBJ-SAME: "-o" "{{[^"]*}}t-{{[^"]*}}.s" + +// An output with the input's name in another directory is a different file. +// RUN: cd %t/obj && %clang --target=x86_64-apple-darwin -c -save-temps ../t.s -o t.o -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=OTHER-DIR +// OTHER-DIR: "-E" +// OTHER-DIR-SAME: "-o" "t.s" `````````` </details> https://github.com/llvm/llvm-project/pull/235413 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
