| Message ID | 20241129202621.721159-1-hbathini@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] selftests/ftrace: adjust offset for kprobe syntax error test | expand |
On Sat, 30 Nov 2024 01:56:21 +0530 Hari Bathini <hbathini@linux.ibm.com> wrote: > In 'NOFENTRY_ARGS' test case for syntax check, any offset X of > `vfs_read+X` except function entry offset (0) fits the criterion, > even if that offset is not at instruction boundary, as the parser > comes before probing. But with "ENDBR64" instruction on x86, offset > 4 is treated as function entry. So, X can't be 4 as well. Thus, 8 > was used as offset for the test case. On 64-bit powerpc though, any > offset <= 16 can be considered function entry depending on build > configuration (see arch_kprobe_on_func_entry() for implementation > details). So, use `vfs_read+20` to accommodate that scenario too. > > Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> Shuah, Can you take this through your tree? Thanks, -- Steve > --- > > Changes in v2: > * Use 20 as offset for all arches. > > > .../selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > index a16c6a6f6055..8f1c58f0c239 100644 > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc > @@ -111,7 +111,7 @@ check_error 'p vfs_read $arg* ^$arg*' # DOUBLE_ARGS > if !grep -q 'kernel return probes support:' README; then > check_error 'r vfs_read ^$arg*' # NOFENTRY_ARGS > fi > -check_error 'p vfs_read+8 ^$arg*' # NOFENTRY_ARGS > +check_error 'p vfs_read+20 ^$arg*' # NOFENTRY_ARGS > check_error 'p vfs_read ^hoge' # NO_BTFARG > check_error 'p kfree ^$arg10' # NO_BTFARG (exceed the number of parameters) > check_error 'r kfree ^$retval' # NO_RETVAL
On 12/2/24 12:41, Steven Rostedt wrote: > On Sat, 30 Nov 2024 01:56:21 +0530 > Hari Bathini <hbathini@linux.ibm.com> wrote: > >> In 'NOFENTRY_ARGS' test case for syntax check, any offset X of >> `vfs_read+X` except function entry offset (0) fits the criterion, >> even if that offset is not at instruction boundary, as the parser >> comes before probing. But with "ENDBR64" instruction on x86, offset >> 4 is treated as function entry. So, X can't be 4 as well. Thus, 8 >> was used as offset for the test case. On 64-bit powerpc though, any >> offset <= 16 can be considered function entry depending on build >> configuration (see arch_kprobe_on_func_entry() for implementation >> details). So, use `vfs_read+20` to accommodate that scenario too. >> >> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> >> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > Shuah, > > Can you take this through your tree? Yes I can take it. I do have question about whether this is a fix - sounds like it is from the change log. Clearly stating that it is a fix will help so it can be picked up for stables. thanks, -- Shuah
On Tue, 3 Dec 2024 18:01:06 -0700 Shuah Khan <skhan@linuxfoundation.org> wrote: > On 12/2/24 12:41, Steven Rostedt wrote: > > On Sat, 30 Nov 2024 01:56:21 +0530 > > Hari Bathini <hbathini@linux.ibm.com> wrote: > > > >> In 'NOFENTRY_ARGS' test case for syntax check, any offset X of > >> `vfs_read+X` except function entry offset (0) fits the criterion, > >> even if that offset is not at instruction boundary, as the parser > >> comes before probing. But with "ENDBR64" instruction on x86, offset > >> 4 is treated as function entry. So, X can't be 4 as well. Thus, 8 > >> was used as offset for the test case. On 64-bit powerpc though, any > >> offset <= 16 can be considered function entry depending on build > >> configuration (see arch_kprobe_on_func_entry() for implementation > >> details). So, use `vfs_read+20` to accommodate that scenario too. > >> > >> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> > >> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> > > > > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > > > Shuah, > > > > Can you take this through your tree? > > Yes I can take it. I do have question about whether this is > a fix - sounds like it is from the change log. > > Clearly stating that it is a fix will help so it can be picked > up for stables. I would say it's a fix, as the test currently fails in certain scenarios for powerpc. You can add: Fixes: 4231f30fcc34a ("selftests/ftrace: Add BTF arguments test cases") -- Steve
On 12/3/24 18:20, Steven Rostedt wrote: > On Tue, 3 Dec 2024 18:01:06 -0700 > Shuah Khan <skhan@linuxfoundation.org> wrote: > >> On 12/2/24 12:41, Steven Rostedt wrote: >>> On Sat, 30 Nov 2024 01:56:21 +0530 >>> Hari Bathini <hbathini@linux.ibm.com> wrote: >>> >>>> In 'NOFENTRY_ARGS' test case for syntax check, any offset X of >>>> `vfs_read+X` except function entry offset (0) fits the criterion, >>>> even if that offset is not at instruction boundary, as the parser >>>> comes before probing. But with "ENDBR64" instruction on x86, offset >>>> 4 is treated as function entry. So, X can't be 4 as well. Thus, 8 >>>> was used as offset for the test case. On 64-bit powerpc though, any >>>> offset <= 16 can be considered function entry depending on build >>>> configuration (see arch_kprobe_on_func_entry() for implementation >>>> details). So, use `vfs_read+20` to accommodate that scenario too. >>>> >>>> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> >>>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> >>> >>> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> >>> >>> Shuah, >>> >>> Can you take this through your tree? >> >> Yes I can take it. I do have question about whether this is >> a fix - sounds like it is from the change log. >> >> Clearly stating that it is a fix will help so it can be picked >> up for stables. > > I would say it's a fix, as the test currently fails in certain scenarios > for powerpc. > > You can add: > > Fixes: 4231f30fcc34a ("selftests/ftrace: Add BTF arguments test cases") > I applied this to linux-kselftest fixes - will send it up for rc2 or rc3 thanks, -- Shuah
On Thu, 5 Dec 2024 09:06:43 -0700
Shuah Khan <skhan@linuxfoundation.org> wrote:
> I applied this to linux-kselftest fixes - will send it up for rc2 or rc3
Thanks Shuah,
-- Steve
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc index a16c6a6f6055..8f1c58f0c239 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc @@ -111,7 +111,7 @@ check_error 'p vfs_read $arg* ^$arg*' # DOUBLE_ARGS if !grep -q 'kernel return probes support:' README; then check_error 'r vfs_read ^$arg*' # NOFENTRY_ARGS fi -check_error 'p vfs_read+8 ^$arg*' # NOFENTRY_ARGS +check_error 'p vfs_read+20 ^$arg*' # NOFENTRY_ARGS check_error 'p vfs_read ^hoge' # NO_BTFARG check_error 'p kfree ^$arg10' # NO_BTFARG (exceed the number of parameters) check_error 'r kfree ^$retval' # NO_RETVAL
In 'NOFENTRY_ARGS' test case for syntax check, any offset X of `vfs_read+X` except function entry offset (0) fits the criterion, even if that offset is not at instruction boundary, as the parser comes before probing. But with "ENDBR64" instruction on x86, offset 4 is treated as function entry. So, X can't be 4 as well. Thus, 8 was used as offset for the test case. On 64-bit powerpc though, any offset <= 16 can be considered function entry depending on build configuration (see arch_kprobe_on_func_entry() for implementation details). So, use `vfs_read+20` to accommodate that scenario too. Suggested-by: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com> --- Changes in v2: * Use 20 as offset for all arches. .../selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)