[cfe-commits] [PATCH] Fixes tooling segfaults when reusing the stat cache from pch loading in subsequent translation units

[Manuel Klimek]

On Tue, Jul 31, 2012 at 3:44 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
>
> Sent from my iPhone
>
> On Jul 31, 2012, at 4:58 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> Hi Doug,
>>
>> this is hopefully exactly what you proposed in the other thread (not
>> unexpectedly, your analysis turned out to be spot on ;)
>
> Looks good, thanks!

Thanks. Submitted as r161047.

>
>> Thanks!
>> /Manuel
>>
>> diff --git a/include/clang/Basic/FileManager.h
>> b/include/clang/Basic/FileManager.h
>> index 930523f..b00f2b7 100644
>> --- a/include/clang/Basic/FileManager.h
>> +++ b/include/clang/Basic/FileManager.h
>> @@ -186,6 +186,9 @@ public:
>>   /// \brief Removes the specified FileSystemStatCache object from the manager.
>>   void removeStatCache(FileSystemStatCache *statCache);
>>
>> +  /// \brief Removes all FileSystemStatCache objects from the manager.
>> +  void clearStatCaches();
>> +
>>   /// \brief Lookup, cache, and verify the specified directory (real or
>>   /// virtual).
>>   ///
>> diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
>> index b921f3e..c6b894c 100644
>> --- a/lib/Basic/FileManager.cpp
>> +++ b/lib/Basic/FileManager.cpp
>> @@ -223,6 +223,10 @@ void
>> FileManager::removeStatCache(FileSystemStatCache *statCache) {
>>   PrevCache->setNextStatCache(statCache->getNextStatCache());
>> }
>>
>> +void FileManager::clearStatCaches() {
>> +  StatCache.reset(0);
>> +}
>> +
>> /// \brief Retrieve the directory that the given file name resides in.
>> /// Filename can point to either a real file or a virtual file.
>> static const DirectoryEntry *getDirectoryFromFile(FileManager &FileMgr,
>> diff --git a/lib/Tooling/Tooling.cpp b/lib/Tooling/Tooling.cpp
>> index 5d41172..e93e0c9 100644
>> --- a/lib/Tooling/Tooling.cpp
>> +++ b/lib/Tooling/Tooling.cpp
>> @@ -212,6 +212,7 @@ bool ToolInvocation::runInvocation(
>>   const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
>>
>>   Compiler.resetAndLeakFileManager();
>> +  Files->clearStatCaches();
>>   return Success;
>> }
>>
>> diff --git a/test/Tooling/Inputs/pch-fail.h b/test/Tooling/Inputs/pch-fail.h
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/test/Tooling/Inputs/pch.cpp b/test/Tooling/Inputs/pch.cpp
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/test/Tooling/Inputs/pch.h b/test/Tooling/Inputs/pch.h
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/test/Tooling/pch.cpp b/test/Tooling/pch.cpp
>> new file mode 100644
>> index 0000000..bfa20d8
>> --- /dev/null
>> +++ b/test/Tooling/pch.cpp
>> @@ -0,0 +1,21 @@
>> +// This is a regression test for handling of stat caches within the tooling
>> +// infrastructure. This test reproduces the problem under valgrind:
>> +
>> +// First, create a pch that we can later load. Loading the pch will insert
>> +// a stat cache into the FileManager:
>> +// RUN: %clang -x c++-header %S/Inputs/pch.h -o %t1
>> +
>> +// Use the generated pch and enforce a subsequent stat miss by by using
>> +// the test file with an unrelated include as second translation unit:
>> +// Do not directly pipe into FileCheck, as that would hide errors from
>> +// valgrind due to pipefail not being set in lit.
>> +// RUN: clang-check "%S/Inputs/pch.cpp" "%s" -- -include-pch %t1 -I
>> "%S" -c >%t2 2>&1
>> +// RUN: FileCheck %s < %t2
>> +
>> +#include "Inputs/pch-fail.h"
>> +
>> +// CHECK: Processing
>> +
>> +// FIXME: This is incompatible to -fms-compatibility.
>> +// XFAIL: win32
>> +
>> <fix-stat-cache.patch>