[DTrace-devel] [PATCH] Introduce -xdisasm=n option
Kris Van Hees
kris.van.hees at oracle.com
Wed Apr 8 16:38:48 PDT 2020
You do bring up a valid point that there is some inconsistency in how I did
the implementation vs how I describe it.
New patch will be coming later today.
On Wed, Apr 08, 2020 at 03:47:11PM -0700, Eugene Loh wrote:
> On 04/08/2020 02:57 PM, Kris Van Hees wrote:
>
> > On Wed, Apr 08, 2020 at 02:40:07PM -0700, Eugene Loh wrote:
> >> On 04/08/2020 08:00 AM, Kris Van Hees wrote:
> >>> In order to prepare for more functionality for the disassembler in
> >>> DTrace, an option is being added to be able to specify a bitmap that
> >>> indicates which disassembly listings the user wishes to have printed
> >>> if '-S' is supplied as command line option.
> >>>
> >>> The default is 1 which indicates that the disassembler listing should
> >>> be printed after program compilation.
> >>>
> >>> The comment describing -xdisasm=n documents two additional bits that
> >>> are not yet implemented:
> >>>
> >>> 2 = disassembler listing after the program is linked
> >>> 3 = disassembler listing right before loading the program
> >> Need a consistent way of talking about these things. Since you call it
> >> a bitmap and because that's what the user will specify (in order to be
> >> able to specify multiple bits with a single argument), best (correct?)
> >> to say the choices are 1, 2, and 4. So, change 3 to 4.
> >>
> >> I don't know if calling them 0x1, 0x2, and 0x4 helps readers recognize
> >> these things as masks.
> > This is not documentation for the user - this documents the implementation,
> > and there the argument to the macro is the bit to check for, which would then
> > be 1, 2, or 3.
> >
> > Anyway, I can rework it to mention that it is the 2nd bit and the 3rd bit,
> > and mention that the value to be specified to -xdisasm= is a numeric value
> > with the bits set for the requested listings.
>
> I think the real point, which you make further below, is that you're
> trying to be consistent with TREEDUMP_PASS.
>
> But gratuitous differences between implementation and user-visible
> behaviors just cost productivity in the long run. Might as well have
> one coherent way of talking about this rather than translating back and
> forth between two different ways of talking about these things. Indeed,
> the macro would be simpler if we checked for "&m" rather than for
> "&(1<<((m)-1))". It makes most sense to me to align the implementation
> with what we're trying to expose to the user. And, again, phrases like
> "bit 2" are ambiguous. Is it 0x4?
>
> But I have nothing new to add to the discussion, so just do whatever you
> want. No point in discussing this further.
>
> >> Someday need tests for this option.
> >>
> >>> Since the vlaue supplied to -xdisasm is a bitmap, any combination of
> >> vlaue
> >> ->
> >> value
> >>
> >> More below.
> >>
> >>> disassembler listings can be requested.
> >>>
> >>> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> >>> ---
> >>> libdtrace/dt_cc.c | 30 ++++++++++++++++++------------
> >>> libdtrace/dt_impl.h | 14 ++++++++++++++
> >>> libdtrace/dt_open.c | 1 +
> >>> libdtrace/dt_options.c | 16 +++++++++++++++-
> >>> 4 files changed, 48 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> >>> index ab987fc7..c8c6dd0c 100644
> >>> --- a/libdtrace/dt_cc.c
> >>> +++ b/libdtrace/dt_cc.c
> >>> @@ -2184,12 +2184,15 @@ dtrace_program_strcompile(dtrace_hdl_t *dtp, const char *s,
> >>> {
> >>> dtrace_prog_t *rv;
> >>>
> >>> - if ((rv = dt_compile(dtp, DT_CTX_DPROG,
> >>> - spec, NULL, cflags, argc, argv, NULL, s)) != NULL) {
> >>> - if (cflags & DTRACE_C_DIFV)
> >>> - dt_dis_program(dtp, rv, stderr);
> >>> - }
> >>> - return (rv);
> >>> + rv = dt_compile(dtp, DT_CTX_DPROG, spec, NULL, cflags,
> >>> + argc, argv, NULL, s);
> >>> + if (rv == NULL)
> >>> + return NULL;
> >>> +
> >>> + if (cflags & DTRACE_C_DIFV && DT_DISASM(dtp, 1))
> >>> + dt_dis_program(dtp, rv, stderr);
> >>> +
> >>> + return rv;
> >>> }
> >>>
> >>> dtrace_prog_t *
> >>> @@ -2198,12 +2201,15 @@ dtrace_program_fcompile(dtrace_hdl_t *dtp, FILE *fp,
> >>> {
> >>> dtrace_prog_t *rv;
> >>>
> >>> - if ((rv = dt_compile(dtp, DT_CTX_DPROG,
> >>> - DTRACE_PROBESPEC_NAME, NULL, cflags, argc, argv, fp, NULL)) != NULL) {
> >>> - if (cflags & DTRACE_C_DIFV)
> >>> - dt_dis_program(dtp, rv, stderr);
> >>> - }
> >>> - return (rv);
> >>> + rv = dt_compile(dtp, DT_CTX_DPROG, DTRACE_PROBESPEC_NAME, NULL, cflags,
> >>> + argc, argv, fp, NULL);
> >>> + if (rv == NULL)
> >>> + return NULL;
> >>> +
> >>> + if (cflags & DTRACE_C_DIFV && DT_DISASM(dtp, 1))
> >>> + dt_dis_program(dtp, rv, stderr);
> >>> +
> >>> + return rv;
> >>> }
> >>>
> >>> int
> >>> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> >>> index 536b1745..ca8b7851 100644
> >>> --- a/libdtrace/dt_impl.h
> >>> +++ b/libdtrace/dt_impl.h
> >>> @@ -326,6 +326,7 @@ struct dtrace_hdl {
> >>> uint_t dt_xlatemode; /* dtrace translator linking mode (see below) */
> >>> uint_t dt_stdcmode; /* dtrace stdc compatibility mode (see below) */
> >>> uint_t dt_treedump; /* dtrace tree debug bitmap (see below) */
> >>> + uint_t dt_disasm; /* dtrace disassembler bitmap (see below) */
> >>> uint64_t dt_options[DTRACEOPT_MAX]; /* dtrace run-time options */
> >>> int dt_version; /* library version requested by client */
> >>> int dt_ctferr; /* error resulting from last CTF failure */
> >>> @@ -413,6 +414,19 @@ struct dtrace_hdl {
> >>> */
> >>> #define DT_TREEDUMP_PASS(dtp, p) ((dtp)->dt_treedump & (1 << ((p) - 1)))
> >>>
> >>> +/*
> >>> + * Macro to test whether a given disassembler bit is set in the dt_disasm
> >>> + * bit-vector. If the bit for mode 'm' is set, the D disassembler will be
> >>> + * invoked for that specific mode. The '-S' option must also be supplied in
> >>> + * order for disassembler output to be generated.
> >>> + *
> >>> + * Supported modes are:
> >>> + * 0 Print disassembler listing for each compiled clause.
> >>> + * 1 Print disassembler listing for the final linked program.
> >>> + * 2 Print disassembler listing for the program right before loading.
> >> Instead of 0, 1, and 2, call them 1, 2, and 4 (or even prefix with 0x
> >> per above suggestion if you like).
> > No, because this refers to the implementation which where the macro argument is
> > the bit to test for. This is consistent with the DT_TREEDUMP_PASS(dtp, p)
> > macro.
> >
> >>> + */
> >>> +#define DT_DISASM(dtp, m) ((dtp)->dt_disasm & (1 << ((m) - 1)))
> >>> +
> >> (1 << ((m) - 1)))
> >> becomes simply
> >> (m)
> >>
> >>> /*
> >>> * Macros for accessing the cached CTF container and type ID for the common
> >>> * types "int", "string", and <DYN>, which we need to use frequently in the D
> >>> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> >>> index 6b0ba71d..ab60a803 100644
> >>> --- a/libdtrace/dt_open.c
> >>> +++ b/libdtrace/dt_open.c
> >>> @@ -709,6 +709,7 @@ dt_vopen(int version, int flags, int *errp,
> >>> dtp->dt_linktype = DT_LTYP_ELF;
> >>> dtp->dt_xlatemode = DT_XL_STATIC;
> >>> dtp->dt_stdcmode = DT_STDC_XA;
> >>> + dtp->dt_disasm = 1;
> >>> dtp->dt_version = version;
> >>> dtp->dt_cdefs_fd = -1;
> >>> dtp->dt_ddefs_fd = -1;
> >>> diff --git a/libdtrace/dt_options.c b/libdtrace/dt_options.c
> >>> index a199cfdb..8c6e0e42 100644
> >>> --- a/libdtrace/dt_options.c
> >>> +++ b/libdtrace/dt_options.c
> >>> @@ -397,6 +397,19 @@ dt_opt_module_path(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> >>> return (0);
> >>> }
> >>>
> >>> +/*ARGSUSED*/
> >>> +static int
> >>> +dt_opt_disasm(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> >>> +{
> >>> + int m;
> >>> +
> >>> + if (arg == NULL || (m = atoi(arg)) <= 0)
> >>> + return (dt_set_errno(dtp, EDT_BADOPTVAL));
> >>> +
> >>> + dtp->dt_disasm = m;
> >>> + return (0);
> >>> +}
> >>> +
> >>> /*ARGSUSED*/
> >>> static int
> >>> dt_opt_evaltime(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
> >>> @@ -1032,11 +1045,12 @@ static const dt_option_t _dtrace_ctoptions[] = {
> >>> { "ctypes", dt_opt_ctypes },
> >>> { "ctfpath", dt_opt_ctfa_path },
> >>> { "defaultargs", dt_opt_cflags, DTRACE_C_DEFARG },
> >>> - { "dtypes", dt_opt_dtypes },
> >>> { "debug", dt_opt_debug },
> >>> { "debugassert", dt_opt_debug_assert },
> >>> { "define", dt_opt_cpp_opts, (uintptr_t)"-D" },
> >>> + { "disasm", dt_opt_disasm },
> >>> { "droptags", dt_opt_droptags },
> >>> + { "dtypes", dt_opt_dtypes },
> >>> { "empty", dt_opt_cflags, DTRACE_C_EMPTY },
> >>> { "errtags", dt_opt_cflags, DTRACE_C_ETAGS },
> >>> { "evaltime", dt_opt_evaltime },
> >>
> >> _______________________________________________
> >> DTrace-devel mailing list
> >> DTrace-devel at oss.oracle.com
> >> https://oss.oracle.com/mailman/listinfo/dtrace-devel
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list