[PATCH] D146412: [NFC] Fix potential use-after-free in DumpModuleInfoAction::ExecuteAction()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146412#4227644 <https://reviews.llvm.org/D146412#4227644>, @Fznamznon wrote:

>> Should OutputStream be made into a shared_ptr so we can clarify the memory ownership instead of leaving it as a raw pointer? That should also address the use-after-free.
>
> Well, I was thinking about that and wasn't sure about it either. https://reviews.llvm.org/D129456 added initialization of `OutputStream` from the outside. When last `shared_ptr` is destroyed, it destroys undelying object as well. In case underlying object came from the outside in a form of raw pointer, we may end up accidentally deleting it when `DumpModuleInfoAction` is destroyed.

Yeah, exposing that data member was not the cleanest way to implement this functionality. This does fix the issue, but it keeps the unclean design -- I wonder if we could make that member private and use a constructor to specify it, then we can enforce the shared ownership property. (If this suggestion turns out to be a slog, then I'm okay with the current approach as well.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146412