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

Make add_remove_uprobe test case more robust against various real
binary name.
Current add_remove_uprobe.tc test expects the real binary of /bin/sh
is '*/bin/*sh', but it does not work on busybox environment.
Instead of using fixed pattern, use readlink to identify real binary
name.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
index a275decdc880..86c76679c56e 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
@@ -6,8 +6,10 @@
 echo 0 > events/enable
 echo > dynamic_events
 
+REALBIN=`readlink -f /bin/sh`
+
 echo 'cat /proc/$$/maps' | /bin/sh | \
-	grep "r-xp .*/bin/.*sh$" | \
+	grep "r-xp .*${REALBIN}$" | \
 	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events
 
 grep -q myevent uprobe_events

On Tue, Jan 07, 2025 at 09:11:16PM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Make add_remove_uprobe test case more robust against various real
> binary name.
> Current add_remove_uprobe.tc test expects the real binary of /bin/sh
> is '*/bin/*sh', but it does not work on busybox environment.
> Instead of using fixed pattern, use readlink to identify real binary
> name.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> index a275decdc880..86c76679c56e 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> @@ -6,8 +6,10 @@
>  echo 0 > events/enable
>  echo > dynamic_events
>  
> +REALBIN=`readlink -f /bin/sh`
> +
>  echo 'cat /proc/$$/maps' | /bin/sh | \
> -	grep "r-xp .*/bin/.*sh$" | \
> +	grep "r-xp .*${REALBIN}$" | \
>  	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events

This test fails on s390, not because of this change, but since the
test seems to assume that the entry point of /bin/sh is at offset 0;
or at least there is some code that is executed at least once at
offset 0; but that is not the case (at least) for s390.

Not sure how this can be addressed. Using "readelf -f" to get the
entry point address of the executable and use that for the uprobe
would work, but it would also add a dependency to binutils for the
ftrace selftests, which doesn't exist yet, as far as I can tell.

Would it maybe an option for architectures to opt-in to use readelf
and skip the test (exit_unsupported) if readelf is not available?

On Mon, 17 Feb 2025 14:42:44 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> > +REALBIN=`readlink -f /bin/sh`
> > +
> >  echo 'cat /proc/$$/maps' | /bin/sh | \
> > -	grep "r-xp .*/bin/.*sh$" | \
> > +	grep "r-xp .*${REALBIN}$" | \
> >  	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events  
> 
> This test fails on s390, not because of this change, but since the
> test seems to assume that the entry point of /bin/sh is at offset 0;
> or at least there is some code that is executed at least once at
> offset 0; but that is not the case (at least) for s390.
> 
> Not sure how this can be addressed. Using "readelf -f" to get the
> entry point address of the executable and use that for the uprobe
> would work, but it would also add a dependency to binutils for the
> ftrace selftests, which doesn't exist yet, as far as I can tell.
> 
> Would it maybe an option for architectures to opt-in to use readelf
> and skip the test (exit_unsupported) if readelf is not available?

I'm OK for adding a dependency on readelf existing on the test system, as
it is part of binutils.

Masami and Shuah,

Are you OK with that?

-- Steve