From 382b430aeac8fd5420ef79b3efbc2518f6c83136 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Tue, 25 Mar 2025 14:58:58 +1100 Subject: [PATCH 1/4] eval: ensure debugging saved lines have an IV part perldebguts documents that the lines stored in @{"_<$filename"} arrays have a numeric value in addition to the text of the source, ensure that is true for evals. Non-zero IV values indicate the lines are breakable (they represent the address of the COP for that line) Fixes #23151 --- MANIFEST | 1 + pp_ctl.c | 3 +++ t/op/debug.t | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 t/op/debug.t diff --git a/MANIFEST b/MANIFEST index 7db1f4610f26..ed35c4d1d516 100644 --- a/MANIFEST +++ b/MANIFEST @@ -6305,6 +6305,7 @@ t/op/cproto.t Check builtin prototypes t/op/crypt.t See if crypt works t/op/current_sub.t __SUB__ tests t/op/dbm.t See if dbmopen/dbmclose work +t/op/debug.t Test mechanisms used by the debugger t/op/decl-refs.t See if my \$foo works t/op/defer.t See if defer blocks work t/op/defined.t See if defined() edge cases work diff --git a/pp_ctl.c b/pp_ctl.c index c9419ae79fec..f983dd761476 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3744,6 +3744,9 @@ S_save_lines(pTHX_ AV *array, SV *sv) t = send; sv_setpvn_fresh(tmpstr, s, t - s); + /* not breakable until we compile a COP for it */ + SvIV_set(tmpstr, 0); + SvIOK_on(tmpstr); av_store(array, line++, tmpstr); s = t; } diff --git a/t/op/debug.t b/t/op/debug.t new file mode 100644 index 000000000000..e3d4c1e3cda2 --- /dev/null +++ b/t/op/debug.t @@ -0,0 +1,64 @@ +#!./perl + +# intended for testing language mechanisms that debuggers use not for +# testing the debugger itself + +BEGIN { + chdir 't' if -d 't'; + require "./test.pl"; + set_up_inc( qw(. ../lib) ); +} + +use strict; +use warnings; + +SKIP: +{ + skip_if_miniperl("need XS", 1); + # github 23151 + # trivial debugger + local $ENV{PERL5DB} = 'sub DB::DB {}'; + # eval code trimmed from code generated by Sub::Quote + fresh_perl_is(<<'CODE', <<'EXPECT', +use B qw(SVf_IOK); + +sub _do_eval { + eval $_[0] or die $!; +} + +_do_eval(<<'EVAL'); +{ + sub table { + } +} +1; +EVAL + +# look for lines that don't have an IV set +my ($f) = grep /\(eval/, keys %::; +my $x = $::{$f}; +my $lineno = 0; +for my $l (@$x) { + if ($l) { + my $b = B::svref_2object(\$l); + if (!($b->FLAGS & SVf_IOK)) { + print "No IV for $f line $lineno: $l\n"; + last + } + } + ++$lineno; +} + +print "Done\n"; +CODE +Done +EXPECT + { + switches => [ '-d' ], + stderr => 1, + }, + "saved lines all have an IV" + ); +} + +done_testing(); From 57d1f60fa71f973bf4537b15a63e5bd6e5918faa Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Mon, 31 Mar 2025 15:39:54 +1100 Subject: [PATCH 2/4] perldelta for ensuring saved debugger source lines have IV parts --- pod/perldelta.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index baf1d278054a..0469be3b726f 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -366,7 +366,8 @@ manager will later use a regex to expand these into links. =item * -XXX +In some cases an C would not add integer parts to the source +lines saved by the debugger. [GH #23151] =back From faa3343f712c41db7c4958f5e6e8824ce9891113 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Mon, 7 Apr 2025 11:05:42 +1000 Subject: [PATCH 3/4] debugger source lines: use PVIV instead of PVMG for source lines These were saved as PVMG but as bulk88 suggested in https://github.com/Perl/perl5/pull/23171#issuecomment-2780007725 we only need PVIV, since the source lines don't need magic, aren't blessed and store an integer, not an NV. So create them as PVIV instead. If it turns out we do need PVMG instead for some future use, simply remove the test added here, it's simply to confirm we don't need PVMG here. --- pp_ctl.c | 2 +- t/op/debug.t | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ toke.c | 5 +++-- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index f983dd761476..70a53f06fc72 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3735,7 +3735,7 @@ S_save_lines(pTHX_ AV *array, SV *sv) while (s && s < send) { const char *t; - SV * const tmpstr = newSV_type(SVt_PVMG); + SV * const tmpstr = newSV_type(SVt_PVIV); t = (const char *)memchr(s, '\n', send - s); if (t) diff --git a/t/op/debug.t b/t/op/debug.t index e3d4c1e3cda2..d0dcb03a1529 100644 --- a/t/op/debug.t +++ b/t/op/debug.t @@ -61,4 +61,64 @@ EXPECT ); } +SKIP: +{ + # Historically lines were stored as PVMG, but we don't need + # magic on these lines. + # + # This checks that none of these lines get upgraded, ie. that + # we don't need them to be PVMG + # + # If this test fails perhaps we do need to make them PVMG + # and toke.c:S_update_debugger_info and pp_ctl.c:S_save_lines + # can be switched back to using SVt_PVMG and this test + # removed. + # + # See https://github.com/Perl/perl5/pull/23171#issuecomment-2780007725 + skip_if_miniperl("need B"); + local $ENV{PERL5DB} = 'sub DB::DB {}'; + fresh_perl_is(<<'CODE', <<'EXPECT', +use B; + +sub _do_eval { + eval $_[0] or die $!; +} + +_do_eval(<<'EVAL'); + +sub some_code { + print "Hello"; +} + +1; +EVAL + +# check if any lines have been upgraded from PVIV +my @files = grep /^_ [ '-d' ], + stderr => 1, + }, + "saved lines are all PVIV" + ); +} + done_testing(); diff --git a/toke.c b/toke.c index 8fb5f83b7b88..e6acb5a70177 100644 --- a/toke.c +++ b/toke.c @@ -2012,10 +2012,11 @@ S_update_debugger_info(pTHX_ SV *orig_sv, const char *const buf, STRLEN len) AV *av = CopFILEAVx(PL_curcop); if (av) { SV * sv; - if (PL_parser->preambling == NOLINE) sv = newSV_type(SVt_PVMG); + if (PL_parser->preambling == NOLINE) + sv = newSV_type(SVt_PVIV); else { sv = *av_fetch(av, 0, 1); - SvUPGRADE(sv, SVt_PVMG); + SvUPGRADE(sv, SVt_PVIV); } if (!SvPOK(sv)) SvPVCLEAR(sv); if (orig_sv) From 3b40a0a3498ce4241b298bed248e5b119a16b068 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Mon, 7 Apr 2025 11:06:02 +1000 Subject: [PATCH 4/4] perldelta for save source lines as PVIV instead of as PVMG --- pod/perldelta.pod | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index 0469be3b726f..90be67b5bd38 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -369,6 +369,14 @@ manager will later use a regex to expand these into links. In some cases an C would not add integer parts to the source lines saved by the debugger. [GH #23151] +=item * + +Save debugger lines as C SVs rather than as C SVs as they +don't need magic, aren't blessed and don't need to store a floating +point part. This should save 24 bytes per stored line for 64-bit +systems, more for C<-Duselongdouble> or C<-Dusequadmath> builds. +Discussed in [GH #23171]. + =back =head1 Known Problems