[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

Mon Apr 19 16:26:59 PDT 2021

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM. I still kinda like the unique_ptr deleter but let's not block bot-fixes with refactoring requests. I'll open a review for the unique_ptr as a follow up.

In D100800#2699984 <https://reviews.llvm.org/D100800#2699984>, @MaskRay wrote:

> In D100800#2699940 <https://reviews.llvm.org/D100800#2699940>, @teemperor wrote:
>
>> Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot for LLDB...
>>
>> I just have some minor comments but otherwise this LGTM.
>
> Agree.. The 45+ `check-lldb` failures need to be fixed first..

Do you have a list of test failures around? Otherwise I can run the test suite myself when I'm back in the (home) office.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
             attrs.is_inline);
+        free(buf);

----------------
MaskRay wrote:
> teemperor wrote:
> > `std::free` ?
> `std::` for C library functions is uncommon.
> 
> For some common functions (free,strcpy,memset,memcpy,...), the unqualified version is more common. I can find some `::foo` as well but `std::foo` is really rare.
I believe we're using that more often in newer code (including the demangler), but that was more of nit-pick.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800