Date: Mon, 16 Sep 2013 13:19:57 -0500 From: Nico Williams <nico@cryptonector.com> To: Mark Johnston <markj@freebsd.org> Cc: dtrace-discuss@lists.dtrace.org, freebsd-dtrace@freebsd.org Subject: Re: [dtrace-discuss] pr_psargs on FreeBSD Message-ID: <CAK3OfOg_0paqdQpAjt4u0ZPfaV8eRQ5pWNf-9TADGT6g8PPNXQ@mail.gmail.com> In-Reply-To: <20130915221622.GA2981@raichu> References: <20130915221622.GA2981@raichu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Sep 15, 2013 at 5:16 PM, Mark Johnston <markj@freebsd.org> wrote: > Hi! > > One of the problems with FreeBSD's DTrace implementation is that it > doesn't have a good way of getting the arguments of a given process. > This is because of the way that they're stored, which is as a single > buffer with null-terminator between consecutive arguments. In > particular, there is no way within DTrace to convert such a buffer to a > single string (say by replacing all but the last terminator with > spaces). So scripts like execsnoop will only print the first element of > a process' argv on FreeBSD, which isn't particularly helpful for > interpreted programs. > > You can get a fixed number of arguments with something like > > printf("%s %s %s", cupsinfo->pr_psargs, > curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1, > curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1 + > strlen(curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1) + 1); > > but this is less than ideal. :) > > Apparently OS X has the same problem, but I don't have a machine to test > with, so I'm not completely sure. > > It seems to me that there are two ways to solve this problem: change the > format that FreeBSD uses to store process arguments, or add a function > to DTrace which can convert the argument buffer to a string. I'm not too > keen on the former approach, so here's a proposal for the latter. It'd > be great to get some feedback on it and find out whether anyone thinks > it's a terrible idea. :) > > It's kind of a strong-armed approach, but the problem's been around for a > while and I can't think of any clever solutions - I'd love to see an > alternative solution. > > My patch against FreeBSD (pasted below) adds a function called memstr() > to DTrace. memstr() takes three arguments: addr, c, and len, and returns > a copy of addr of length len with all null-terminators replaced by c, > and with the last byte replaced by a null-terminator. In particular, > memstr() always returns a string of length len - 1, unless len == 0, in > which case we return the empty string. > > With this function, the translator for the pr_psargs fields becomes > > translator psinfo_t < struct proc *T > { > ... > pr_psargs = memstr(T->p_args->ar_args, ' ', T->p_args->ar_length); > ... > }; > > and execsnoop works properly. Any thoughts on this function? Have I missed > a better solution? A patch for FreeBSD is below. > > Thanks, > -Mark > > diff --git a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > index 9c9b2a6..83c81e5 100644 > --- a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > +++ b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > @@ -122,8 +122,9 @@ > #define DT_VERS_1_8_1 DT_VERSION_NUMBER(1, 8, 1) > #define DT_VERS_1_9 DT_VERSION_NUMBER(1, 9, 0) > #define DT_VERS_1_9_1 DT_VERSION_NUMBER(1, 9, 1) > -#define DT_VERS_LATEST DT_VERS_1_9_1 > -#define DT_VERS_STRING "Sun D 1.9.1" > +#define DT_VERS_1_9_2 DT_VERSION_NUMBER(1, 9, 2) > +#define DT_VERS_LATEST DT_VERS_1_9_2 > +#define DT_VERS_STRING "Sun D 1.9.2" > > const dt_version_t _dtrace_versions[] = { > DT_VERS_1_0, /* D API 1.0.0 (PSARC 2001/466) Solaris 10 FCS */ > @@ -145,6 +146,7 @@ const dt_version_t _dtrace_versions[] = { > DT_VERS_1_8_1, /* D API 1.8.1 */ > DT_VERS_1_9, /* D API 1.9 */ > DT_VERS_1_9_1, /* D API 1.9.1 */ > + DT_VERS_1_9_2, /* D API 1.9.2 */ > 0 > }; > > @@ -311,6 +313,8 @@ static const dt_ident_t _dtrace_globals[] = { > &dt_idops_func, "void(@)" }, > { "memref", DT_IDENT_FUNC, 0, DIF_SUBR_MEMREF, DT_ATTR_STABCMN, DT_VERS_1_1, > &dt_idops_func, "uintptr_t *(void *, size_t)" }, > +{ "memstr", DT_IDENT_FUNC, 0, DIF_SUBR_MEMSTR, DT_ATTR_STABCMN, DT_VERS_1_0, > + &dt_idops_func, "string(void *, char, size_t)" }, > { "min", DT_IDENT_AGGFUNC, 0, DTRACEAGG_MIN, DT_ATTR_STABCMN, DT_VERS_1_0, > &dt_idops_func, "void(@)" }, > { "mod", DT_IDENT_ACTFUNC, 0, DT_ACT_MOD, DT_ATTR_STABCMN, > diff --git a/cddl/lib/libdtrace/psinfo.d b/cddl/lib/libdtrace/psinfo.d > index 068e72e..b2a009a 100644 > --- a/cddl/lib/libdtrace/psinfo.d > +++ b/cddl/lib/libdtrace/psinfo.d > @@ -57,7 +57,7 @@ translator psinfo_t < struct proc *T > { > pr_gid = T->p_ucred->cr_rgid; > pr_egid = T->p_ucred->cr_groups[0]; > pr_addr = 0; > - pr_psargs = stringof(T->p_args->ar_args); > + pr_psargs = memstr(T->p_args->ar_args, ' ', T->p_args->ar_length); > pr_arglen = T->p_args->ar_length; > pr_jailid = T->p_ucred->cr_prison->pr_id; > }; > diff --git a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > index babc42c4..6d991fb 100644 > --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > @@ -4920,6 +4920,38 @@ inetout: regs[rd] = (uintptr_t)end + 1; > break; > } > > + case DIF_SUBR_MEMSTR: { > + char *str = (char *)mstate->dtms_scratch_ptr; > + uintptr_t mem = tupregs[0].dttk_value; > + char c = tupregs[1].dttk_value; > + size_t size = tupregs[2].dttk_value; > + uint8_t n; > + int i; > + > + regs[rd] = 0; > + > + if (size == 0) > + break; > + > + if (!dtrace_canload(mem, size - 1, mstate, vstate)) > + break; > + > + if (!DTRACE_INSCRATCH(mstate, size)) { > + DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH); > + break; > + } > + > + for (i = 0; i < size - 1; i++) { > + n = dtrace_load8(mem++); > + str[i] = (n == 0) ? c : n; > + } > + str[size - 1] = 0; > + > + regs[rd] = (uintptr_t)str; > + mstate->dtms_scratch_ptr += size; > + break; > + } > + > case DIF_SUBR_TYPEREF: { > uintptr_t size = 4 * sizeof(uintptr_t); > uintptr_t *typeref = (uintptr_t *) P2ROUNDUP(mstate->dtms_scratch_ptr, sizeof(uintptr_t)); > @@ -9102,6 +9134,7 @@ dtrace_difo_validate_helper(dtrace_difo_t *dp) > subr == DIF_SUBR_NTOHL || > subr == DIF_SUBR_NTOHLL || > subr == DIF_SUBR_MEMREF || > + subr == DIF_SUBR_MEMSTR || > subr == DIF_SUBR_TYPEREF) > break; > > diff --git a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > index 8728e30..295457c 100644 > --- a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > +++ b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > @@ -311,8 +311,9 @@ typedef enum dtrace_probespec { > #define DIF_SUBR_SX_SHARED_HELD 48 > #define DIF_SUBR_SX_EXCLUSIVE_HELD 49 > #define DIF_SUBR_SX_ISEXCLUSIVE 50 > +#define DIF_SUBR_MEMSTR 51 > > -#define DIF_SUBR_MAX 50 /* max subroutine value */ > +#define DIF_SUBR_MAX 51 /* max subroutine value */ > > typedef uint32_t dif_instr_t; > > > > ------------------------------------------- > dtrace-discuss > Archives: https://www.listbox.com/member/archive/184261/=now > RSS Feed: https://www.listbox.com/member/archive/rss/184261/21484527-a123725d > Modify Your Subscription: https://www.listbox.com/member/?member_id=21484527&id_secret=21484527-085b137e > Powered by Listbox: http://www.listbox.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAK3OfOg_0paqdQpAjt4u0ZPfaV8eRQ5pWNf-9TADGT6g8PPNXQ>