Save strings for CC_PRINT env vars

[SeanP]

Summary

The contents of the string returned by getenv() is not guaranteed across calls to getenv(). The code to handle the CC_PRINT etc env vars calls getenv() and saves the results in just a char *. The string returned by getenv() needs to be copied and saved. Switching the type of the strings from char * to std::string will do this and manage the alloated memory.

[hubert.reinterpretcast]

clang/include/clang/Driver/Driver.h

160

I'm seeing code left unchanged like:

TheDriver.CCPrintOptionsFilename = ::getenv("CC_PRINT_OPTIONS_FILE");

Assigning to a std::string from a null char * is known to cause things like

Segmentation fault

clang/lib/Driver/Compilation.cpp

173

Please update for the clang-format suggestions.

[SeanP]

Changed the code checking the env vars so it doesn't assign a null_ptr to the string.

[SeanP]

clang/include/clang/Driver/Driver.h

160

I've fixed in newest patch.

[hubert.reinterpretcast]

LGTM; thanks.

clang/lib/Driver/Driver.cpp

4043

Just noting that this means having the environment variable set (but empty) will now "work" instead of generating an error.

clang/include/clang/Driver/Driver.h

public:
 
   /// Driver title to use with help.
   std::string DriverTitle;
 
   /// Information about the host which can be overridden by the user.
   std::string HostBits, HostMachine, HostSystem, HostRelease;
 
   /// The file to log CC_PRINT_PROC_STAT_FILE output to, if enabled.
-  const char *CCPrintStatReportFilename;
+  std::string CCPrintStatReportFilename;
 
   /// The file to log CC_PRINT_OPTIONS output to, if enabled.
-  const char *CCPrintOptionsFilename;
+  std::string CCPrintOptionsFilename;
 
   /// The file to log CC_PRINT_HEADERS output to, if enabled.
-  const char *CCPrintHeadersFilename;
+  std::string CCPrintHeadersFilename;
 
   /// The file to log CC_LOG_DIAGNOSTICS output to, if enabled.
-  const char *CCLogDiagnosticsFilename;
+  std::string CCLogDiagnosticsFilename;
 
   /// A list of inputs and their types for the given arguments.
   typedef SmallVector<std::pair<types::ID, const llvm::opt::Arg *>, 16>
       InputList;
 
   /// Whether the driver should follow g++ like behavior.
   bool CCCIsCXX() const { return Mode == GXXMode; }
 

clang/lib/Driver/Compilation.cpp

int Compilation::ExecuteCommand(const Command &C,
                                 const Command *&FailingCommand) const {
   if ((getDriver().CCPrintOptions ||
        getArgs().hasArg(options::OPT_v)) && !getDriver().CCGenDiagnostics) {
     raw_ostream *OS = &llvm::errs();
     std::unique_ptr<llvm::raw_fd_ostream> OwnedStream;
 
     // Follow gcc implementation of CC_PRINT_OPTIONS; we could also cache the
     // output stream.
-    if (getDriver().CCPrintOptions && getDriver().CCPrintOptionsFilename) {
+    if (getDriver().CCPrintOptions && !getDriver().CCPrintOptionsFilename.empty()) {
       std::error_code EC;
       OwnedStream.reset(new llvm::raw_fd_ostream(
-          getDriver().CCPrintOptionsFilename, EC,
+          getDriver().CCPrintOptionsFilename.c_str(), EC,
           llvm::sys::fs::OF_Append | llvm::sys::fs::OF_Text));
       if (EC) {
         getDriver().Diag(diag::err_drv_cc_print_options_failure)
             << EC.message();
         FailingCommand = &C;
         return 1;
       }
       OS = OwnedStream.get();

clang/lib/Driver/Driver.cpp

 }
 
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
                DiagnosticsEngine &Diags, std::string Title,
                IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
     : Diags(Diags), VFS(std::move(VFS)), Mode(GCCMode),
       SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone), LTOMode(LTOK_None),
       ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
-      DriverTitle(Title), CCPrintStatReportFilename(nullptr),
-      CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),
-      CCLogDiagnosticsFilename(nullptr), CCCPrintBindings(false),
+      DriverTitle(Title), CCPrintStatReportFilename(),
+      CCPrintOptionsFilename(), CCPrintHeadersFilename(),
+      CCLogDiagnosticsFilename(), CCCPrintBindings(false),
       CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
       CCGenDiagnostics(false), CCPrintProcessStats(false),
       TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
       CheckInputsExist(true), GenReproducer(false),
       SuppressMissingInputWarning(false) {
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
     this->VFS = llvm::vfs::getRealFileSystem();
C.setPostCallback([=](const Command &Cmd, int Res) {
       const char *LinkingOutput = nullptr;
       if (FinalOutput)
         LinkingOutput = FinalOutput->getValue();
       else if (!Cmd.getOutputFilenames().empty())
         LinkingOutput = Cmd.getOutputFilenames().front().c_str();
       else
         LinkingOutput = getDefaultImageName();
 
-      if (!CCPrintStatReportFilename) {
+      if (CCPrintStatReportFilename.empty()) {
         using namespace llvm;
         // Human readable output.
         outs() << sys::path::filename(Cmd.getExecutable()) << ": "
                << "output=" << LinkingOutput;
         outs() << ", total="
                << format("%.3f", ProcStat->TotalTime.count() / 1000.) << " ms"
                << ", user="
                << format("%.3f", ProcStat->UserTime.count() / 1000.) << " ms"
                << ", mem=" << ProcStat->PeakMemory << " Kb\n";
       } else {
         // CSV format.
         std::string Buffer;
         llvm::raw_string_ostream Out(Buffer);
         llvm::sys::printArg(Out, llvm::sys::path::filename(Cmd.getExecutable()),
                             /*Quote*/ true);
         Out << ',';
         llvm::sys::printArg(Out, LinkingOutput, true);
         Out << ',' << ProcStat->TotalTime.count() << ','
             << ProcStat->UserTime.count() << ',' << ProcStat->PeakMemory
             << '\n';
         Out.flush();
         std::error_code EC;
-        llvm::raw_fd_ostream OS(CCPrintStatReportFilename, EC,
+        llvm::raw_fd_ostream OS(CCPrintStatReportFilename.c_str(), EC,
                                 llvm::sys::fs::OF_Append);
         if (EC)
           return;
         auto L = OS.lock();
         if (!L) {
           llvm::errs() << "ERROR: Cannot lock file "
                        << CCPrintStatReportFilename << ": "
                        << toString(L.takeError()) << "\n";

clang/lib/Driver/ToolChains/Clang.cpp

#endif
 
   if (Args.getLastArg(options::OPT_H)) {
     CmdArgs.push_back("-H");
     CmdArgs.push_back("-sys-header-deps");
   }
 
   if (D.CCPrintHeaders && !D.CCGenDiagnostics) {
     CmdArgs.push_back("-header-include-file");
-    CmdArgs.push_back(D.CCPrintHeadersFilename ? D.CCPrintHeadersFilename
+    CmdArgs.push_back(!D.CCPrintHeadersFilename.empty() ? D.CCPrintHeadersFilename.c_str()
                                                : "-");
     CmdArgs.push_back("-sys-header-deps");
   }
   Args.AddLastArg(CmdArgs, options::OPT_P);
   Args.AddLastArg(CmdArgs, options::OPT_print_ivar_layout);
 
   if (D.CCLogDiagnostics && !D.CCGenDiagnostics) {
     CmdArgs.push_back("-diagnostic-log-file");
-    CmdArgs.push_back(D.CCLogDiagnosticsFilename ? D.CCLogDiagnosticsFilename
+    CmdArgs.push_back(!D.CCLogDiagnosticsFilename.empty() ? D.CCLogDiagnosticsFilename.c_str()
                                                  : "-");
   }
 
   // Give the gen diagnostics more chances to succeed, by avoiding intentional
   // crashes.
   if (D.CCGenDiagnostics)
     CmdArgs.push_back("-disable-pragma-debug-crash");