[DTrace-devel] [PATCH] Introduce -xdisasm=n option
Eugene Loh
eugene.loh at oracle.com
Wed Apr 8 15:47:11 PDT 2020
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
More information about the DTrace-devel
mailing list