aprantl added inline comments.

================
Comment at: lldb/source/Symbol/CompilerType.cpp:35
+  bool unused;
+  if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) {
+    lldbassert(false && "CompilerType belongs to deleted Typesystem");
----------------
hawkinsw wrote:
> I am sorry if this is obvious, but is `CompilerType` used in a multithreaded 
> environment? So, is there a possibility that we could pass the check on line 
> 32 but become invalid by the use of `m_type_system` here and fall victim to 
> an attempt (in `Lookup`, perhaps?) to dereference a `NULL` pointer? Again, I 
> am sorry if that is a stupid question!
Yes, CompilerType could be used on any thread and this will not catch a 
use-after-free if the TypeSystem is deleted between the IsValid check and its 
use.

A solution would be to store a TypeSystemSP/WP in CompilerType, but I'm 
expecting this to be rather expensive. The intention of this patch is as a 
bug-finding tool. It's not intended to make holding on to CompilerType objects 
safe. That said, I'm open to explore ideas for how to efficiently make this 
safe in a future patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136650/new/

https://reviews.llvm.org/D136650

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to