[PATCH] D78950: Adds LRU caching of compile units in DWARFContext.

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

I think this is definitely cleaner than the previous patch. However, I still see some potential problems here. The way that this patch implements the reference counting means not all "usages" of DWARFUnits will be recorded. For example, during a call to `GetLocalsForAddress`, we end up calling `Die.getAttributeValueAsReferencedDie(DW_AT_type))` here <https://github.com/llvm/llvm-project/blob/master/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp#L1155>. The type DIE may end up being in a different compile unit (DW_FORM_ref_addr, DW_FORM_ref_sig8, DW_FORM_GNU_ref_alt, only the first one seems to be implemented correctly), which can cause additional units to be parsed "behind your back". Now, the question is what to do about it...

If we don't do anything about that, then the code will still be "correct", but it may end up using more memory than you expect. And if we try to do something about it, then we're back to the problem of DIEs disappearing from under you -- that function continues to use the `Die` object after it gets the type, but getting the type can cause the memory that `Die` points to to "disappear"...

I don't know whether you're ok with this quirk of the implementation. I also don't know whether we are ok with it. At this point it would really be nice to get someone else's opinion on all of this...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78950