labath added a comment.

The Halloween theme is nice, but I think it'd be more traditional if we kept a 
list of active instances, instead of inactive ones. That said, a part of me 
dies every time I see a class keeping a list of its instances.

I don't know if you've seen it, but I think the `Module::m_old_symfiles` member 
exists for the very same reason, and it could be why this does not happen so 
often outside of swift (which, I believe, stores most of its types in the 
target type system instead of the module systems).



================
Comment at: lldb/source/Symbol/CompilerType.cpp:35
+  bool unused;
+  if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) {
+    lldbassert(false && "CompilerType belongs to deleted Typesystem");
----------------
aprantl wrote:
> 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.
Well.. if you're only interested in SB uses, then I'd say that the SBType 
object should hold an SP/WP on the type system or some parent of it.


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