Skip to content

Improve -Dx, -Dt etc output, especially in threaded builds #23322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: blead
Choose a base branch
from

Conversation

iabyn
Copy link
Contributor

@iabyn iabyn commented May 24, 2025

This series of commits improves the way various OPs are displayed by Perl_op_dump() (as used by perl -Dx) and by Perl_debop() (as used by perl -Dt).

In particular it tries, where possible, to display information about an SV or GV attached to the OP when that SV has been moved to the pad on threaded builds.

  • This set of changes does not require a perldelta entry.

iabyn added 2 commits May 24, 2025 12:16
Currently SVOPs and METHOPs are dumped using the same case branch within
op_dump(). This works at the moment because the op_sv field and the
op_meth_sv field happen to occupy the same offset within their
respective structs.

This commit separates out the handling of those two OP classes, which will
also allow another commit shortly to handle METHOD ops more specifically.

OP_COREARGS is also added as another dumpable SVOP type (i.e. an op
which may have an SV hanging off op_sv).

This commit also makes it so that it now always displays the value of the
op_sv field of SVOPs, which on threaded builds starts off holding an SV,
but later gets set to 0 to indicate that the SV has been moved to the
pad.
Some types of method call have a redirect class in addition to the
method name, e.g. $obj->BAR::foo(). This value 'BAR' wasn't being
displayed by op_dump(). So this commit makes it do so.

I also took the opportunity to add comments to the various OP_METHOD_FOO
cases to identify what sort of method calls it handled, and added a
stub OP_METHOD case rather than it just being handled by the default
branch. This is to make it clearer that OP_METHOD *does* exist, but it
doesn't have any values (like 'foo' or 'BAR') which need dumping.
@iabyn iabyn added the defer-next-dev This PR should not be merged yet, but await the next development cycle label May 24, 2025
iabyn added 5 commits May 24, 2025 15:53
Some OPs, such as OP_CONST, OP_GVSV and OP_METHOD_NAMED, point to an SV
or GV. In threaded builds, these SVs are moved to the pad and an index is
stored in the OP instead (typically op_targ or op_padix).

When op_dump() is called upon to display an OP (typically during
debugging or via perl -Dx), then currently, information about the linked
SV (e.g. the glob's name) is displayed only on non-threaded builds,
since op_dump() can't assume that PL_curpad[] is associated with this
particular op. Thus you get things like an OP_CONST being dumped that
doesn't display the const's value. This is annoying during debugging.

This commit makes it so that when dumping common OPs which have an SV in
the pad, it tries to find the CV, if any, associated with that op, and
if so, uses that CV's pad to lookup the value. If unsuccessful, it
falls back to not displaying the SV. This commit uses two main
techniques to find the CV. Both rely on first following the op_parent
chain from the current op to find the root op of the optree which this
op is embedded in. Then, if compiling, it compares this with the roots
of the optrees currently on the parse stack, and so, uses the associated
CV which is is pointed to from that slot on the parse stack.  Or, if
runtime, looks for a SUB or EVAL context on the context stack and sees
if that sub or eval's CvROOT() / PL_eval_root matches the root of the
op's tree.

The next two commits will extend this to handle 'perl -Dx' too.

This commit also tries to show the state of the fields on CONST and
METHOD_FOO ops which can hold an SV or index, in addition to showing the SV
that is retrieved from them.

Here are examples of some op dumps on threaded builds before and after
this commit:

    --------------------------------------------------------------

    const SVOP(0x2a051578) ===> 6 [gvsv 0x2a0515e8]
     TARG = 2
     FLAGS = (SCALAR,SLABBED,MORESIB)

    gvsv PADOP(0x2a0515e8) ===> 5 [sassign 0x2a051538]
     FLAGS = (SCALAR,SLABBED)
     PADIX = 1

    method_redir METHOP(0x13dd4318) ===> 5 [entersub 0x13dd4358]
     TARG = 4
     FLAGS = (UNKNOWN,SLABBED)

    --------------------------------------------------------------

    const SVOP(0x22f655b8) ===> 6 [gvsv 0x22f65628]
     TARG = 2
     FLAGS = (SCALAR,SLABBED,MORESIB)
     OP_SV = 0x0
     SV = PV("abc"\0) (0x22f65768)

    gvsv PADOP(0x22f65628) ===> 5 [sassign 0x22f65578]
     FLAGS = (SCALAR,SLABBED)
     PADIX = 1
     GV = main::x (0x22f58f20)

    method_redir METHOP(0x1d83f318) ===> 5 [entersub 0x1d83f358]
     TARG = 4
     FLAGS = (UNKNOWN,SLABBED)
     OP_METH_SV = 0x0
     METH = PV("foo") (0x1d833010)
     RCLASS_TARG = 2
     RCLASS = PV("BAR") (0x1d83f638)

    --------------------------------------------------------------
A minor refactor in preparation for the next commit: make
Perl_dump_sub_perl() invoke S_do_op_dump_bar() directly, rather than
going via op_dump() which indirectly calls the former.

Should make no functional difference.
A couple of commits ago I added a mechanism to display the values of the
SV for ops (such as OP_CONST and OP_GVSV) on threaded builds when
possible, where the SV has been moved into the pad. This commit extends
that mechanism to work when a sub's optree is being dumped via the '-Dx'
perl command-line switch.

That previous commit tried to find the CV (and thus pad) associated with
the op being dumped by rummaging around on the context and parse stacks.
But the -Dx mechanism is neither of those things. It dumps all the subs
it can find in packages after compilation, but before execution.

This commit adds an extra parameter to S_do_op_dump_bar() which
optionally indicates what CV is having its optree dumped. The -Dx
mechanism can use this parameter to pass a hint to the SV-in-pad finding
code. If the parameter is null, it falls back to the mechanisms added
in the previous commits.
The OP_RCATLINE op has a GV attached. So When dumping OPs, display its
value, similarly to what is already done for other GV-holding ops like
OP_GVSV.

Similarly, OP_ANONCODE has a CV attached.
Perl_debop() displays an op in a compact one-line form, typically used
by 'perl -Dt' to show the next op to be executed.

This commit improves the display of some ops slightly: in particular,
where the name of a GV argument to the op, or the name of the associated
lexical var is displayed, sometimes this wasn't being done, for example
for the new op OP_PADSV_STORE, which probably just got missed when being
added.

It also now displays:

* the name of the lexical var for ops which have the OPpTARGET_MY
  optimisation;

* the name of the method and redirect class for method ops;

* the index of the aelemfast and aelemfast_lex op

For example, with the following code:

    my ($sum);
    sub Bar::foo {}
    my $obj = bless {}, 'Foo';
    my @lexary;

    $^D='t';

    $sum = 1;
    $sum = $ary[-2] + $lexary[3];
    $obj->Bar::foo();
    $x .= <>;

then before, the -Dt output for certain lines was:

    padsv_store
    aelemfast
    aelemfast_lex
    add
    method_redir
    rcatline

and is now:

    padsv_store($sum)
    aelemfast(main::ary)[-2]
    aelemfast_lex(@lexary)[3]
    add($sum)
    method_redir(PV("foo"))(PV("Bar"))
    rcatline(main::ARGV)
@iabyn iabyn force-pushed the davem/dump_padop branch from 2a90c22 to d842683 Compare May 24, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-next-dev This PR should not be merged yet, but await the next development cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant