Page MenuHomeFreeBSD
Differential D38732

dtrace(1): add -d flag to dump D script post-dt_sugar
ClosedPublic
Actions

Authored by christos on Feb 22 2023, 4:42 PM.
Tags
Referenced Files
Unknown Object (File)
Sun, Apr 14, 5:29 PM
Unknown Object (File)
Mon, Apr 8, 1:58 PM
Unknown Object (File)
Mar 4 2024, 1:13 AM
Unknown Object (File)
Mar 4 2024, 1:13 AM
Unknown Object (File)
Mar 4 2024, 1:13 AM
Unknown Object (File)
Mar 4 2024, 1:13 AM
Unknown Object (File)
Mar 4 2024, 1:13 AM
Unknown Object (File)
Mar 4 2024, 1:13 AM
View All 63 Files
Subscribers
0mp
imp

Details

Reviewers
markj
gnn
Commits
rG1e136a9cbd3a: dtrace(1): add -d flag to dump D script post-dt_sugar
Summary

By specifying the -d flag, libdtrace will dump the D script after it has applied
syntactical sugar transformations (e.g if/else). This is useful for both understanding
what dt_sugar does, as well as debugging it.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

christos created this revision.Feb 22 2023, 4:42 PM
Herald added a reviewer: gnn. · View Herald TranscriptFeb 22 2023, 4:42 PM
Herald added a subscriber: imp. · View Herald Transcript
christos requested review of this revision.Feb 22 2023, 4:42 PM
markj added inline comments.Feb 22 2023, 4:51 PM
cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
199

Probably clarify that it dumps to stdout.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_sugar.c
521 ↗(On Diff #117771)

Do we actually need to do this here? Why not have the compiler dump the output script after calling dt_compile_sugar()? That seems somewhat simpler.

christos updated this revision to Diff 117819.Feb 23 2023, 4:56 PM
christos marked an inline comment as done.

Move dump code to dt_cc.c.

christos edited the summary of this revision. (Show Details)Feb 23 2023, 5:25 PM
0mp added a subscriber: 0mp.Feb 23 2023, 5:55 PM

Would it be wiser to implement this as an extension to the -x flag?

markj accepted this revision.Feb 23 2023, 5:56 PM
markj added inline comments.
cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
199
cddl/contrib/opensolaris/cmd/dtrace/dtrace.c
155
This revision is now accepted and ready to land.Feb 23 2023, 5:56 PM
markj added a comment.Feb 23 2023, 5:59 PM
In D38732#881808, @0mp wrote:

Would it be wiser to implement this as an extension to the -x flag?

This option is in the same vein as -S, whereas -x is used to set runtime parameters for the dtrace script. So, -x feels like the wrong place to me, but maybe I'm missing something?

0mp added a comment.Feb 23 2023, 6:58 PM
In D38732#881813, @markj wrote:
In D38732#881808, @0mp wrote:

Would it be wiser to implement this as an extension to the -x flag?

This option is in the same vein as -S, whereas -x is used to set runtime parameters for the dtrace script. So, -x feels like the wrong place to me, but maybe I'm missing something?

I don't think you are missing anything :) I just wonder if there is a way to avoid taking up a whole flag for a debug feature.

OTOH, I checked macOS DTrace implementation and they are not using -d for anything, so it seems it is fine if we dedicate -d to dumping D scripts post dt_sugar.

christos updated this revision to Diff 117849.Feb 24 2023, 12:03 AM
christos marked an inline comment as done.

s/stdout/standard output/

This revision now requires review to proceed.Feb 24 2023, 12:03 AM
christos updated this revision to Diff 117850.Feb 24 2023, 2:14 AM

Print dnp as well.

christos updated this revision to Diff 117874.Feb 24 2023, 1:51 PM
christos marked 2 inline comments as done.

Typo fixes.

markj accepted this revision.Feb 24 2023, 4:10 PM
This revision is now accepted and ready to land.Feb 24 2023, 4:10 PM
christos updated this revision to Diff 118326.Mar 4 2023, 1:32 PM

remove if/else comment in dt_cc as dt_sugar now also performs transformations
on kinst probes.

This revision now requires review to proceed.Mar 4 2023, 1:32 PM
christos added a comment.Mar 18 2023, 9:38 PM

I don't think there are any pending changes for this PR, it can get through.

markj added a comment.May 23 2023, 2:04 PM
In D38732#891505, @christos wrote:

I don't think there are any pending changes for this PR, it can get through.

Sorry for the delay. One last comment, and then I think it's ok.

cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
200

It would be helpful to expand on this a bit: mention the example of if/else statements being implemented using syntactic transformations.

christos added inline comments.May 23 2023, 2:06 PM
cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
200

Will "...have been applied, such as if/else statements" suffice?

markj added inline comments.May 23 2023, 2:09 PM
cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
200

I'd suggest: "... have been applied. For example, if-statements in D are implemented using such transformations: a conditional clause in a probe body is replaced at compile-time by a separate probe predicated on the original condition."

christos marked 2 inline comments as done.May 23 2023, 2:10 PM
christos added inline comments.
cddl/contrib/opensolaris/cmd/dtrace/dtrace.1
200

That's way better indeed. Thanks.

This revision was not accepted when it landed; it landed in state Needs Review.May 23 2023, 2:20 PM
Closed by commit rG1e136a9cbd3a: dtrace(1): add -d flag to dump D script post-dt_sugar (authored by christos). · Explain Why
This revision was automatically updated to reflect the committed changes.
christos marked an inline comment as done.
christos added a commit: rG1e136a9cbd3a: dtrace(1): add -d flag to dump D script post-dt_sugar.

Revision Contents
Changeset List

PathSize
cddl/
contrib/
opensolaris/
cmd/
dtrace/
7 lines
9 lines
lib/
libdtrace/
common/
10 lines
3 lines

Diff 117849

View Options

cddl/contrib/opensolaris/cmd/dtrace/dtrace.1

Loading...
View Options

cddl/contrib/opensolaris/cmd/dtrace/dtrace.c

Loading...
View Options

cddl/contrib/opensolaris/lib/libdtrace/common/dt_cc.c

Loading...
View Options

cddl/contrib/opensolaris/lib/libdtrace/common/dtrace.h

Loading...