Skip to content

Commit

Permalink
[clang] Make crash reproducer work with clang-cl
Browse files Browse the repository at this point in the history
When clang crashes, it writes a standalone source file and shell script
to reproduce the crash.

The Driver used to set `Mode = CPPMode` in generateCompilationDiagnostics()
to force preprocessing mode. This has the side effect of making
IsCLMode() return false, which in turn meant Clang::AddClangCLArgs()
didn't get called when creating the standalone source file, which meant
the stand-alone file was preprocessed with the gcc driver's defaults
In particular, exceptions default to on with the gcc driver, but to
off with the cl driver. The .sh script did use the original command
line, so in the reproducer for a clang-cl crash, the standalone source
file could contain exception-using code after preprocessing that the
compiler invocation in the shell script would then complain about.

This patch removes the `Mode = CPPMode;` line and instead additionally
checks for `CCGenDiagnostics` in most places that check `CCCIsCPP().
This also matches the strategy Clang::ConstructJob() uses to add
-frewrite-includes for creating the standalone source file for a crash
report.

Fixes PR52007.

Differential Revision: https://reviews.llvm.org/D110783
  • Loading branch information
nico committed Sep 30, 2021
1 parent dbaa408 commit 8dfbe9b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 37 deletions.
11 changes: 6 additions & 5 deletions clang/lib/Driver/Driver.cpp
Expand Up @@ -277,7 +277,8 @@ phases::ID Driver::getFinalPhase(const DerivedArgList &DAL,
if (CCCIsCPP() || (PhaseArg = DAL.getLastArg(options::OPT_E)) ||
(PhaseArg = DAL.getLastArg(options::OPT__SLASH_EP)) ||
(PhaseArg = DAL.getLastArg(options::OPT_M, options::OPT_MM)) ||
(PhaseArg = DAL.getLastArg(options::OPT__SLASH_P))) {
(PhaseArg = DAL.getLastArg(options::OPT__SLASH_P)) ||
CCGenDiagnostics) {
FinalPhase = phases::Preprocess;

// --precompile only runs up to precompilation.
Expand Down Expand Up @@ -1343,7 +1344,6 @@ void Driver::generateCompilationDiagnostics(
PrintVersion(C, llvm::errs());

// Suppress driver output and emit preprocessor output to temp file.
Mode = CPPMode;
CCGenDiagnostics = true;

// Save the original job command(s).
Expand Down Expand Up @@ -2263,6 +2263,7 @@ void Driver::BuildInputs(const ToolChain &TC, DerivedArgList &Args,
//
// Otherwise emit an error but still use a valid type to avoid
// spurious errors (e.g., no inputs).
assert(!CCGenDiagnostics && "stdin produces no crash reproducer");
if (!Args.hasArgNoClaim(options::OPT_E) && !CCCIsCPP())
Diag(IsCLMode() ? clang::diag::err_drv_unknown_stdin_type_clang_cl
: clang::diag::err_drv_unknown_stdin_type);
Expand All @@ -2278,10 +2279,10 @@ void Driver::BuildInputs(const ToolChain &TC, DerivedArgList &Args,
Ty = TC.LookupTypeForExtension(Ext + 1);

if (Ty == types::TY_INVALID) {
if (CCCIsCPP())
Ty = types::TY_C;
else if (IsCLMode() && Args.hasArgNoClaim(options::OPT_E))
if (IsCLMode() && (Args.hasArgNoClaim(options::OPT_E) || CCGenDiagnostics))
Ty = types::TY_CXX;
else if (CCCIsCPP() || CCGenDiagnostics)
Ty = types::TY_C;
else
Ty = types::TY_Object;
}
Expand Down
24 changes: 0 additions & 24 deletions clang/test/Driver/crash-report-clang-cl.c

This file was deleted.

45 changes: 45 additions & 0 deletions clang/test/Driver/crash-report-clang-cl.cpp
@@ -0,0 +1,45 @@
// RUN: rm -rf %t
// RUN: mkdir %t

// RUN: not %clang_cl -fsyntax-only /Brepro /source-charset:utf-8 \
// RUN: -fcrash-diagnostics-dir=%t -- %s 2>&1 | FileCheck %s
// RUN: cat %t/crash-report-clang-cl-*.cpp | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-clang-cl-*.sh | FileCheck --check-prefix=CHECKSH %s

// REQUIRES: crash-recovery

#pragma clang __debug crash

// CHECK: Preprocessed source(s) and associated run script(s) are located at:

// __has_feature(cxx_exceptions) is default-off in the cl-compatible driver.
FOO
#if __has_feature(cxx_exceptions)
int a = 1;
#else
int a = 0;
#endif
// CHECKSRC: {{^}}FOO
// CHECKSRC-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#if __has_feature(cxx_exceptions)
// CHECKSRC-NEXT: {{^}}#endif
// CHECKSRC-NEXT: {{^}}#endif /* disabled by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#
// CHECKSRC-NEXT: {{^}}int a = 1;
// CHECKSRC-NEXT: {{^}}#else
// CHECKSRC-NEXT: {{^}}#
// CHECKSRC-NEXT: {{^}}int a = 0;
// CHECKSRC-NEXT: {{^}}#endif

// CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-clang-cl-{{.*}}.cpp
// CHECKSH: # Crash reproducer
// CHECKSH-NEXT: # Driver args: {{.*}}"-fsyntax-only"
// CHECKSH-SAME: /Brepro
// CHECKSH-SAME: /source-charset:utf-8
// CHECKSH-NOT: -mno-incremental-linker-compatible
// CHECKSH-NOT: -finput-charset=utf-8
// CHECKSH-NEXT: # Original command: {{.*$}}
// CHECKSH-NEXT: "-cc1"
// CHECKSH: "-main-file-name" "crash-report-clang-cl.cpp"
// CHECKSH: "crash-report-{{[^ ]*}}.cpp"
Expand Up @@ -6,25 +6,25 @@
// RUN: -iprefix /the/prefix -iwithprefix /tmp -iwithprefixbefore /tmp/ \
// RUN: -Xclang -internal-isystem -Xclang /tmp/ \
// RUN: -Xclang -internal-externc-isystem -Xclang /tmp/ \
// RUN: -Xclang -main-file-name -Xclang foo.c \
// RUN: -Xclang -main-file-name -Xclang foo.cpp \
// RUN: -DFOO=BAR -DBAR="BAZ QUX"' > %t.rsp

// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1 \
// RUN: CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
// RUN: not %clang %s @%t.rsp -DPARSER 2>&1 | FileCheck %s
// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.cpp | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s

// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1 \
// RUN: CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
// RUN: not %clang %s @%t.rsp -DCRASH 2>&1 | FileCheck %s
// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.cpp | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s

// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1 \
// RUN: CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
// RUN: not %clang %s @%t.rsp -DFATAL 2>&1 | FileCheck %s
// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.cpp | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s

// REQUIRES: crash-recovery
Expand All @@ -38,16 +38,35 @@
#endif

// CHECK: Preprocessed source(s) and associated run script(s) are located at:
// CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-{{.*}}.c
// CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-{{.*}}.cpp

// __has_feature(cxx_exceptions) is default-on in the gcc-compatible driver.
FOO
// CHECKSRC: FOO
#if __has_feature(cxx_exceptions)
int a = 1;
#else
int a = 0;
#endif
// CHECKSRC: {{^}}FOO
// CHECKSRC-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#if __has_feature(cxx_exceptions)
// CHECKSRC-NEXT: {{^}}#endif
// CHECKSRC-NEXT: {{^}}#endif /* disabled by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */
// CHECKSRC-NEXT: {{^}}#
// CHECKSRC-NEXT: {{^}}int a = 1;
// CHECKSRC-NEXT: {{^}}#else
// CHECKSRC-NEXT: {{^}}#
// CHECKSRC-NEXT: {{^}}int a = 0;
// CHECKSRC-NEXT: {{^}}#endif

// CHECKSH: # Crash reproducer
// CHECKSH-NEXT: # Driver args: {{.*}}"-fsyntax-only"
// CHECKSH-SAME: "-D" "FOO=BAR"
// CHECKSH-SAME: "-D" "BAR=BAZ QUX"
// CHECKSH-NEXT: # Original command: {{.*$}}
// CHECKSH-NEXT: "-cc1"
// CHECKSH: "-main-file-name" "crash-report.c"
// CHECKSH: "-main-file-name" "crash-report.cpp"
// CHECKSH-NOT: "-header-include-file"
// CHECKSH-NOT: "-diagnostic-log-file"
// CHECKSH: "-D" "FOO=BAR"
Expand All @@ -63,4 +82,4 @@ FOO
// CHECKSH-NOT: "-internal-isystem" "/tmp/"
// CHECKSH-NOT: "-internal-externc-isystem" "/tmp/"
// CHECKSH-NOT: "-dwarf-debug-flags"
// CHECKSH: "crash-report-{{[^ ]*}}.c"
// CHECKSH: "crash-report-{{[^ ]*}}.cpp"

0 comments on commit 8dfbe9b

Please sign in to comment.