[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

Mon Jan 9 13:49:15 PST 2023

JDevlieghere added a comment.

In D141318#4037640 <https://reviews.llvm.org/D141318#4037640>, @augusto2112 wrote:

> In D141318#4037557 <https://reviews.llvm.org/D141318#4037557>, @JDevlieghere wrote:
>
>> In D141318#4037541 <https://reviews.llvm.org/D141318#4037541>, @augusto2112 wrote:
>>
>>> I changed the value of the map to a shared pointer, but could change it to a weak pointer instead, not sure which is more appropriate in this case.
>>
>> I was about to ask the same thing. What currently determines the lifetime of the stored objects?
>
> Currently, a bunch of functions in `DWARFASTParserClang` and `SymbolFileDWARF` will create a new `Type` SP, store the underlying pointer in the map, and return the SP. The lifetime of the stored objects varies by whatever the caller does with the returned SP. Looking at some of the callers, I don't see a very clear pattern, most seem to use it and immediately discard it  (use after free when we look up the same type again and the underlying pointer is still in the map), but at least one caller (`SymbolFileDWARF::ResolveType`) returns the underlying pointer to //its// callers, which would still be UB even if we changed the map to store weak pointers. Based on that I'm thinking we should store them as shared pointers instead of weak ones.

Then maybe the solution is to store the type as unique pointers in the map and hand out raw pointers, relying on the map to keep the object alive? I feel pretty uncomfortable with handing out shared pointers without clearly understanding its lifetime cycle.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141318