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

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

Reply via email to