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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
