[cfe-commits] r114187 - in /cfe/trunk: include/clang/Basic/DiagnosticFrontendKinds.td include/clang/Frontend/CompilerInstance.h lib/Frontend/CompilerInstance.cpp test/CodeGen/2008-07-17-no-emit-on-error.c

Argyrios Kyrtzidis akyrtzi at gmail.com
Sun Sep 19 02:20:24 PDT 2010


Ugh! Thanks for looking into it.

On Sep 18, 2010, at 7:02 PM, Francois Pichet wrote:

> This commit is failing some clang lit tests sporadically on Windows
> 
> for example:
> error: unable to rename temporary 'D:/Dev/llvm/llvm_trunk/tools/clang/test/CodeG
> enObjC/Output/ns-constant-strings.m.tmp-000000' to output file 'D:\Dev\llvm\llvm
> _trunk\tools\clang\test\CodeGenObjC\Output\ns-constant-strings.m.tmp': 'Can't mo
> ve 'D:/Dev/llvm/llvm_trunk/tools/clang/test/CodeGenObjC/Output/ns-constant-strin
> gs.m.tmp-000000' to 'D:/Dev/llvm/llvm_trunk/tools/clang/test/CodeGenObjC/Output/
> ns-constant-strings.m.tmp': Access is denied.
> 
> 
> I am going to take a look at it.
> 
> On Fri, Sep 17, 2010 at 1:38 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Fri Sep 17 12:38:48 2010
>> New Revision: 114187
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=114187&view=rev
>> Log:
>> Use a temporary file for output which gets renamed after all the writing is finished.
>> 
>> This mainly prevents failures and/or crashes when multiple processes try to read/write the same PCH file. (rdar://8392711&8294781); suggestion & review by Daniel!
>> 
>> Modified:
>>    cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>>    cfe/trunk/include/clang/Frontend/CompilerInstance.h
>>    cfe/trunk/lib/Frontend/CompilerInstance.cpp
>>    cfe/trunk/test/CodeGen/2008-07-17-no-emit-on-error.c
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=114187&r1=114186&r2=114187&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Fri Sep 17 12:38:48 2010
>> @@ -69,6 +69,8 @@
>>     DefaultFatal;
>>  def err_fe_unable_to_open_output : Error<
>>     "unable to open output file '%0': '%1'">;
>> +def err_fe_unable_to_rename_temp : Error<
>> +    "unable to rename temporary '%0' to output file '%1': '%2'">;
>>  def err_fe_unable_to_open_logfile : Error<
>>     "unable to open logfile file '%0': '%1'">;
>>  def err_fe_pth_file_has_no_source_header : Error<
>> 
>> Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=114187&r1=114186&r2=114187&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
>> +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Fri Sep 17 12:38:48 2010
>> @@ -95,8 +95,23 @@
>>   /// The frontend timer
>>   llvm::OwningPtr<llvm::Timer> FrontendTimer;
>> 
>> +  /// \brief Holds information about the output file.
>> +  ///
>> +  /// If TempFilename is not empty we must rename it to Filename at the end.
>> +  /// TempFilename may be empty and Filename non empty if creating the temporary
>> +  /// failed.
>> +  struct OutputFile {
>> +    std::string Filename;
>> +    std::string TempFilename;
>> +    llvm::raw_ostream *OS;
>> +
>> +    OutputFile(const std::string &filename, const std::string &tempFilename,
>> +               llvm::raw_ostream *os)
>> +      : Filename(filename), TempFilename(tempFilename), OS(os) { }
>> +  };
>> +
>>   /// The list of active output files.
>> -  std::list< std::pair<std::string, llvm::raw_ostream*> > OutputFiles;
>> +  std::list<OutputFile> OutputFiles;
>> 
>>   void operator=(const CompilerInstance &);  // DO NOT IMPLEMENT
>>   CompilerInstance(const CompilerInstance&); // DO NOT IMPLEMENT
>> @@ -442,9 +457,8 @@
>> 
>>   /// addOutputFile - Add an output file onto the list of tracked output files.
>>   ///
>> -  /// \param Path - The path to the output file, or empty.
>> -  /// \param OS - The output stream, which should be non-null.
>> -  void addOutputFile(llvm::StringRef Path, llvm::raw_ostream *OS);
>> +  /// \param OutFile - The output file info.
>> +  void addOutputFile(const OutputFile &OutFile);
>> 
>>   /// clearOutputFiles - Clear the output file list, destroying the contained
>>   /// output streams.
>> @@ -565,7 +579,8 @@
>>   ///
>>   /// If \arg OutputPath is empty, then createOutputFile will derive an output
>>   /// path location as \arg BaseInput, with any suffix removed, and \arg
>> -  /// Extension appended.
>> +  /// Extension appended. If OutputPath is not stdout createOutputFile will
>> +  /// create a new temporary file that must be renamed to OutputPath in the end.
>>   ///
>>   /// \param OutputPath - If given, the path to the output file.
>>   /// \param Error [out] - On failure, the error message.
>> @@ -575,11 +590,14 @@
>>   /// \param Binary - The mode to open the file in.
>>   /// \param ResultPathName [out] - If given, the result path name will be
>>   /// stored here on success.
>> +  /// \param TempPathName [out] - If given, the temporary file path name
>> +  /// will be stored here on success.
>>   static llvm::raw_fd_ostream *
>>   createOutputFile(llvm::StringRef OutputPath, std::string &Error,
>>                    bool Binary = true, llvm::StringRef BaseInput = "",
>>                    llvm::StringRef Extension = "",
>> -                   std::string *ResultPathName = 0);
>> +                   std::string *ResultPathName = 0,
>> +                   std::string *TempPathName = 0);
>> 
>>   /// }
>>   /// @name Initialization Utility Methods
>> 
>> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=114187&r1=114186&r2=114187&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
>> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Sep 17 12:38:48 2010
>> @@ -35,6 +35,7 @@
>>  #include "llvm/System/Host.h"
>>  #include "llvm/System/Path.h"
>>  #include "llvm/System/Program.h"
>> +#include "llvm/System/Signals.h"
>>  using namespace clang;
>> 
>>  CompilerInstance::CompilerInstance()
>> @@ -370,18 +371,30 @@
>> 
>>  // Output Files
>> 
>> -void CompilerInstance::addOutputFile(llvm::StringRef Path,
>> -                                     llvm::raw_ostream *OS) {
>> -  assert(OS && "Attempt to add empty stream to output list!");
>> -  OutputFiles.push_back(std::make_pair(Path, OS));
>> +void CompilerInstance::addOutputFile(const OutputFile &OutFile) {
>> +  assert(OutFile.OS && "Attempt to add empty stream to output list!");
>> +  OutputFiles.push_back(OutFile);
>>  }
>> 
>>  void CompilerInstance::clearOutputFiles(bool EraseFiles) {
>> -  for (std::list< std::pair<std::string, llvm::raw_ostream*> >::iterator
>> +  for (std::list<OutputFile>::iterator
>>          it = OutputFiles.begin(), ie = OutputFiles.end(); it != ie; ++it) {
>> -    delete it->second;
>> -    if (EraseFiles && !it->first.empty())
>> -      llvm::sys::Path(it->first).eraseFromDisk();
>> +    delete it->OS;
>> +    if (!it->TempFilename.empty()) {
>> +      llvm::sys::Path TempPath(it->TempFilename);
>> +      if (EraseFiles)
>> +        TempPath.eraseFromDisk();
>> +      else {
>> +        std::string Error;
>> +        if (TempPath.renamePathOnDisk(llvm::sys::Path(it->Filename), &Error)) {
>> +          getDiagnostics().Report(diag::err_fe_unable_to_rename_temp)
>> +            << it->TempFilename << it->Filename << Error;
>> +          TempPath.eraseFromDisk();
>> +        }
>> +      }
>> +    } else if (!it->Filename.empty() && EraseFiles)
>> +      llvm::sys::Path(it->Filename).eraseFromDisk();
>> +
>>   }
>>   OutputFiles.clear();
>>  }
>> @@ -399,10 +412,11 @@
>>                                    bool Binary,
>>                                    llvm::StringRef InFile,
>>                                    llvm::StringRef Extension) {
>> -  std::string Error, OutputPathName;
>> +  std::string Error, OutputPathName, TempPathName;
>>   llvm::raw_fd_ostream *OS = createOutputFile(OutputPath, Error, Binary,
>>                                               InFile, Extension,
>> -                                              &OutputPathName);
>> +                                              &OutputPathName,
>> +                                              &TempPathName);
>>   if (!OS) {
>>     getDiagnostics().Report(diag::err_fe_unable_to_open_output)
>>       << OutputPath << Error;
>> @@ -411,7 +425,8 @@
>> 
>>   // Add the output file -- but don't try to remove "-", since this means we are
>>   // using stdin.
>> -  addOutputFile((OutputPathName != "-") ? OutputPathName : "", OS);
>> +  addOutputFile(OutputFile((OutputPathName != "-") ? OutputPathName : "",
>> +                TempPathName, OS));
>> 
>>   return OS;
>>  }
>> @@ -422,8 +437,9 @@
>>                                    bool Binary,
>>                                    llvm::StringRef InFile,
>>                                    llvm::StringRef Extension,
>> -                                   std::string *ResultPathName) {
>> -  std::string OutFile;
>> +                                   std::string *ResultPathName,
>> +                                   std::string *TempPathName) {
>> +  std::string OutFile, TempFile;
>>   if (!OutputPath.empty()) {
>>     OutFile = OutputPath;
>>   } else if (InFile == "-") {
>> @@ -436,15 +452,37 @@
>>   } else {
>>     OutFile = "-";
>>   }
>> +
>> +  if (OutFile != "-") {
>> +    llvm::sys::Path OutPath(OutFile);
>> +    // Only create the temporary if we can actually write to OutPath, otherwise
>> +    // we want to fail early.
>> +    if (!OutPath.exists() ||
>> +        (OutPath.isRegularFile() && OutPath.canWrite())) {
>> +      // Create a temporary file.
>> +      llvm::sys::Path TempPath(OutFile);
>> +      if (!TempPath.createTemporaryFileOnDisk())
>> +        TempFile = TempPath.str();
>> +    }
>> +  }
>> +
>> +  std::string OSFile = OutFile;
>> +  if (!TempFile.empty())
>> +    OSFile = TempFile;
>> 
>>   llvm::OwningPtr<llvm::raw_fd_ostream> OS(
>> -    new llvm::raw_fd_ostream(OutFile.c_str(), Error,
>> +    new llvm::raw_fd_ostream(OSFile.c_str(), Error,
>>                              (Binary ? llvm::raw_fd_ostream::F_Binary : 0)));
>>   if (!Error.empty())
>>     return 0;
>> 
>> +  // Make sure the out stream file gets removed if we crash.
>> +  llvm::sys::RemoveFileOnSignal(llvm::sys::Path(OSFile));
>> +
>>   if (ResultPathName)
>>     *ResultPathName = OutFile;
>> +  if (TempPathName)
>> +    *TempPathName = TempFile;
>> 
>>   return OS.take();
>>  }
>> 
>> Modified: cfe/trunk/test/CodeGen/2008-07-17-no-emit-on-error.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/2008-07-17-no-emit-on-error.c?rev=114187&r1=114186&r2=114187&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/2008-07-17-no-emit-on-error.c (original)
>> +++ cfe/trunk/test/CodeGen/2008-07-17-no-emit-on-error.c Fri Sep 17 12:38:48 2010
>> @@ -1,6 +1,7 @@
>>  // RUN: rm -f %t1.bc
>>  // RUN: %clang_cc1 -DPASS %s -emit-llvm-bc -o %t1.bc
>>  // RUN: test -f %t1.bc
>> +// RUN: rm -f %t1.bc
>>  // RUN: not %clang_cc1 %s -emit-llvm-bc -o %t1.bc
>>  // RUN: not test -f %t1.bc
>> 
>> 
>> 
>> _______________________________________________
>> 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