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

[Eugene Loh]

On 4/26/22 5:56 PM, Kris Van Hees wrote:

> 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.
Aha.  I had to read the documentation further to see where arg0 was 
discussed.
>> 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.
I'm working on a patch (and tests).
>
>> 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.
https://docs.oracle.com/en/operating-systems/oracle-linux/dtrace-guide/dt_prov.html#dt_fbtargs_prov 
docs just arg0 and arg1.
>>>> 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