develooper Front page | perl.perl5.porters | Postings from April 2019

Re: Storable and Leaky hooks which die

Thread Previous | Thread Next
From:
Petr Pisar
Date:
April 17, 2019 11:33
Subject:
Re: Storable and Leaky hooks which die
Message ID:
slrnqbe3oe.7fd.ppisar@dhcp-0-146.brq.redhat.com
On 2019-04-17, Tony Cook <tony@develop-help.com> wrote:
> On Mon, Apr 15, 2019 at 11:48:13AM +0100, Dave Mitchell wrote:
>> Storage has some tests which leak, and thus cause smoke failures on blead
>> when run under Address Sanitizer.
>> 
>> An example of code which leaks is:
>> 
>>     use Storable qw(store);
>>     sub FreezeHookDies::STORABLE_freeze { die ${$_[0]} }
>>     my $x = bless [], "FreezeHookDies";
>>     eval { store($x, "store99"); 1 };
>> 
>> I had a quick look at the source (which I'm not very familiar with)
>> and decided it was non-trivial and non-obvious what the best way to fix
>> it is. It's in the general category of mallocing temp buffers pointed
>> to be local vars, which don't get freed if the code dies (and is caught by
>> an eval).
>> 
>> Since it looks too messy to fix for 5.30.0 I have, for now, with
>> v5.29.9-133-g2cf7500760, skipped the leaky tests when run under ASan,
>> but the skip will auto-re-enable in 5.31.x
>> 
>> Dopes anyone more familiar with the internals of Storable want to have a
>> go at fixing it, or at least suggest the best approach?
>
> For some reason I'm not seeing any leaks from an ASAN build.
>
>  ./Configure -des -Dusedevel -Accflags=-fsanitize=address -Aldflags=-fsanitize=address -DDEBUGGING -Doptimize=-O0\ -g -Dcc=/opt/gcc-8.2.0/bin/gcc
>
> valgrind picked up a leak of the ptr table from your example which is
> fixed in 1d7b2a7e3a7a0e05c254d065923488b768eb3ce0.
>
While the patch seems correct, it does not fix the issue for me. When
I run the code under gdb, the storable_free() is never called. valgrind
reports these issues after your patch:

==12089== 9 bytes in 1 blocks are possibly lost in loss record 15 of 835
==12089==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==12089==    by 0x4AF5E1: Perl_safesysmalloc (util.c:153)
==12089==    by 0x4AFAB6: Perl_savepvn (util.c:1084)
==12089==    by 0x4E8210: Perl_sv_magicext (sv.c:5733)
==12089==    by 0x4ECCB3: Perl_sv_magic (sv.c:5834)
==12089==    by 0x44E0E1: Perl_Gv_AMupdate (gv.c:2892)
==12089==    by 0x44E438: Perl_amagic_call (gv.c:3138)
==12089==    by 0x44FEBD: Perl_amagic_deref_call (gv.c:3072)
==12089==    by 0x4F8F3B: Perl_pp_rv2sv (pp.c:263)
==12089==    by 0x4CE285: Perl_runops_standard (run.c:41)
==12089==    by 0x43E954: Perl_call_sv (perl.c:3026)
==12089==    by 0x52EECC4: array_call (Storable.xs:2224)
==12089==    by 0x52EECC4: store_hook (Storable.xs:3751)
==12089==    by 0x52EECC4: store_blessed (Storable.xs:4162)

==12089== 16 bytes in 1 blocks are possibly lost in loss record 70 of 835
==12089==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==12089==    by 0x4AF5E1: Perl_safesysmalloc (util.c:153)
==12089==    by 0x4D8381: Perl_pp_entersub (pp_hot.c:5142)
==12089==    by 0x43EA1B: Perl_call_sv (perl.c:3026)
==12089==    by 0x52EECC4: array_call (Storable.xs:2224)
==12089==    by 0x52EECC4: store_hook (Storable.xs:3751)
==12089==    by 0x52EECC4: store_blessed (Storable.xs:4162)
==12089==    by 0x52F1556: store (Storable.xs:4492)
==12089==    by 0x52F2B4B: do_store.isra.8 (Storable.xs:4685)
==12089==    by 0x52F2FCF: XS_Storable_pstore (Storable.xs:7829)

==12089== 48 bytes in 1 blocks are possibly lost in loss record 283 of 835
==12089==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==12089==    by 0x4AF5E1: Perl_safesysmalloc (util.c:153)
==12089==    by 0x4DE83A: Perl_ptr_table_new (sv.c:13810)
==12089==    by 0x52F2A08: init_store_context (Storable.xs:1625)
==12089==    by 0x52F2A08: do_store.isra.8 (Storable.xs:4674)
==12089==    by 0x52F2FCF: XS_Storable_pstore (Storable.xs:7829)

==12089== 48 bytes in 1 blocks are possibly lost in loss record 284 of 835
==12089==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==12089==    by 0x4B038C: Perl_safesyscalloc (util.c:439)
==12089==    by 0x4E816A: Perl_sv_magicext (sv.c:5685)
==12089==    by 0x4ECCB3: Perl_sv_magic (sv.c:5834)
==12089==    by 0x44E0E1: Perl_Gv_AMupdate (gv.c:2892)
==12089==    by 0x44E438: Perl_amagic_call (gv.c:3138)
==12089==    by 0x44FEBD: Perl_amagic_deref_call (gv.c:3072)
==12089==    by 0x4F8F3B: Perl_pp_rv2sv (pp.c:263)
==12089==    by 0x4CE285: Perl_runops_standard (run.c:41)
==12089==    by 0x43E954: Perl_call_sv (perl.c:3026)
==12089==    by 0x52EECC4: array_call (Storable.xs:2224)
==12089==    by 0x52EECC4: store_hook (Storable.xs:3751)
==12089==    by 0x52EECC4: store_blessed (Storable.xs:4162)
==12089==    by 0x52F1556: store (Storable.xs:4492)

==12089== 56 bytes in 1 blocks are possibly lost in loss record 321 of 835
==12089==    at 0x483AD19: realloc (vg_replace_malloc.c:836)
==12089==    by 0x4AFBA9: Perl_safesysrealloc (util.c:271)
==12089==    by 0x4E693A: Perl_sv_grow (sv.c:1596)
==12089==    by 0x4DFE90: S_sv_catpvn_simple (sv.c:10996)
==12089==    by 0x4DFE90: Perl_sv_vcatpvfn_flags (sv.c:12021)
==12089==    by 0x4E3BD1: Perl_sv_catpvf (sv.c:10896)
==12089==    by 0x4B0875: Perl_mess_sv (util.c:1435)
==12089==    by 0x4B00F2: Perl_vcroak (util.c:1695)
==12089==    by 0x4B0293: Perl_die (util.c:1630)
==12089==    by 0x4F9037: Perl_pp_rv2sv (pp.c:268)
==12089==    by 0x4CE285: Perl_runops_standard (run.c:41)
==12089==    by 0x43E954: Perl_call_sv (perl.c:3026)
==12089==    by 0x52EECC4: array_call (Storable.xs:2224)
==12089==    by 0x52EECC4: store_hook (Storable.xs:3751)
==12089==    by 0x52EECC4: store_blessed (Storable.xs:4162)

==12089== 64 bytes in 1 blocks are possibly lost in loss record 335 of 835
==12089==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==12089==    by 0x4B038C: Perl_safesyscalloc (util.c:439)
==12089==    by 0x4C9368: Perl_hv_common (hv.c:590)
==12089==    by 0x4CA743: Perl_hv_common_key_len (hv.c:339)
==12089==    by 0x52EBDAB: pkg_fetchmeth (Storable.xs:2053)
==12089==    by 0x52EBDAB: pkg_can (Storable.xs:2133)
==12089==    by 0x52EE976: store_blessed (Storable.xs:4160)
==12089==    by 0x52F1556: store (Storable.xs:4492)
==12089==    by 0x52F2B4B: do_store.isra.8 (Storable.xs:4685)
==12089==    by 0x52F2FCF: XS_Storable_pstore (Storable.xs:7829)

==12089== 4,096 bytes in 1 blocks are possibly lost in loss record 762 of 835
==12089==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==12089==    by 0x4B038C: Perl_safesyscalloc (util.c:439)
==12089==    by 0x4DE874: Perl_ptr_table_new (sv.c:13816)
==12089==    by 0x52F2A08: init_store_context (Storable.xs:1625)
==12089==    by 0x52F2A08: do_store.isra.8 (Storable.xs:4674)
==12089==    by 0x52F2FCF: XS_Storable_pstore (Storable.xs:7829)

==12089== 8,192 bytes in 1 blocks are possibly lost in loss record 801 of 835
==12089==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==12089==    by 0x4AF5E1: Perl_safesysmalloc (util.c:153)
==12089==    by 0x4DEAA4: Perl_ptr_table_store (sv.c:13872)
==12089==    by 0x52F14D5: store (Storable.xs:4472)
==12089==    by 0x52F2B4B: do_store.isra.8 (Storable.xs:4685)
==12089==    by 0x52F2FCF: XS_Storable_pstore (Storable.xs:7829)

(There are other leaks pertaining boot_Storable() that I ignored).

Let's look at the last report: do_store() calls store() at 4685 and store()
makes a long jump because of the die exception. Thus do_store() never
advances to clean_store_context() at 4717 that would free the
cxt->pseen. The cxt is probably supposed to be deallocated when
unwinding a stack while handling the exception. However, I have no
idea how it is this implemented in Perl.

-- Petr

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About