[DTrace-devel] [PATCH] ERROR probe implementation

[Kris Van Hees]

On Thu, Jan 28, 2021 at 12:24:24AM -0500, Eugene Loh wrote:
> In addition to previous comments, there are apparently also testing 
> issues with this patch.
> 
> This test was introduced and will in general fail because the .r file 
> hardcodes a tick-n probe ID.  The test cannot rely on a tick-n probe 
> having a specific, numerical value.
>              test/unittest/error/tst.clause_scope-regular.d

Yes, and we shouldn't be relying on any probe having a specific ID.

> These tests started to FAIL.  They need XFAIL annotations.  (The problem 
> is not so much the patch;  these tests "should not" have been passing in 
> the first place.)  Also, if one is going to touch 
> builtinvar/tst.caller1.d anyhow, fix its s/form/from/ typo.
>              test/unittest/builtinvar/tst.caller.d
>              test/unittest/builtinvar/tst.caller1.d
>              test/unittest/builtinvar/tst.ipl.d
>              test/unittest/builtinvar/tst.ipl1.d
>              test/unittest/builtinvar/tst.vtimestamp.d
>              test/unittest/printa/tst.walltimestamp.sh
>              test/demo/builtin/caller.d
>              test/demo/builtin/errno.d
>              test/demo/builtin/ipl.d
>              test/demo/builtin/ucaller.d
>              test/demo/builtin/ustackdepth.d
>              test/demo/builtin/vtimestamp.d
>              test/demo/builtin/walltimestamp.d

Yes, this is a fortunate side effect of finally being able to flag a fault
for unimplemented builtin variables.

> These tests started to XPASS.  I am not familiar with the tests, but I 
> assume their XFAIL annotations can be removed.
>              test/unittest/pragma/err.circlibdep.sh
>              test/unittest/pragma/err.invalidlibdep.sh
>              test/unittest/pragma/tst.libdepfullyconnected.sh

I need to look a bit more indeed these because I am not entirely clear on
whether they reflect spurious passes or valid passes.

> These tests started to XPASS spuriously -- that is, they actually "fail".
>              test/unittest/variables/bvar/tst.caller.d
>              test/unittest/variables/bvar/tst.errno.d
>              test/unittest/variables/bvar/tst.ipl.d
>              test/unittest/variables/bvar/tst.ucaller.d
>              test/unittest/variables/bvar/tst.ustackdepth.d
>              test/unittest/variables/bvar/tst.vtimestamp.d
>              test/unittest/variables/bvar/tst.walltimestamp.d
> I do not yet understand what is happening here.  Note that "caller" is 
> not yet implemented.  Consider:
>       # dtrace -n 'BEGIN { trace(caller); }'
>       dtrace: error on enabled probe ID 2 (ID 1: dtrace:::BEGIN): 
> illegal operation in action #1
>       dtrace: processing aborted: Invalid library ERROR action
>       # echo $?
>       1
>       # dtrace -n 'BEGIN { trace(caller); exit(0) }'
>       dtrace: error on enabled probe ID 2 (ID 1: dtrace:::BEGIN): 
> illegal operation in action #1
>       dtrace: processing aborted: Invalid library ERROR action
>       # echo $?
>       1
>       # dtrace -n 'BEGIN { trace(caller); exit(1) }'
>       dtrace: error on enabled probe ID 2 (ID 1: dtrace:::BEGIN): 
> illegal operation in action #1
>       dtrace: processing aborted: Invalid library ERROR action
>       # echo $?
>       1
>       # dtrace -n 'BEGIN { trace(caller); exit(caller != -1 ? 0 : 1); }'
>       dtrace: error on enabled probe ID 2 (ID 1: dtrace:::BEGIN): 
> illegal operation in action #1
>       # echo $?
>       0
> The return value looks wrong.  If you like, I can look into this some, 
> but I assume this reflects a bug.

This is almost certainly a side effect of the consume bug that I mentioned
earlier - where BEGIN, ERROR (and END) are all fired before we even start
processing data buffers.

If you want to take a look at that consume code (dt_consume_begin and related
functions), that would be quite useful.  Especially for test cases, this is a
situation tht pops up quite often.