Fix some leaks in lld's YAML parser
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 16 17:00:27 PDT 2016
Hi Pete,
I managed to catch Nick quickly, and his take matched my intuition: It's
fine to leak atom memory but we really should run the atom destructors.
I'll look into that, but it may be non-trivial. Your fix should stay in for
now.
Cheers,
Lang.
On Wed, Mar 16, 2016 at 4:36 PM, Pete Cooper <peter_cooper at apple.com> wrote:
> Thanks. r263676/r263677.
>
> Tomorrow I'm going to go and pick Nick's brain about the intended memory
> model for lld - we should either properly delete atoms, or document the
> policy that anything they contain be allocated on the same allocator.
>
> Yeah. I’ll say that i’m fine with the current model of leaking atoms in
> an allocator, but it should be documented.
>
> Pete
>
>
> - Lang.
>
> On Wed, Mar 16, 2016 at 3:57 PM, Pete Cooper via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Ping. @Lang, does this look ok?
>>
>> Pete
>> > On Feb 4, 2016, at 4:57 PM, Pete Cooper via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>> >
>> > Thanks for pinging this. I’d forgotten about it.
>> >
>> > Spoke to Nick in person. He said he was happy to defer to Lang on this
>> one.
>> >
>> > And thanks for your LGTM. Assuming Lang is ok with it i’ll commit it.
>> >
>> > Cheers,
>> > Pete
>> >> On Feb 4, 2016, at 4:53 PM, Rafael Espíndola <
>> rafael.espindola at gmail.com> wrote:
>> >>
>> >> Nick, any thought on this? It would be awesome to have check-lld asan
>> clean.
>> >>
>> >> The patch looks good to me in that all that I see is an allocator
>> >> being passed to more places, but I don't know if that was the
>> >> intention.
>> >>
>> >> Cheers,
>> >> Rafael
>> >>
>> >>
>> >> On 26 January 2016 at 10:42, Rafael Espíndola
>> >> <rafael.espindola at gmail.com> wrote:
>> >>> Nick is probably the best reviewer for the yaml code.
>> >>>
>> >>> Cheers,
>> >>> Rafael
>> >>>
>> >>>
>> >>> On 25 January 2016 at 12:34, Pete Cooper <peter_cooper at apple.com>
>> wrote:
>> >>>> Hi Rafael
>> >>>>
>> >>>> This should fix some of the leaks seen in PR 21466.
>> >>>>
>> >>>> The cause was the MappingNormalizationHeap struct which was
>> allocating atoms. In the binary file parser we allocate atoms in the
>> file::allocator() so that we can avoid the free cost later.
>> >>>>
>> >>>> This patch makes atoms use the file allocator in the YAML parser
>> when needed.
>> >>>>
>> >>>> Note, this doesn’t fix all the leaks. There’s still a bunch I’m
>> trying to understand, but its a good start.
>> >>>>
>> >>>> Thanks
>> >>>> Pete
>> >>>>
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160316/94724957/attachment.html>
More information about the llvm-commits
mailing list