-
Notifications
You must be signed in to change notification settings - Fork 580
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
iabyn
wants to merge
7
commits into
blead
Choose a base branch
from
davem/dump_padop
base: blead
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.