Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.88k stars 530 forks source link

7d6175e breaks each.t and hash.t with -DDEBUGGING, PERL_DESTRUCT_LEVEL #11459

Closed p5pRT closed 13 years ago

p5pRT commented 13 years ago

Migrated from rt.perl.org#93454 (status was 'resolved')

Searchable as RT93454$

p5pRT commented 13 years ago

From @craigberry

With PERL_DESTRUCT_LEVEL set in the environment on a -DDEBUGGING build\, t/op/hash.t and t/op/each.t now end with​:

Unbalanced string table refcount​: (1) for "a" during global destruction.

On VMS\, this causes the tests to fail like so​:

t/op/each......................................................FAILED--unexpected output at test 56 t/op/hash......................................................FAILED--unexpected output at test 9

Presumably due to some difference in the test driver\, the tests don't fail on other platforms even though the warning is easily reproducible. I've bisected it on Mac OS X with results below.

This is the report from git bisect​:

7d6175ef71f6339fae97e36c1cdae9e4f47f74d0 is the first bad commit commit 7d6175ef71f6339fae97e36c1cdae9e4f47f74d0 Author​: Father Chrysostomos \sprout@​cpan\.org Date​: Sun Jun 12 14​:46​:44 2011 -0700

  Completely free hashes containing nulls  
  This fixes a regression introduced since 5.14.0\, by commit e0171a1a3.  
  The new Perl_hfree_next_entry function that that commit introduced   returns the value of the hash element\, or NULL if there are none left.   If the value of the hash element is NULL\, the two cases are indistin-   guishable.  
  Before e0171a1a3\, all the hash code took null values into account.   mro_package_moved took advantage of that\, stealing values out of a   hash and leaving it to the freeing code to delete the elements.  
  The two places that call Perl_hfree_next_entry (there was only one\,   S_hfreeentries\, with commit e0171a1a3\, but the following commit\,   104d7b699c\, made sv_clear call it\, too) were not accounting for NULL   values’ being returned\, and could terminate early\, resulting in mem-   ory leaks.  
  One could argue that the perl core should not be assigning nulls to   HeVAL\, but HeVAL is part of the public API and there could be CPAN   code assigning NULL to it\, too.  
  So the safest approach seems to be to modify Perl_hfree_next_entry’s   callers to check the number of keys and not to attribute a signifi-   cance to a returned NULL.

:040000 040000 49c4c8eae8aff3a90bce904cb698542878aa3563 52262ef3e181def0d84640b6768b3666678479ee M ext :100644 100644 51c782a3bfcd4ca4aed07b50daf7240dbd5d5169 a230c167fcf73ce1ae04321ea9bfa5a3e002140d M hv.c :100644 100644 faddfdc8aa33bf52a0f9060cf7650c06646d7863 6750ef197ec952cf3261eb31810ea536962dad00 M sv.c bisect run success

This was my bisect script​:

% cat ~/run #!/bin/tcsh git clean -dxf

sh Configure -des -Dusedevel -DDEBUGGING test -f config.sh || exit 125 make test_prep [ -x ./perl ] || exit 125 setenv PERL_DESTRUCT_LEVEL 2 set ret=0 ./perl -Ilib t/op/hash.t 2>&tmp.log grep -c 'global destruction' tmp.log test $? -gt 0 || set ret=2 git clean -dxf exit $ret

% ./perl -Ilib -V Summary of my perl5 (revision 5 version 15 subversion 0) configuration​:   Commit id​: 30b0736d971c494432c032b4ca9fd2e8dcd31680   Platform​:   osname=darwin\, osvers=10.7.0\, archname=darwin-2level   uname='darwin triamond.local 10.7.0 darwin kernel version 10.7.0​: sat jan 29 15​:17​:16 pst 2011; root​:xnu-1504.9.37~1release_i386 i386 '   config_args='-Dusedevel -DDEBUGGING -des'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=undef\, usemultiplicity=undef   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=define\, use64bitall=define\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -I/opt/local/include'\,   optimize='-O3 -g'\,   cppflags='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -I/opt/local/include'   ccversion=''\, gccversion='4.2.1 (Apple Inc. build 5666) (dot 3)'\, gccosandvers=''   intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc'\, ldflags =' -fstack-protector -L/usr/local/lib -L/opt/local/lib'   libpth=/usr/local/lib /opt/local/lib /usr/lib   libs=-ldbm -ldl -lm -lutil -lc   perllibs=-ldl -lm -lutil -lc   libc=/usr/lib/libc.dylib\, so=dylib\, useshrplib=false\, libperl=libperl.a   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=bundle\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags=' '\, lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -L/opt/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:   Compile-time options​: DEBUGGING PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP   PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL   USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE   USE_LOCALE_COLLATE USE_LOCALE_CTYPE   USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF   Built under darwin   Compiled at Jun 23 2011 21​:30​:43   @​INC​:   lib   /usr/local/lib/perl5/site_perl/5.15.0/darwin-2level   /usr/local/lib/perl5/site_perl/5.15.0   /usr/local/lib/perl5/5.15.0/darwin-2level   /usr/local/lib/perl5/5.15.0   /usr/local/lib/perl5/site_perl   .

________________________________________ Craig A. Berry mailto​:craigberry@​mac.com

"... getting out of a sonnet is much more difficult than getting in."   Brad Leithauser

p5pRT commented 13 years ago

From @cpansprout

On Thu Jun 23 20​:11​:19 2011\, craigberry wrote​:

With PERL_DESTRUCT_LEVEL set in the environment on a -DDEBUGGING build\, t/op/hash.t and t/op/each.t now end with​:

Unbalanced string table refcount​: (1) for "a" during global destruction.

That’s odd. That’s precisely the warning I was getting from mro.c before making that change.

p5pRT commented 13 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 13 years ago

From @cpansprout

This is now fixed with commit 6d1c68e64.

p5pRT commented 13 years ago

@cpansprout - Status changed from 'open' to 'resolved'