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