develooper Front page | perl.perl5.porters | Postings from March 2003

[unPATCH] Re: Smoke [5.8.1] 19076 FAIL(F) openbsd 3.2 (i386/1 cpu)

From:
Nicholas Clark
Date:
March 29, 2003 11:11
Subject:
[unPATCH] Re: Smoke [5.8.1] 19076 FAIL(F) openbsd 3.2 (i386/1 cpu)
Message ID:
20030329185807.GL274@Bagpuss.unfortu.net
On Sat, Mar 29, 2003 at 02:06:58AM +0100, Abe Timmerman wrote:
> Op een frisse lentedag (Friday 28 March 2003 22:19), schreef Nicholas Clark:
> 
> > On Fri, Mar 28, 2003 at 05:42:00PM +0100, Abe Timmerman wrote:
> > > F - F -      -Uuseperlio
> > >
> > > openbsd     [stdio       ]-Dcc='ccache egcc'
> > >               (cont) -Dcf_email=abe@ztreet.demon.nl -Uuseperlio
> > > openbsd     [stdio       ]-DDEBUGGING -Dcc='ccache egcc'
> > >               (cont) -Dcf_email=abe@ztreet.demon.nl -Uuseperlio
> > >     ext/Devel/Peek/Peek..................FAILED at test 14
> > > ../ext/Devel/Peek/Peek....FAILED test 14
> >
> > How does this fail? I've just built perl with -Uperlio and it passed all
> > tests.
> 
> This is 5.8.1, no problems with bleadperl, but here it is:
> 
> [SV = RV(0xe3850) at 0xc5210
>   REFCNT = 1
>   FLAGS = (ROK)
>   RV = 0xde288
>   SV = PVCV(0xcea30) at 0xde288
>     REFCNT = 3
>     FLAGS = ()
>     IV = 0
>     NV = 0
>     COMP_STASH = 0xc5084        "main"
>     START = 0xf5bc0 ===> 2522
>     ROOT = 0x110e40
>     XSUB = 0x0
>     XSUBANY = 0
>     GVGV::GV = 0xded50  "main" :: "do_test"
>     FILE = "../ext/Devel/Peek/Peek.t"
>     DEPTH = 1
>     FLAGS = 0x0
>     OUTSIDE_SEQ = 217
>     PADLIST = 0xde294
>     PADNAME = 0xde2a0(0xfab00) PAD = 0xde2ac(0xfaa00)
>        1. 0xde2b8<1> (217,225) "_"
>       19. 0xd021c<2> FAKE "_"
>       20. 0xc8708<1> (218,219) "_"
>     OUTSIDE = 0xc5288 (MAIN)
> ] vs [SV = RV\(0x[[:xdigit:]]+\) at 0x[[:xdigit:]]+
>   REFCNT = 1
>   FLAGS = \(ROK\)
>   RV = 0x[[:xdigit:]]+
>   SV = PVCV\(0x[[:xdigit:]]+\) at 0x[[:xdigit:]]+
>     REFCNT = (3|4)
>     FLAGS = \(\)
>     IV = 0
>     NV = 0
>     COMP_STASH = 0x[[:xdigit:]]+\t"main"
>     START = 0x[[:xdigit:]]+ ===> \d+
>     ROOT = 0x[[:xdigit:]]+
>     XSUB = 0x0
>     XSUBANY = 0
>     GVGV::GV = 0x[[:xdigit:]]+\t"main" :: "do_test"
>     FILE = ".*\b(?i:peek\.t)"
>     DEPTH = 1
> (?:    MUTEXP = 0x[[:xdigit:]]+
>     OWNER = 0x[[:xdigit:]]+
> )?    FLAGS = 0x0
>     OUTSIDE_SEQ = \d+
>     PADLIST = 0x[[:xdigit:]]+
>     PADNAME = 0x[[:xdigit:]]+\(0x[[:xdigit:]]+\) PAD = 0x[[:xdigit:]]+\(0x[[:xdigit:]]+\)
>        \d+\. 0x[[:xdigit:]]+<\d+> \(\d+,\d+\) "\$pattern"
>       \d+\. 0x[[:xdigit:]]+<\d+> FAKE "\$DEBUG"
>       \d+\. 0x[[:xdigit:]]+<\d+> \(\d+,\d+\) "\$dump"
>     OUTSIDE = 0x[[:xdigit:]]+ \(MAIN\)]
> not ok 14

On Tue, Jan 07, 2003 at 10:31:17AM +0000, hv@crypt.org wrote:
> Nicholas Clark <nick@unfortu.net> wrote:
> :On Mon, Dec 23, 2002 at 06:22:02AM +0000, hv@crypt.org wrote:
> :> John Peacock <jpeacock@rowman.com> wrote:
> :> :Nicholas Clark wrote:
> :> :> I wonder how many parts of the perl core can be reduced slightly in size
> :> :> by changing %s SvPV_nolen(sv) to %_ sv.
> :> :
> :> :I was wondering that myself.  I may give it a shot, but it might be a fairly 
> :> :megapatch.  We'll see if Hugo likes it...
> :> 
> :> Sounds like a fine idea to me: less code, no speed penalty that I can see.
> :> If the patch is very large, best not to spam p5p with it - you can send
> :> it to me direct, or put it up for ftp/http somewhere.
> :
> :It's not huge. I left out patching the OS/2 specific code in Devel::DProf,
> :and I'll send the patch to ext/Encode/Unicode/Unicode.xs direct to Dan
> :Kogai
> 
> Thanks, the three that refer to PerlIO_printf:
> :+++ ./pad.c    Wed Dec 25 21:20:45 2002
> :@@ -878,8 +878,8 @@ Perl_intro_my(pTHX)
> :@@ -926,8 +926,8 @@ Perl_pad_leavemy(pTHX)
> :+++ ./sv.c Wed Dec 25 19:24:59 2002
> :@@ -9359,8 +9360,9 @@ Perl_sv_dup(pTHX_ SV *sstr, CLONE_PARAMS
> 
> give compiler warnings:
> pad.c: In function `Perl_intro_my':
> pad.c:878: warning: unknown conversion type character `_' in format
> pad.c:878: warning: long unsigned int format, pointer arg (arg 4)
> pad.c:878: warning: too many arguments for format
> 
> I'm not quite sure why, since PerlIO_printf does not map to a system
> library call for this build; however, it would do so in some other
> cases:
> perlsdio.h:#define PerlIO_printf                        PerlSIO_printf
> perlsfio.h:#define PerlIO_printf                        sfprintf
> iperlsys.h: #define PerlSIO_printf            Perl_fprintf_nocontext
> iperlsys.h: #define PerlSIO_printf                    fprintf
> perlsdio.h: #define PerlIO_printf                     PerlSIO_printf
> .. and I'm not sure what would happen in those situations.
> 
> I've applied the patch without those three fragments as #18456.

And it turns out that I was wrong; this patch did introduce bugs. Jarkko
integrated it into maint as 18881, and it's the cause of test 14 failing.
Reverting it fixes that bug:

--- pad.c.orig	Tue Mar 11 08:13:07 2003
+++ pad.c	Sat Mar 29 18:06:39 2003
@@ -1187,21 +1187,21 @@ Perl_do_dump_pad(pTHX_ I32 level, PerlIO
 	if (namesv) {
 	    if (SvFAKE(namesv))
 		Perl_dump_indent(aTHX_ level+1, file,
-		    "%2d. 0x%"UVxf"<%lu> FAKE \"%"SVf"\"\n",
+		    "%2d. 0x%"UVxf"<%lu> FAKE \"%s\"\n",
 		    (int) ix,
 		    PTR2UV(ppad[ix]),
 		    (unsigned long) (ppad[ix] ? SvREFCNT(ppad[ix]) : 0),
-		    namesv
+		    SvPVX(namesv)
 		);
 	    else
 		Perl_dump_indent(aTHX_ level+1, file,
-		    "%2d. 0x%"UVxf"<%lu> (%lu,%lu) \"%"SVf"\"\n",
+		    "%2d. 0x%"UVxf"<%lu> (%lu,%lu) \"%s\"\n",
 		    (int) ix,
 		    PTR2UV(ppad[ix]),
 		    (unsigned long) (ppad[ix] ? SvREFCNT(ppad[ix]) : 0),
 		    (unsigned long)I_32(SvNVX(namesv)),
 		    (unsigned long)SvIVX(namesv),
-		    namesv
+		    SvPVX(namesv)
 		);
 	}
 	else if (full) {

I'm not sure what got reverted in blead as the Changes only go up to 30th
December.

However, I think that all my SVf changes in dump.c should be reverted, as
they all also call Perl_dump_indent, and it is implemented as a call to
PerlIO_printf, which on stdio doesn't understand %_
pad.c and dump.c seem to be the only 2 functions which call Perl_dump_indent

No regression tests for any of dump.c's calls that currently use %_?

On Wed, Jan 08, 2003 at 11:12:15AM +0000, Dave Mitchell wrote:
> On Tue, Jan 07, 2003 at 10:40:54PM +0000, Nicholas Clark wrote:
> > On Tue, Jan 07, 2003 at 06:01:49PM +0100, Rafael Garcia-Suarez wrote:
> > > hv@crypt.org wrote:
> > > > Thanks, the three that refer to PerlIO_printf:
> > > > :+++ ./pad.c    Wed Dec 25 21:20:45 2002
> > > > :@@ -878,8 +878,8 @@ Perl_intro_my(pTHX)
> > > > :@@ -926,8 +926,8 @@ Perl_pad_leavemy(pTHX)
> > > > :+++ ./sv.c Wed Dec 25 19:24:59 2002
> > > > :@@ -9359,8 +9360,9 @@ Perl_sv_dup(pTHX_ SV *sstr, CLONE_PARAMS
> > > > 
> > > > give compiler warnings:
> > > > pad.c: In function `Perl_intro_my':
> > > > pad.c:878: warning: unknown conversion type character `_' in format
> > > > pad.c:878: warning: long unsigned int format, pointer arg (arg 4)
> > > > pad.c:878: warning: too many arguments for format
> 
> Note that %_ should *not* be used to print SVs containing the names
> of pad lexicals, since for these the SvCUR() field is hijacked for the
> generation number. Sticking with %s and PVX(sv) is the Right Thing to do
> here.  (See PAD_COMPNAME_GEN()).

Is all this code exclusively in pad.c now? ie does the above reversion
remove any wrong use of %_ ?

Nicholas Clark



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