tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
The kprobe probepoint.tc test started failing because of the added __pfx_
symbols that were added because of -fpatchable-function-entry=X,Y causing
unwinders to see them as part of the previous functions. But kprobes can
not be added on top of them. The selftest looks for tracefs_create_dir and
picks it and the previous and following functions to add at their address.
This caused it to include __pfx_tracefs_create_dir which is invalid to
attach a kprobe to and caused the test to fail.
Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
index 624269c8d534..e1b7506c1b11 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
@@ -21,7 +21,7 @@ set_offs() { # prev target next
# We have to decode symbol addresses to get correct offsets.
# If the offset is not an instruction boundary, it cause -EILSEQ.
-set_offs `grep -A1 -B1 ${TARGET_FUNC} /proc/kallsyms | cut -f 1 -d " " | xargs`
+set_offs `grep -v __pfx_ /proc/kallsyms | grep -A1 -B1 ${TARGET_FUNC} | cut -f 1 -d " " | xargs`
UINT_TEST=no
# printf "%x" -1 returns (unsigned long)-1.
--
2.39.0On Tue, 7 Feb 2023 13:51:47 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> The kprobe probepoint.tc test started failing because of the added __pfx_
> symbols that were added because of -fpatchable-function-entry=X,Y causing
> unwinders to see them as part of the previous functions. But kprobes can
> not be added on top of them. The selftest looks for tracefs_create_dir and
> picks it and the previous and following functions to add at their address.
> This caused it to include __pfx_tracefs_create_dir which is invalid to
> attach a kprobe to and caused the test to fail.
>
> Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Shuah,
Can you pick this patch up?
Thanks,
-- Steve
On Mon, 13 Feb 2023 20:46:43 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > Shuah, > > Can you pick this patch up? ping? -- Steve
On 3/18/23 12:42, Steven Rostedt wrote: > On Mon, 13 Feb 2023 20:46:43 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> Shuah, >> >> Can you pick this patch up? > > ping? > > -- Steve l I will queue this up. Sorry for the delay. thanks, -- Shuah
On 3/20/23 06:56, Shuah Khan wrote: > On 3/18/23 12:42, Steven Rostedt wrote: >> On Mon, 13 Feb 2023 20:46:43 -0500 >> Steven Rostedt <rostedt@goodmis.org> wrote: >> >>> Shuah, >>> >>> Can you pick this patch up? >> >> ping? >> >> -- Steve > > l I will queue this up. Sorry for the delay. > Steve, The patch doesn't apply to linux-kselftest fixes branch. Please rebase and resend with cc to linux-kselftest. Makes sense why it got buried in my regular Inbox. This one didn't show up in kselftest list. Fixes tag SHA is 13 char long. I fixed it and tried to apply. When you resend, please fix the Fixes tag as well. thanks, -- Shuah
On Mon, 20 Mar 2023 10:30:44 -0600 Shuah Khan <skhan@linuxfoundation.org> wrote: > The patch doesn't apply to linux-kselftest fixes branch. > Please rebase and resend with cc to linux-kselftest. > Makes sense why it got buried in my regular Inbox. And it doesn't apply because Masami fixed it already. I'll just drop it. Thanks! -- Steve
On Mon, 20 Mar 2023 10:30:44 -0600 Shuah Khan <skhan@linuxfoundation.org> wrote: > > Steve, > > The patch doesn't apply to linux-kselftest fixes branch. > Please rebase and resend with cc to linux-kselftest. > Makes sense why it got buried in my regular Inbox. > > This one didn't show up in kselftest list. Fixes tag SHA > is 13 char long. I fixed it and tried to apply. When you > resend, please fix the Fixes tag as well. Will do. Thanks Shuah! -- Steve
On Tue, 7 Feb 2023 13:51:47 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> The kprobe probepoint.tc test started failing because of the added __pfx_
> symbols that were added because of -fpatchable-function-entry=X,Y causing
> unwinders to see them as part of the previous functions. But kprobes can
> not be added on top of them. The selftest looks for tracefs_create_dir and
> picks it and the previous and following functions to add at their address.
> This caused it to include __pfx_tracefs_create_dir which is invalid to
> attach a kprobe to and caused the test to fail.
>
> Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This looks good to me.
Acked-by:
Thanks,
> ---
> tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
> index 624269c8d534..e1b7506c1b11 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
> @@ -21,7 +21,7 @@ set_offs() { # prev target next
>
> # We have to decode symbol addresses to get correct offsets.
> # If the offset is not an instruction boundary, it cause -EILSEQ.
> -set_offs `grep -A1 -B1 ${TARGET_FUNC} /proc/kallsyms | cut -f 1 -d " " | xargs`
> +set_offs `grep -v __pfx_ /proc/kallsyms | grep -A1 -B1 ${TARGET_FUNC} | cut -f 1 -d " " | xargs`
>
> UINT_TEST=no
> # printf "%x" -1 returns (unsigned long)-1.
> --
> 2.39.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Tue, 7 Feb 2023 13:51:47 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> The kprobe probepoint.tc test started failing because of the added __pfx_
> symbols that were added because of -fpatchable-function-entry=X,Y causing
> unwinders to see them as part of the previous functions. But kprobes can
> not be added on top of them. The selftest looks for tracefs_create_dir and
> picks it and the previous and following functions to add at their address.
> This caused it to include __pfx_tracefs_create_dir which is invalid to
> attach a kprobe to and caused the test to fail.
>
> Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This is assuming that kprobes can not be added on top of these. But another
solution could be to have kprobes just pick the function the __pfx_ is for.
Would that be a better solution?
-- Steve
On Tue, Feb 07, 2023 at 01:54:02PM -0500, Steven Rostedt wrote:
> On Tue, 7 Feb 2023 13:51:47 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> >
> > The kprobe probepoint.tc test started failing because of the added __pfx_
> > symbols that were added because of -fpatchable-function-entry=X,Y causing
> > unwinders to see them as part of the previous functions. But kprobes can
> > not be added on top of them. The selftest looks for tracefs_create_dir and
> > picks it and the previous and following functions to add at their address.
> > This caused it to include __pfx_tracefs_create_dir which is invalid to
> > attach a kprobe to and caused the test to fail.
> >
> > Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>
> This is assuming that kprobes can not be added on top of these. But another
> solution could be to have kprobes just pick the function the __pfx_ is for.
> Would that be a better solution?
Simply refusing them is simplest. I don't see a compelling reason to
make this complicated.
On Wed, 8 Feb 2023 19:05:08 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Feb 07, 2023 at 01:54:02PM -0500, Steven Rostedt wrote:
> > On Tue, 7 Feb 2023 13:51:47 -0500
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > >
> > > The kprobe probepoint.tc test started failing because of the added __pfx_
> > > symbols that were added because of -fpatchable-function-entry=X,Y causing
> > > unwinders to see them as part of the previous functions. But kprobes can
> > > not be added on top of them. The selftest looks for tracefs_create_dir and
> > > picks it and the previous and following functions to add at their address.
> > > This caused it to include __pfx_tracefs_create_dir which is invalid to
> > > attach a kprobe to and caused the test to fail.
> > >
> > > Fixes: 9f2899fe36a62 ("objtool: Add option to generate prefix symbols")
> > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> >
> > This is assuming that kprobes can not be added on top of these. But another
> > solution could be to have kprobes just pick the function the __pfx_ is for.
> > Would that be a better solution?
>
> Simply refusing them is simplest. I don't see a compelling reason to
> make this complicated.
Yeah, and __pfx_ symbols has some "range", that means it is hard to translate
the probe address if user specify __pfx_*+offset.
BTW, currently kprobe event rejects this __pfx_ symbols because it is notrace
symbols, thus we can trace it if CONFIG_KPROBE_EVENTS_ON_NOTRACE=y.
But I guess it should not probe that place always because it should never
executed right?
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Thu, Feb 09, 2023 at 11:23:05PM +0900, Masami Hiramatsu wrote: > BTW, currently kprobe event rejects this __pfx_ symbols because it is notrace > symbols, thus we can trace it if CONFIG_KPROBE_EVENTS_ON_NOTRACE=y. > But I guess it should not probe that place always because it should never > executed right? Execution can take place in those ranges when X86_FEATURE_CALL_DEPTH is enabled or when CONFIG_KCFI && X86_FEATURE_IBT. In the first of those cases the prefix bytes are filled with call accounting instructions and every direct call to $sym is patched to point to __pfx_$sym+6 (aka $sym-10). https://lore.kernel.org/all/20220915111039.092790446@infradead.org/ In the second case (FineIBT) it is probably easiest if you look at the comment in arch/x86/kernel/alternative.c near CONFIG_FINEIBT. https://lore.kernel.org/all/20221027092812.185993858@infradead.org/ The __pfx_ and __cfi_ symbols are the same (in fact, when CONFIG_CFI_CLANG=y the compiler generates them and objtool no longer emits the __pfx_ symbols). https://lore.kernel.org/all/20221028194453.592512209@infradead.org/
On Wed, 8 Feb 2023 19:05:08 +0100 Peter Zijlstra <peterz@infradead.org> wrote: > > This is assuming that kprobes can not be added on top of these. But another > > solution could be to have kprobes just pick the function the __pfx_ is for. > > Would that be a better solution? > > Simply refusing them is simplest. I don't see a compelling reason to > make this complicated. OK, so you are good with the patch as is then? -- Steve
On Wed, Feb 08, 2023 at 05:03:04PM -0500, Steven Rostedt wrote: > On Wed, 8 Feb 2023 19:05:08 +0100 > Peter Zijlstra <peterz@infradead.org> wrote: > > > > This is assuming that kprobes can not be added on top of these. But another > > > solution could be to have kprobes just pick the function the __pfx_ is for. > > > Would that be a better solution? > > > > Simply refusing them is simplest. I don't see a compelling reason to > > make this complicated. > > OK, so you are good with the patch as is then? Yeah, but given I've no idea about the whole test thing or .tc files I didn't ack.
© 2016 - 2026 Red Hat, Inc.