Re: [PATCH v13 07/12] selftests: tracing: Add a basic testcase for wprobe

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Sat, 22 Aug 2026 09:36:38 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Unreachable error handling logic due to early exit under 'set -e'.
> --
> 
> --- Patch [7]: [PATCH v13 07/12] selftests: tracing: Add a basic testcase for wprobe ---
> commit 6d72a05750c5dfc08de6b03a6fc65e5144b20b9f
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> selftests: tracing: Add a basic testcase for wprobe
> 
> Add an add_remove_wprobe.tc testcase for testing wprobe events that
> tests adding and removing operations.
> 
> > diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
> > new file mode 100644
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +# description: Generic dynamic event - add/remove wprobe events
> > +# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README
> > +
> > +echo 0 > events/enable
> > +echo > dynamic_events
> > +
> > +# Use jiffies as a variable that is frequently written to.
> > +TARGET=jiffies
> > +
> > +echo "w:my_wprobe w@$TARGET" >> dynamic_events
> > +
> > +if ! grep -q my_wprobe dynamic_events; then
> > +    echo "Failed to create wprobe event"
> > +    exit_fail
> > +fi
> 
> [Severity: Medium]
> Since the ftracetest framework runs these scripts with set -e (errexit) 
> enabled, will this script exit immediately if the kernel rejects the event 
> creation and the echo command fails?
> 
> If so, the subsequent diagnostic grep check and exit_fail call wouldn't be
> reached, leading to a silent failure without the intended error message.

This is expected, the later grep checks whether if creation command succeeded
but the event is not shown in the dynamic_events. Different check.

> > +
> > +if [ ! -d events/wprobes/my_wprobe ]; then
> > +    echo "Failed to create wprobe event directory"
> > +    exit_fail
> > +fi
> > +
> > +echo 1 > events/wprobes/my_wprobe/enable
> > +
> > +# Check if the event is enabled
> > +if ! grep -q 1 events/wprobes/my_wprobe/enable; then
> > +    echo "Failed to enable wprobe event"
> > +    exit_fail
> > +fi
> 
> [Severity: Medium]
> Similarly, if enabling the wprobe fails at the sysfs boundary, would this
> echo cause an early exit and bypass the diagnostic error message here as well?

Also, this is a kind of double check. Even if writing 1 to enable
succeeded, it is failed to set internal flag. This checks such problem.

> 
> [ ... ]
> 
> > +echo "-:my_wprobe" >> dynamic_events
> > +
> > +if grep -q my_wprobe dynamic_events; then
> > +    echo "Failed to remove wprobe event"
> > +    exit_fail
> > +fi
> 
> [Severity: Medium]
> Could this echo also trigger an early exit if removing the wprobe fails,
> bypassing the failure message?

Ditto.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>