[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

[Manman Ren via Phabricator via cfe-commits]

manmanren added a comment.

I forgot to upload the testing case, I will add it now.

Manman



================
Comment at: include/clang/Basic/FileManager.h:176
+  /// Manage memory buffers associated with pcm files.
+  std::unique_ptr<PCMCache> BufferMgr;
+
----------------
benlangmuir wrote:
> Why is this inside the FileManager? It isn't used by the FileManager.
I initially had another patch that puts PCMCache object at the top level (i.e whenever we create a FileManager, we create a PCMCache object). The initial patch is much bigger than this patch. PCMCache is cacheing the content of a PCM File, and is shared across threads that we spawned to build modules implicitly, just like FileManager. I had some discussions with Bruno and Adrian, we felt FileManager is a better place. But if you have other suggestions, please let me know!


================
Comment at: include/clang/Basic/FileManager.h:308
+  /// a thread, we pop a ThreadContext.
+  struct ThreadContext {
+    /// Keep track of all module files that have been validated in this thread
----------------
benlangmuir wrote:
> Can we call this something like "ModuleLoadContext" or maybe "ModuleCompilationContext"?  The word thread is misleading, since the threads are not run concurrently, and are only an implementation detail of our crash recovery.
> 
> Also, can you explain why it is only necessary to keep information about the the current stack of contexts?  In a situation like
> 
> A imports B imports C
> A imports D imports C
> 
> We would no longer have information about C being validated, right?  What happens if there's a mismatch there?  Can this never happen?
Sure, we can call it ModuleCompilationContext because we are spawning threads to build modules implicitly.

The issue we are trying to detect is use-after-free, i.e we don't want a thread to hold on to an invalid FileEntry. We error out before trying to delete the FileEntry that is live in another thread.

In a situation like
A imports B imports C
A imports D imports C

A will start a thread to build B, and B will start a thread to build C, C will be validated in B's compilation thread and A's compilation thread. Later on, if D tries to invalidate C, D knows that an ancestor thread A has validated C, so the compiler will error out. And it is okay for A to invalidate C in A's compilation thread, because we can clean up the references to C's FileEntry.



https://reviews.llvm.org/D28299