Date: Mon, 19 Dec 2016 23:16:47 -0800 From: Mark Johnston <markj@FreeBSD.org> To: Hiroki Sato <hrs@FreeBSD.org> Cc: freebsd-dtrace@freebsd.org Subject: Re: clause-local variable with copyin() Message-ID: <20161220071647.GB82223@wkstn-mjohnston.west.isilon.com> In-Reply-To: <20161220.030224.323335605995825210.hrs@allbsd.org> References: <20161217.151014.1579687141761225852.hrs@allbsd.org> <20161219030125.GB57753@wkstn-mjohnston.west.isilon.com> <20161220.030224.323335605995825210.hrs@allbsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Dec 20, 2016 at 03:02:24AM +0900, Hiroki Sato wrote:
> Mark Johnston <markj@FreeBSD.org> wrote
> in <20161219030125.GB57753@wkstn-mjohnston.west.isilon.com>:
>
> ma> On Sat, Dec 17, 2016 at 03:10:14PM +0900, Hiroki Sato wrote:
> ma> > Do I misunderstand clause-local variable? I noticed this when I use
> ma> > if-then clause which was recently implemented as a syntax sugar to
> ma> > split a probe automatically. The following ended up with the same
> ma> > result:
> ma>
> ma> I think this is more a quirk of copyin() than of clause-local variables.
> ma> In particular:
> ma> - your example works as expected if copyinstr() is used instead of
> ma> copyin(), and
> ma> - your example works if one assigns this->st = stringof(copyin(...)).
I meant to also note here that the example fails if a thread-local
variable is used instead.
> ma>
> ma> copyin() and copyinstr() both copy data into a scratch buffer. However,
> ma> copyinstr() returns a pass-by-reference string, while copyin() returns a
> ma> pass-by-value pointer. The DIF instruction which saves to a clause-local
> ma> variable, STLS, performs a deep copy of pass-by-reference variables to
> ma> some dedicated storage. The scratch space containing the
> ma> copyin()/copyinstr() is not preserved between enablings of the same
> ma> probe, so the string copied during the first probe is not available in
> ma> the second probe when copyin() is used.
>
> The difference of the scratch space when using copyin() and
> copyinstr() were the following ("-" is copyin() and "+" is
> copyinstr()):
>
> NAME ID KND SCP FLAG TYPE
> arg0 106 scl glb r D type (integer) (size 8)
> -st 500 scl loc w D type (pointer) (size 8)
> +st 500 scl loc w string (unknown) by ref (size 256)
>
> As you explained copyinstr() had DIF_TF_BYREF and DIF_OP_STLS
> performed dtrace_vcopy(). However, I still do not understand the
> difference of the behavior across the boundary of two clauses for a
> single probe. Is it correct that the cause is that the contents of
> the scratch space which came from copyin() or copyinstr() are not
> preserved across multiple clauses of a single probe?
Indeed. These multiple clauses are considered to be distinct enablings
of the probe, and thus get their own scratch space - see the
dtrace_buffer_reserve() call near the beginning of the enabling loop in
dtrace_probe(). Each enabling reserves space in the tracing buffer, but
it seems that there's no mechanism to reserve space for the result of a
copyin(). dtrace_ecb_action_add() computes the amount of reserved space
required for each enabling of a probe and includes space for the buffer
created by printf(), for example.
So what happens is that STLS saves a pointer into scratch space that
gets overwritten by the record header for the second enabling. I'm not
claiming that this isn't a bug - perhaps copyin calls should cause space
to be reserved.
> If it is true, I am still wondering why copyinstr() works. I think
> DIF_OP_LDLS in the second probe to load this->st always fails if the
> scratch space is not preserved regardless of whether the data type
> involves dereference or not.
See the handling for pass-by-reference types in the STLS and LDLS
implementations. STLS saves the entire string to the vstate struct in
the enabling state. But variable state is shared between the enablings,
so the LDLS instruction from the second probe is able to access the
string.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161220071647.GB82223>