<div dir="ltr"><pre style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">+def err_module_cache_path_not_writable : Error<
+  "implicitly generated module cache %0 is not writable">, 
+  DefaultFatal;
</pre><div>Don't we get here for both an implicit module cache and for an explicitly-specified one?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 18, 2014 at 6:45 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, Sep 18, 2014 at 7:50 AM, Vassil Vassilev<br>
<div><div class="h5"><<a href="mailto:vasil.georgiev.vasilev@cern.ch">vasil.georgiev.vasilev@cern.ch</a>> wrote:<br>
> On 09/17/2014 09:58 PM, Aaron Ballman wrote:<br>
><br>
> On Wed, Sep 17, 2014 at 4:04 AM, Vassil Vassilev<br>
> <<a href="mailto:vasil.georgiev.vasilev@cern.ch">vasil.georgiev.vasilev@cern.ch</a>> wrote:<br>
><br>
> Hi,<br>
>   I am attaching a patch addressing <a href="http://llvm.org/bugs/show_bug.cgi?id=20467" target="_blank">llvm.org/bugs/show_bug.cgi?id=20467</a><br>
> Vassil<br>
><br>
> Index: lib/Frontend/CompilerInstance.cpp<br>
> ===================================================================<br>
> --- lib/Frontend/CompilerInstance.cpp (revision 217432)<br>
> +++ lib/Frontend/CompilerInstance.cpp (working copy)<br>
> @@ -339,6 +339,15 @@<br>
>    if (!getHeaderSearchOpts().DisableModuleHash)<br>
>      llvm::sys::path::append(SpecificModuleCache,<br>
>                              getInvocation().getModuleHash());<br>
> +<br>
> +  // If the path is not writable we can't do anything but diagnose.<br>
> +  if (llvm::sys::fs::exists(SpecificModuleCache.str()) &&<br>
> +       !llvm::sys::fs::can_write(SpecificModuleCache.str())) {<br>
> +    SourceLocation NoLoc;<br>
> +    PP->getDiagnostics().Report(NoLoc,<br>
> diag::err_module_cache_path_not_writable)<br>
> +      << SpecificModuleCache;<br>
> +  }<br>
> +<br>
><br>
> This has TOCTOU bugs -- the path could be writable at the point of<br>
> your check, but at the point which actually attempts to create a file<br>
> on that path can still fail. I think the correct way to handle this is<br>
> to handle the failure at the point of using the path, not attempting<br>
> to fail early.<br>
><br>
> Thanks! IIUC a good place to do this would be<br>
> clang::HeaderSearch::getModuleCachePath ? Would that be acceptable?<br>
<br>
</div></div>Not quite, because that would still suffer from the same TOCTOU bug. I<br>
was thinking more along the lines of in GlobalModuleIndex::writeIndex<br>
and GlobalModuleIndex::readIndex (or somewhere around there). When we<br>
go to open the file for reading/writing (create directories, etc),<br>
that action will fail. We should be diagnosing that once the failure<br>
occurs instead of preemptively.<br>
<br>
Basically, we shouldn't be doing:<br>
<br>
if (!can_write(blah))<br>
  report(why);<br>
<br>
fopen(blah);<br>
<br>
instead, we should be doing:<br>
<br>
fopen(blah);<br>
if (that_failed())<br>
  report(why);<br>
<div class="HOEnZb"><div class="h5"><br>
~Aaron<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>