+def err_module_cache_path_not_writable : Error< + "implicitly generated module cache %0 is not writable">, + DefaultFatal;
Don't we get here for both an implicit module cache and for an explicitly-specified one? On Thu, Sep 18, 2014 at 6:45 AM, Aaron Ballman <[email protected]> wrote: > On Thu, Sep 18, 2014 at 7:50 AM, Vassil Vassilev > <[email protected]> wrote: > > On 09/17/2014 09:58 PM, Aaron Ballman wrote: > > > > On Wed, Sep 17, 2014 at 4:04 AM, Vassil Vassilev > > <[email protected]> wrote: > > > > Hi, > > I am attaching a patch addressing llvm.org/bugs/show_bug.cgi?id=20467 > > Vassil > > > > Index: lib/Frontend/CompilerInstance.cpp > > =================================================================== > > --- lib/Frontend/CompilerInstance.cpp (revision 217432) > > +++ lib/Frontend/CompilerInstance.cpp (working copy) > > @@ -339,6 +339,15 @@ > > if (!getHeaderSearchOpts().DisableModuleHash) > > llvm::sys::path::append(SpecificModuleCache, > > getInvocation().getModuleHash()); > > + > > + // If the path is not writable we can't do anything but diagnose. > > + if (llvm::sys::fs::exists(SpecificModuleCache.str()) && > > + !llvm::sys::fs::can_write(SpecificModuleCache.str())) { > > + SourceLocation NoLoc; > > + PP->getDiagnostics().Report(NoLoc, > > diag::err_module_cache_path_not_writable) > > + << SpecificModuleCache; > > + } > > + > > > > This has TOCTOU bugs -- the path could be writable at the point of > > your check, but at the point which actually attempts to create a file > > on that path can still fail. I think the correct way to handle this is > > to handle the failure at the point of using the path, not attempting > > to fail early. > > > > Thanks! IIUC a good place to do this would be > > clang::HeaderSearch::getModuleCachePath ? Would that be acceptable? > > Not quite, because that would still suffer from the same TOCTOU bug. I > was thinking more along the lines of in GlobalModuleIndex::writeIndex > and GlobalModuleIndex::readIndex (or somewhere around there). When we > go to open the file for reading/writing (create directories, etc), > that action will fail. We should be diagnosing that once the failure > occurs instead of preemptively. > > Basically, we shouldn't be doing: > > if (!can_write(blah)) > report(why); > > fopen(blah); > > instead, we should be doing: > > fopen(blah); > if (that_failed()) > report(why); > > ~Aaron > _______________________________________________ > 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
