[DTrace-devel] [PATCH] test: Clean up fbtprovider/tst.return0.d

[Kris Van Hees]

On Tue, Apr 26, 2022 at 04:11:56PM -0400, Eugene Loh via DTrace-devel wrote:
> On 4/22/22 2:53 AM, Kris Van Hees wrote:
> 
> > On Thu, Apr 21, 2022 at 09:23:56PM -0400, eugene.loh--- via DTrace-devel wrote:
> > > From: Eugene Loh <eugene.loh at oracle.com>
> > > 
> > > Remove the vestigial -Z since the remaining probe is actually needed
> > > for the test not to time out.  Change references to "arg0" (which is
> > > not defined for fbt:::return probes) to "arg1".
> > Removing -Z is correct.
> Okay.
> > CHanging from arg0 to arg1 is not correct.  The test verifies that the return
> > value (in arg1) is 0 by means of the predicate.  But it prints out arg0 (and
> > also refers to that in the test assertion.
> > 
> > Now, the arg0 value is actually wrong but that is a different issue.
> I do not understand why we would examine arg0 or how we could know arg0 is
> wrong:  it seems to me that arg0 is undefined.

arg0 is to be the address of the return that triggered the probe.

> I also do not see why the test doesn't just time out.  Or if we're checking
> arg1, whether we should only be examining the lower 32 bits.
> 
> Importantly, it seems to me that arg1 (the fbt:::return value) is wrong. 
> The FBT trampoline seems to get probe args from the registers in a way that
> makes sense for fbt:::entry but not for fbt:::return.  Specifically, we need
> to get arg1 from a particular register and the other args don't need to be
> retrieved at all (or set to 0?).

Well, we need to get the values from the information provided to us by the
underlying kretprobe.  It is possible that some changes were made there in
more recent kernels that require us to adapt.  I can take a look.

> Our tests for fbt:::return values seem to be
>                 test/stress/misc/tst.roch.d
>                 test/unittest/fbtprovider/tst.functionreturnvalue.d
>                 test/unittest/fbtprovider/tst.ioctlargs.d
>                 test/unittest/fbtprovider/tst.offset.d
>                 test/unittest/fbtprovider/tst.offsetzero.d
> None actually checks the fbt return value.
> 
> I'll try to clean this stuff up.

Make sure to check the documentation concerning what the arguments are that
the return probe provides, and if the tests are not testing those correctly,
let's fix the tests and then we can see what we need to fix on the side of
the implementation.

> > > Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> > > ---
> > >   test/unittest/fbtprovider/tst.return0.d | 8 +++-----
> > >   1 file changed, 3 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/test/unittest/fbtprovider/tst.return0.d b/test/unittest/fbtprovider/tst.return0.d
> > > index 3d58e84f..e37a1632 100644
> > > --- a/test/unittest/fbtprovider/tst.return0.d
> > > +++ b/test/unittest/fbtprovider/tst.return0.d
> > > @@ -1,24 +1,22 @@
> > >   /*
> > >    * Oracle Linux DTrace.
> > > - * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
> > > + * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> > >    * Licensed under the Universal Permissive License v 1.0 as shown at
> > >    * http://oss.oracle.com/licenses/upl.
> > >    */
> > >   /*
> > > - * ASSERTION: simple fbt provider arg0 and probfunc print test.
> > > + * ASSERTION: simple fbt provider arg1 and probfunc print test.
> > >    *
> > >    * SECTION: FBT Provider/Probe arguments
> > >    */
> > > -/* @@runtest-opts: -Z */
> > > -
> > >   #pragma D option quiet
> > >   #pragma D option statusrate=10ms
> > >   fbt::do_sys_poll:return
> > >   /arg1 == 0/
> > >   {
> > > -	printf("%s %x returned 0", probefunc, arg0);
> > > +	printf("%s %x returned 0", probefunc, arg1);
> > >   	exit(0);
> > >   }
> > > -- 
> > > 2.18.4
> > > 
> > > 
> > > _______________________________________________
> > > 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