r211411 - Driver: Record that we're in crashdump and push flags to ConstructJob

Justin Bogner mail at justinbogner.com
Mon Jun 23 15:18:50 PDT 2014


Jordan Rose <jordan_rose at apple.com> writes:
> Hm. .i isn't really correct for something that still has macros in
> it. IIRC Clang happens to expand them again, but if we've merely
> rewritten includes I would expect the resulting file to have a .c
> extension.

It's a bit tricky to get this right consistently in a crashdump context,
due to the way the logic is split between the driver and cc1. Notably, I
can fairly easily special case the driver here to use a .c extension for
crashdumps, but as of r211421 that isn't always right. We don't really
know if we're going to specify rewrite-includes or not at the time that
we decide the filename.

That said, the trade off is a matter of which is worse:

1. Crashdump files named .i/.mi even though they still have macros when
   -fmodules isn't present, or

2. Crashdump files named .c/.m even though they're fully preprocessed
   when -fmodules is present.

I suppose that, given we'd like to make rewrite-includes and module
crashdumps work together in the long term, (2) is a better trade off.
It also avoids changing the status quo. Agree?

> On Jun 20, 2014, at 15:16 , Justin Bogner <mail at justinbogner.com> wrote:
>
>> Author: bogner
>> Date: Fri Jun 20 17:16:00 2014
>> New Revision: 211411
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=211411&view=rev> Log:
>> Driver: Record that we're in crashdump and push flags to ConstructJob
>> 
>> It's more flexible and arguably better layering to set flags to modify
>> compiling for diagnostics in the CC1 job themselves, rather than
>> tweaking the driver flags and letting them propagate.
>> 
>> There is one visible change this causes: crash report files will now
>> get preprocessed names (.i and friends).
>> 
>> Modified:
>>    cfe/trunk/include/clang/Driver/Compilation.h
>>    cfe/trunk/lib/Driver/Compilation.cpp
>>    cfe/trunk/lib/Driver/Driver.cpp
>>    cfe/trunk/lib/Driver/Tools.cpp
>>    cfe/trunk/test/Driver/crash-report.c
>> 
>> Modified: cfe/trunk/include/clang/Driver/Compilation.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=211411&r1=211410&r2=211411&view=diff> ==============================================================================
>> --- cfe/trunk/include/clang/Driver/Compilation.h (original)
>> +++ cfe/trunk/include/clang/Driver/Compilation.h Fri Jun 20 17:16:00 2014
>> @@ -69,6 +69,9 @@ class Compilation {
>>   /// Redirection for stdout, stderr, etc.
>>   const StringRef **Redirects;
>> 
>> +  /// Whether we're compiling for diagnostic purposes.
>> +  bool ForDiagnostics;
>> +
>> public:
>>   Compilation(const Driver &D, const ToolChain &DefaultToolChain,
>>               llvm::opt::InputArgList *Args,
>> @@ -173,6 +176,9 @@ public:
>>   /// so compilation can be reexecuted to generate additional diagnostic
>>   /// information (e.g., preprocessed source(s)).
>>   void initCompilationForDiagnostics();
>> +
>> +  /// Return true if we're compiling for diagnostics.
>> +  bool isForDiagnostics() { return ForDiagnostics; }
>> };
>> 
>> } // end namespace driver
>> 
>> Modified: cfe/trunk/lib/Driver/Compilation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=211411&r1=211410&r2=211411&view=diff> ==============================================================================
>> --- cfe/trunk/lib/Driver/Compilation.cpp (original)
>> +++ cfe/trunk/lib/Driver/Compilation.cpp Fri Jun 20 17:16:00 2014
>> @@ -26,9 +26,9 @@ using namespace llvm::opt;
>> 
>> Compilation::Compilation(const Driver &D, const ToolChain &_DefaultToolChain,
>>                          InputArgList *_Args, DerivedArgList *_TranslatedArgs)
>> -  : TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
>> -    TranslatedArgs(_TranslatedArgs), Redirects(nullptr) {
>> -}
>> +    : TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
>> +      TranslatedArgs(_TranslatedArgs), Redirects(nullptr),
>> +      ForDiagnostics(false) {}
>> 
>> Compilation::~Compilation() {
>>   delete TranslatedArgs;
>> @@ -211,6 +211,8 @@ void Compilation::ExecuteJob(const Job &
>> }
>> 
>> void Compilation::initCompilationForDiagnostics() {
>> +  ForDiagnostics = true;
>> +
>>   // Free actions and jobs.
>>   DeleteContainerPointers(Actions);
>>   Jobs.clear();
>> 
>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=211411&r1=211410&r2=211411&view=diff> ==============================================================================
>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 20 17:16:00 2014
>> @@ -419,8 +419,6 @@ void Driver::generateCompilationDiagnost
>>   // Suppress driver output and emit preprocessor output to temp file.
>>   Mode = CPPMode;
>>   CCGenDiagnostics = true;
>> -  C.getArgs().AddFlagArg(nullptr,
>> -                         Opts->getOption(options::OPT_frewrite_includes));
>> 
>>   // Save the original job command(s).
>>   std::string Cmd;
>> 
>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=211411&r1=211410&r2=211411&view=diff> ==============================================================================
>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Jun 20 17:16:00 2014
>> @@ -3339,10 +3339,6 @@ void Clang::ConstructJob(Compilation &C,
>>   if (Args.getLastArg(options::OPT_fapple_kext))
>>     CmdArgs.push_back("-fapple-kext");
>> 
>> -  if (Args.hasFlag(options::OPT_frewrite_includes,
>> -                   options::OPT_fno_rewrite_includes, false))
>> -    CmdArgs.push_back("-frewrite-includes");
>> -
>>   Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch);
>>   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_print_source_range_info);
>>   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
>> @@ -4037,6 +4033,11 @@ void Clang::ConstructJob(Compilation &C,
>>   }
>> #endif
>> 
>> +  if (Args.hasFlag(options::OPT_frewrite_includes,
>> +                   options::OPT_fno_rewrite_includes, false) ||
>> +      C.isForDiagnostics())
>> +    CmdArgs.push_back("-frewrite-includes");
>> +
>>   // Only allow -traditional or -traditional-cpp outside in preprocessing modes.
>>   if (Arg *A = Args.getLastArg(options::OPT_traditional,
>>                                options::OPT_traditional_cpp)) {
>> 
>> Modified: cfe/trunk/test/Driver/crash-report.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-report.c?rev=211411&r1=211410&r2=211411&view=diff> ==============================================================================
>> --- cfe/trunk/test/Driver/crash-report.c (original)
>> +++ cfe/trunk/test/Driver/crash-report.c Fri Jun 20 17:16:00 2014
>> @@ -7,11 +7,11 @@
>> // RUN:  -Xclang -internal-isystem -Xclang /tmp/                         \
>> // RUN:  -Xclang -internal-externc-isystem -Xclang /tmp/                 \
>> // RUN:  -DFOO=BAR 2>&1 | FileCheck %s
>> -// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
>> +// RUN: cat %t/crash-report-*.i | FileCheck --check-prefix=CHECKSRC %s
>> // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
>> // REQUIRES: crash-recovery
>> 
>> -// because of the glob (*.c, *.sh)
>> +// because of the glob (*.i, *.sh)
>> // REQUIRES: shell
>> 
>> // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH=1 %clang -fsyntax-only -x c /dev/null -lstdc++ 2>&1 | FileCheck %s
>> @@ -21,7 +21,7 @@
>> 
>> #pragma clang __debug parser_crash
>> // CHECK: Preprocessed source(s) and associated run script(s) are located at:
>> -// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
>> +// CHECK-NEXT: note: diagnostic msg: {{.*}}.i
>> FOO
>> // CHECKSRC: FOO
>> // CHECKSH: -cc1
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list