Bug #133424 for PadWalker: [PATCH] Names of utf8 lexical vars not reported correctly

Sat Sep 26 08:09:12 2020 jpl [...] plosquare.com - Ticket created

Subject:	[PATCH] Names of utf8 lexical vars not reported correctly
From:	jpl [...] plosquare.com

Reporting against perl v5.28.1 and PadWalker-2.3 With "use utf8" when Perl source code contains names

Sat Sep 26 08:18:07 2020 jpl [...] plosquare.com - Correspondence added

Reporting against perl v5.28.1 and PadWalker-2.3 With "use utf8" when Perl source code contains lexical variables with names that contain non-ASCII characters, the keys representing those variables are incorrectly reported by peek_my without the utf8 flag. Example code: use Data::Dumper; use PadWalker; use utf8; my $ü = 'bla'; # u umlaut print Dumper(eval { PadWalker::peek_my(0) }); will output $VAR1 = { '$ÃŒ' => \'bla' }; instead of the expected UTF-8 encoded key $VAR1 = { "\$\x{fc}" => \'bla' }; The attached patch fixes the problem (although there may be other instances of it - I did not understand or examine all potentially affected invocations of hv_store contained in the code).

Subject:

padwalker-2.3-utf8.patch

--- old/PadWalker-2.3/PadWalker.xs 2015-10-23 18:55:04.000000000 +0200 +++ new/PadWalker-2.3/PadWalker.xs 2020-09-26 13:57:51.972498043 +0200 @@ -269,7 +269,7 @@ if (!val_sv) val_sv = &PL_sv_undef; } - hv_store((is_our ? our_hash : my_hash), name_str, name_len, + hv_store((is_our ? our_hash : my_hash), name_str, PadnameUTF8(name_sv) ? -name_len : name_len, (val_sv ? newRV_inc(val_sv) : &PL_sv_undef), 0); } }

Sat Sep 26 08:33:28 2020 jpl [...] plosquare.com - Correspondence added

(This bug causes a variables display bug in EPIC Perl IDE - https://sourceforge.net/p/e-p-i-c/bugs/714/)

Sat Sep 26 13:44:17 2020 robin [...] cpan.org - Correspondence added

Thanks! I’ve applied your patch, and version 2.4 is uploading to CPAN now.

Sat Sep 26 13:44:18 2020 The RT System itself - Status changed from 'new' to 'open'

Sat Sep 26 13:45:19 2020 jpl [...] plosquare.com - Correspondence added

Subject:	Re: [rt.cpan.org #133424] [PATCH] Names of utf8 lexical vars not reported correctly
Date:	Sat, 26 Sep 2020 19:45:09 +0200
To:	bug-PadWalker [...] rt.cpan.org
From:	Jan Ploski <jpl [...] plosquare.com>

Excellent! Thank you! On 9/26/20 7:44 PM, Robin Houston via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=133424 > > > Thanks! I’ve applied your patch, and version 2.4 is uploading to CPAN now.

Sun Sep 27 06:23:37 2020 http://pghmcfc.id.fedoraproject.org/ - Correspondence added

On Sat Sep 26 13:45:19 2020, jploski wrote: Show quoted text

> Excellent! Thank you! > > On 9/26/20 7:44 PM, Robin Houston via RT wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=133424 > > > > > Thanks! I’ve applied your patch, and version 2.4 is uploading to CPAN now.

This seems to break PadWalker with Perls older than about 5.18, where the PadnameUTF8 symbol is not available: $ make test PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t Can't load '/builddir/build/BUILD/PadWalker-2.4/blib/arch/auto/PadWalker/PadWalker.so' for module PadWalker: /builddir/build/BUILD/PadWalker-2.4/blib/arch/auto/PadWalker/PadWalker.so: undefined symbol: PadnameUTF8 at /usr/lib/perl5/DynaLoader.pm line 190. at t/closure.t line 2. Compilation failed in require at t/closure.t line 2. BEGIN failed--compilation aborted at t/closure.t line 2. t/closure.t ... Dubious, test returned 255 (wstat 65280, 0xff00) ... Test Summary Report ------------------- t/closure.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/dm.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/foo.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/our.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/recurse.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/sub.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/test.t (Wstat: 65280 Tests: 1 Failed: 1) Failed test: 1 Non-zero exit status: 255 Parse errors: Bad plan. You planned 15 tests but ran 1. t/tt.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output t/var_name.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: No plan found in TAP output Files=9, Tests=1, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.04 cusr 0.00 csys = 0.05 CPU) Result: FAIL Failed 9/9 test programs. 1/1 subtests failed. make: *** [test_dynamic] Error 255

Sun Sep 27 07:36:35 2020 robin [...] cpan.org - Correspondence added

Sorry. That’ll teach me to apply a patch without testing it properly. Will fix.

Sun Sep 27 08:37:41 2020 robin [...] cpan.org - Correspondence added

On Sun Sep 27 06:23:37 2020, http://pghmcfc.id.fedoraproject.org/ wrote: Show quoted text

> This seems to break PadWalker with Perls older than about 5.18, where > the PadnameUTF8 symbol is not available:

Sorry! This should be fixed in 2.5, which is uploading now.

Sun Sep 27 10:10:10 2020 jpl [...] plosquare.com - Correspondence added

On Sun Sep 27 07:36:35 2020, ROBIN wrote: Show quoted text

> Sorry. That'll teach me to apply a patch without testing it properly.

Same goes for my submission... Here is what I found out: Prior to perl-5.18.4 there was no PadnameUTF8 macro (so I decided that the fix should not target such older versions): https://github.com/Perl/perl5/commit/35e035ccb58e33947405288b1177fb0b9c1da197 Macro was added as "PADNAME_UTF8" https://github.com/Perl/perl5/commit/86d2498c0b3132b9d76f935b317b213effc43990 Macro was renamed to PadnameUTF8 (and appears this way in perl-5.18.4) https://github.com/Perl/perl5/commit/2502ffdfca07fac6972c9b2da7ae160d011c2877 Macro always returns true (and appears this way in perl-5.22.4) I'm attaching an improved patch, which is intended to keep the behavior unchanged for Perl versions prior to 5.22.4, and fixed starting from 5.22.4. With earlier versions of Perl the behavior is broken and I see no way to fix, e.g. even after the macro was introduced, the ü is converted for some reason to uppercase Ü in perl-5.18.4, and before that to Ã in perl-5.10.0. perl-5.10.0 however is able to unexpectedly accept a Latin-1 encoded ü together with the "use utf8" macro and then kinda works, leaving the ü unchanged. So I guess we should not hope for making it right with old versions of Perl, but of course keep backward compatibility.

Subject:

padwalker-2.3-utf8-v2.patch

--- old/PadWalker-2.3/PadWalker.xs 2015-10-23 18:55:04.000000000 +0200 +++ new/PadWalker-2.3/PadWalker.xs 2020-09-27 15:49:00.420197404 +0200 @@ -95,6 +95,13 @@ # define PadnamePV(pn) (SvPOKp(pn) ? SvPVX(pn) : NULL) #endif +#ifndef PadnameUTF8 +# if PERL_VERSION >= 22 +# define PadnameUTF8(pn) 1 /* in case the macro is removed some day */ +# else +# define PadnameUTF8(pn) 0 /* to compile with perl < 5.18.4 */ +# endif +#endif /* Originally stolen from pp_ctl.c; now significantly different */ @@ -269,7 +276,7 @@ if (!val_sv) val_sv = &PL_sv_undef; } - hv_store((is_our ? our_hash : my_hash), name_str, name_len, + hv_store((is_our ? our_hash : my_hash), name_str, PadnameUTF8(name_sv) ? -name_len : name_len, (val_sv ? newRV_inc(val_sv) : &PL_sv_undef), 0); } }

Sun Sep 27 10:27:28 2020 jpl [...] plosquare.com - Correspondence added

On Sun Sep 27 08:37:41 2020, ROBIN wrote: Show quoted text

> On Sun Sep 27 06:23:37 2020, http://pghmcfc.id.fedoraproject.org/ wrote:

> > This seems to break PadWalker with Perls older than about 5.18, where > > the PadnameUTF8 symbol is not available:

> > Sorry! This should be fixed in 2.5, which is uploading now.

Your patch looks simpler than mine, but I'm worried it may not be enough: PadnameUTF8 is documented as follows: "NOTE: this function is experimental and may change or be removed without notice. Whether PadnamePV is in UTF-8. Currently, this is always true." Also see comment in https://github.com/Perl/perl5/commit/2502ffdfca07fac6972c9b2da7ae160d011c2877, which indicates that pad names will be "always UTF8". So it seems reasonable to treat them as such when passing to hv_store in newer perl versions. The problem is that if the macro PadnameUTF8 is removed in a future version of perl, then your patch will fall back on the 2.3 behavior and essentially undo/break the fix from this ticket...