[modules][Patch][BugzillaID#20467] Diagnose if -fmodules-cache-path is not writeable

[Vassil Vassilev]

On 09/17/2014 09:58 PM, Aaron Ballman wrote:
> On Wed, Sep 17, 2014 at 4:04 AM, Vassil Vassilev
> <vasil.georgiev.vasilev at cern.ch> 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 
<http://clang.llvm.org/doxygen/classclang_1_1HeaderSearch.html#a99ea51bfb4bb5f4742de10c8dfde0540> 
? Would that be acceptable?
Vassil
>
> ~Aaron

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/fde16071/attachment.html>