[clang] Make crash reproducer work with clang-cl

[thakis]

Summary

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.

[hans]

lgtm, thanks for digging into this!

[dyung]

Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot after your change. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/10939

[wolfgangp]

In D110783#3035158, @dyung wrote:

Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot after your change. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/10939

Seems like adding -fexceptions to the response file fixes it.

[thakis]

In D110783#3035158, @dyung wrote:

Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot after your change. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/10939

Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f

[thakis]

Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f

Your bot cycled green after that change: https://lab.llvm.org/buildbot/#/builders/139/builds/10976

[Orlando]

In D110783#3036403, @thakis wrote:

Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f

Your bot cycled green after that change: https://lab.llvm.org/buildbot/#/builders/139/builds/10976

Thanks for the fix

clang/lib/Driver/Driver.cpp

phases::ID Driver::getFinalPhase(const DerivedArgList &DAL,
                                  Arg **FinalPhaseArg) const {
   Arg *PhaseArg = nullptr;
   phases::ID FinalPhase;
 
   // -{E,EP,P,M,MM} only run the preprocessor.
   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.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT__precompile))) {
     FinalPhase = phases::Precompile;
 
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_fsyntax_only)) ||
void Driver::generateCompilationDiagnostics(
   if (FailingCommand.getCreator().isLinkJob() ||
       FailingCommand.getCreator().isDsymutilJob())
     return;
 
   // Print the version of the compiler.
   PrintVersion(C, llvm::errs());
 
   // Suppress driver output and emit preprocessor output to temp file.
-  Mode = CPPMode;
   CCGenDiagnostics = true;
 
   // Save the original job command(s).
   Command Cmd = FailingCommand;
 
   // Keep track of whether we produce any errors while trying to produce
   // preprocessed sources.
   DiagnosticErrorTrap Trap(Diags);
if (A->getOption().getKind() == Option::InputClass) {
             Ty = types::TY_Fortran;
           } else {
             // If running with -E, treat as a C input (this changes the
             // builtin macros, for example). This may be overridden by -ObjC
             // below.
             //
             // 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);
             Ty = types::TY_C;
           }
         } else {
           // Otherwise lookup by extension.
           // Fallback is C if invoked as C preprocessor, C++ if invoked with
           // clang-cl /E, or Object otherwise.
           // We use a host hook here because Darwin at least has its own
           // idea of what .s is.
           if (const char *Ext = strrchr(Value, '.'))
             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;
           }
 
           // If the driver is invoked as C++ compiler (like clang++ or c++) it
           // should autodetect some input files as C++ for g++ compatibility.
           if (CCCIsCXX()) {
             types::ID OldTy = Ty;

clang/test/Driver/crash-report-clang-cl.c

-// RUN: rm -rf %t
-// RUN: mkdir %t
-
-// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1           \
-// RUN: not %clang_cl -fsyntax-only /Brepro /source-charset:utf-8 \
-// RUN:     -- %s 2>&1 | FileCheck %s
-// RUN: cat %t/crash-report-*.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:
-// CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-clang-cl-{{.*}}.c
-// 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.c"
-// CHECKSH: "crash-report-{{[^ ]*}}.c"

clang/test/Driver/crash-report-clang-cl.cpp

+// 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"

clang/test/Driver/crash-report.cpp

 // RUN: rm -rf %t
 // RUN: mkdir %t
 
 // RUN: echo '-fsyntax-only                                              \
 // RUN:  -F/tmp/ -I /tmp/ -idirafter /tmp/ -iquote /tmp/ -isystem /tmp/  \
 // 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
 
 #ifdef PARSER
 #pragma clang __debug parser_crash
 #elif CRASH
 #pragma clang __debug crash
 #elif FATAL
 #pragma clang __debug llvm_fatal_error
 #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"
 // CHECKSH: "-D" "BAR=BAZ QUX"
 // CHECKSH-NOT: "-F/tmp/"
 // CHECKSH-NOT: "-I" "/tmp/"
 // CHECKSH-NOT: "-idirafter" "/tmp/"
 // CHECKSH-NOT: "-iquote" "/tmp/"
 // CHECKSH-NOT: "-isystem" "/tmp/"
 // CHECKSH-NOT: "-iprefix" "/the/prefix"
 // CHECKSH-NOT: "-iwithprefix" "/tmp/"
 // CHECKSH-NOT: "-iwithprefixbefore" "/tmp/"
 // CHECKSH-NOT: "-internal-isystem" "/tmp/"
 // CHECKSH-NOT: "-internal-externc-isystem" "/tmp/"
 // CHECKSH-NOT: "-dwarf-debug-flags"
-// CHECKSH: "crash-report-{{[^ ]*}}.c"
+// CHECKSH: "crash-report-{{[^ ]*}}.cpp"