clang-cl: Don't store the cl compiler Tool on the stack (PR20131)

[hans]

Summary

The Command will refer back to the Tool as its source, so it has to outlive the Command.

Having the Tool on the stack would cause us to crash when using "clang-cl -GR -fallback", because if the Command fails, Driver::ExecuteCompilation tries to peek at the Command's source.

[rnk]

include/clang/Driver/ToolChain.h

72 ↗

(On Diff #10890)

You should be able to get away with forward declaring tools::visualstudio::Compile, and then using that type here and on the getter.

72 ↗

(On Diff #10890)

Can we make the Clang tool own the CL tool when it's doing fallback? I think that should solve the lifetime issues without adding CL to the base Toolchain class.

[hans]

include/clang/Driver/ToolChain.h

72 ↗

(On Diff #10890)

Yes, that seems nicer.

[hans]

Make the cl tool a member of the Clang tool instead.

[rnk]

lgtm

[hans]

Closed by commit rL211802 (authored by @hans).

lib/Driver/Tools.h

namespace driver {
   class Command;
   class Driver;
 
 namespace toolchains {
   class MachO;
 }
 
 namespace tools {
+
+namespace visualstudio {
+  class Compile;  
+};
+
 using llvm::opt::ArgStringList;
 
   /// \brief Clang compiler tool.
   class LLVM_LIBRARY_VISIBILITY Clang : public Tool {
   public:
     static const char *getBaseInputName(const llvm::opt::ArgList &Args,
                                         const InputInfoList &Inputs);
     static const char *getBaseInputStem(const llvm::opt::ArgList &Args,
private:
 
     ObjCRuntime AddObjCRuntimeArgs(const llvm::opt::ArgList &args,
                                    llvm::opt::ArgStringList &cmdArgs,
                                    RewriteKind rewrite) const;
 
     void AddClangCLArgs(const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs) const;
 
+    visualstudio::Compile *getCLFallback() const;
+
+    mutable std::unique_ptr<visualstudio::Compile> CLFallback;
+
   public:
     Clang(const ToolChain &TC) : Tool("clang", "clang frontend", TC) {}
 
     bool hasGoodDiagnostics() const override { return true; }
     bool hasIntegratedAssembler() const override { return true; }
     bool hasIntegratedCPP() const override { return true; }
 
     void ConstructJob(Compilation &C, const JobAction &JA,
public:
     void ConstructJob(Compilation &C, const JobAction &JA,
                       const InputInfo &Output,
                       const InputInfoList &Inputs,
                       const llvm::opt::ArgList &TCArgs,
                       const char *LinkingOutput) const override;
   };
 } // end namespace dragonfly
 
-  /// Visual studio tools.
 namespace visualstudio {
   class LLVM_LIBRARY_VISIBILITY Link : public Tool {
   public:
     Link(const ToolChain &TC) : Tool("visualstudio::Link", "linker", TC) {}
 
     bool hasIntegratedCPP() const override { return false; }
     bool isLinkJob() const override { return true; }
 

lib/Driver/Tools.cpp

if (SplitDwarf) {
     SplitDwarfOut = SplitDebugName(Args, Inputs);
     CmdArgs.push_back(SplitDwarfOut);
   }
 
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
       Output.getType() == types::TY_Object &&
       (InputType == types::TY_C || InputType == types::TY_CXX)) {
-    tools::visualstudio::Compile CL(getToolChain());
-    Command *CLCommand = CL.GetCommand(C, JA, Output, Inputs, Args,
-                                       LinkingOutput);
+    Command *CLCommand = getCLFallback()->GetCommand(C, JA, Output, Inputs,
+                                                     Args, LinkingOutput);
     // RTTI support in clang-cl is a work in progress.  Fall back to MSVC early
     // if we are using 'clang-cl /fallback /GR'.
     // FIXME: Remove this when RTTI is finished.
     if (Args.hasFlag(options::OPT_frtti, options::OPT_fno_rtti, false)) {
       D.Diag(diag::warn_drv_rtti_fallback) << CLCommand->getExecutable();
       C.addCommand(CLCommand);
     } else {
       C.addCommand(new FallbackCommand(JA, *this, Exec, CmdArgs, CLCommand));
if (!Args.hasArg(options::OPT_fdiagnostics_format_EQ)) {
     CmdArgs.push_back("-fdiagnostics-format");
     if (Args.hasArg(options::OPT__SLASH_fallback))
       CmdArgs.push_back("msvc-fallback");
     else
       CmdArgs.push_back("msvc");
   }
 }
 
+visualstudio::Compile *Clang::getCLFallback() const {
+  if (!CLFallback)
+    CLFallback.reset(new visualstudio::Compile(getToolChain()));
+  return CLFallback.get();
+}
+
 void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
                            const InputInfo &Output,
                            const InputInfoList &Inputs,
                            const ArgList &Args,
                            const char *LinkingOutput) const {
   ArgStringList CmdArgs;
 
   assert(Inputs.size() == 1 && "Unexpected number of inputs.");