[PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[Mike Rapoport]

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

kbuild reports the following warning:

   arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
>> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
     412 |         struct kprobe *p = NULL;
         |                        ^

The variable 'p' is indeed unused since the commit fa5a24b16f94
("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")

Remove that variable along with 'kprobe_opcode_t *addr' which also
becomes unused after 'p' is removed.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/
Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---

I don't know why the warning poped up only now, the code there didn't
change for some time :/

 arch/sh/kernel/kprobes.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/sh/kernel/kprobes.c b/arch/sh/kernel/kprobes.c
index 49c4ffd782d6..a250fb1b9420 100644
--- a/arch/sh/kernel/kprobes.c
+++ b/arch/sh/kernel/kprobes.c
@@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 				       unsigned long val, void *data)
 {
-	struct kprobe *p = NULL;
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
-	kprobe_opcode_t *addr = NULL;
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	addr = (kprobe_opcode_t *) (args->regs->pc);
 	if (val == DIE_TRAP &&
 	    args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) {
 		if (!kprobe_running()) {
@@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
 				ret = NOTIFY_DONE;
 			}
 		} else {
-			p = get_kprobe(addr);
 			if ((kcb->kprobe_status == KPROBE_HIT_SS) ||
 			    (kcb->kprobe_status == KPROBE_REENTER)) {
 				if (post_kprobe_handler(args->regs))
-- 
2.47.2

Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[Geert Uytterhoeven]

Hi Mike,

On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> kbuild reports the following warning:
>
>    arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
> >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
>      412 |         struct kprobe *p = NULL;
>          |                        ^
>
> The variable 'p' is indeed unused since the commit fa5a24b16f94
> ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
>
> Remove that variable along with 'kprobe_opcode_t *addr' which also
> becomes unused after 'p' is removed.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/
> Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Thanks for your patch!

"p" and "addr" are definitely unused (besides side-effects?), so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/sh/kernel/kprobes.c
> +++ b/arch/sh/kernel/kprobes.c
> @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
>                                        unsigned long val, void *data)
>  {
> -       struct kprobe *p = NULL;
>         struct die_args *args = (struct die_args *)data;
>         int ret = NOTIFY_DONE;
> -       kprobe_opcode_t *addr = NULL;
>         struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>
> -       addr = (kprobe_opcode_t *) (args->regs->pc);
>         if (val == DIE_TRAP &&
>             args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) {
>                 if (!kprobe_running()) {
> @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
>                                 ret = NOTIFY_DONE;
>                         }
>                 } else {
> -                       p = get_kprobe(addr);
>                         if ((kcb->kprobe_status == KPROBE_HIT_SS) ||
>                             (kcb->kprobe_status == KPROBE_REENTER)) {
>                                 if (post_kprobe_handler(args->regs))

I have no idea what this code is supposed to do, and if it actually
works.  Red flags are that the assigned "p" was never used at all
since the inception of this function.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[Mike Rapoport]

On Mon, May 19, 2025 at 10:48:20AM +0200, Geert Uytterhoeven wrote:
> Hi Mike,
> 
> On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > kbuild reports the following warning:
> >
> >    arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
> > >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
> >      412 |         struct kprobe *p = NULL;
> >          |                        ^
> >
> > The variable 'p' is indeed unused since the commit fa5a24b16f94
> > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> >
> > Remove that variable along with 'kprobe_opcode_t *addr' which also
> > becomes unused after 'p' is removed.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/
> > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> Thanks for your patch!
> 
> "p" and "addr" are definitely unused (besides side-effects?), so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/sh/kernel/kprobes.c
> > +++ b/arch/sh/kernel/kprobes.c
> > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> >  int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> >                                        unsigned long val, void *data)
> >  {
> > -       struct kprobe *p = NULL;
> >         struct die_args *args = (struct die_args *)data;
> >         int ret = NOTIFY_DONE;
> > -       kprobe_opcode_t *addr = NULL;
> >         struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> >
> > -       addr = (kprobe_opcode_t *) (args->regs->pc);
> >         if (val == DIE_TRAP &&
> >             args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) {
> >                 if (!kprobe_running()) {
> > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> >                                 ret = NOTIFY_DONE;
> >                         }
> >                 } else {
> > -                       p = get_kprobe(addr);
> >                         if ((kcb->kprobe_status == KPROBE_HIT_SS) ||
> >                             (kcb->kprobe_status == KPROBE_REENTER)) {
> >                                 if (post_kprobe_handler(args->regs))
> 
> I have no idea what this code is supposed to do, and if it actually
> works.  Red flags are that the assigned "p" was never used at all
> since the inception of this function.

"p" was used before fa5a24b16f94 ("sh/kprobes: Don't call the
->break_handler() in SH kprobes code"), but I can't say I understand that
code either :)

CC'ing Masami, he probably knows what this code does.
 
> Gr{oetje,eeting}s,
> 
>                         Geert

-- 
Sincerely yours,
Mike.

Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[Geert Uytterhoeven]

Hi Mike,

On Mon, 19 May 2025 at 12:02, Mike Rapoport <rppt@kernel.org> wrote:
> On Mon, May 19, 2025 at 10:48:20AM +0200, Geert Uytterhoeven wrote:
> > On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote:
> > > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> > >
> > > kbuild reports the following warning:
> > >
> > >    arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
> > > >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
> > >      412 |         struct kprobe *p = NULL;
> > >          |                        ^
> > >
> > > The variable 'p' is indeed unused since the commit fa5a24b16f94
> > > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> > >
> > > Remove that variable along with 'kprobe_opcode_t *addr' which also
> > > becomes unused after 'p' is removed.
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/
> > > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >
> > Thanks for your patch!
> >
> > "p" and "addr" are definitely unused (besides side-effects?), so
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > > --- a/arch/sh/kernel/kprobes.c
> > > +++ b/arch/sh/kernel/kprobes.c
> > > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> > >  int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> > >                                        unsigned long val, void *data)
> > >  {
> > > -       struct kprobe *p = NULL;
> > >         struct die_args *args = (struct die_args *)data;
> > >         int ret = NOTIFY_DONE;
> > > -       kprobe_opcode_t *addr = NULL;
> > >         struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> > >
> > > -       addr = (kprobe_opcode_t *) (args->regs->pc);
> > >         if (val == DIE_TRAP &&
> > >             args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) {
> > >                 if (!kprobe_running()) {
> > > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> > >                                 ret = NOTIFY_DONE;
> > >                         }
> > >                 } else {
> > > -                       p = get_kprobe(addr);
> > >                         if ((kcb->kprobe_status == KPROBE_HIT_SS) ||
> > >                             (kcb->kprobe_status == KPROBE_REENTER)) {
> > >                                 if (post_kprobe_handler(args->regs))
> >
> > I have no idea what this code is supposed to do, and if it actually
> > works.  Red flags are that the assigned "p" was never used at all
> > since the inception of this function.
>
> "p" was used before fa5a24b16f94 ("sh/kprobes: Don't call the
> ->break_handler() in SH kprobes code"), but I can't say I understand that
> code either :)

Yes, the _variable_ "p" was used before, but not before assigning a
different value to it first.

I guess this is where Rust would be helpful? Although C can and does
give a warning here, too...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify()

[John Paul Adrian Glaubitz]

Hi Mike,

On Sat, 2025-05-17 at 12:30 +0300, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> 
> kbuild reports the following warning:
> 
>    arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify':
> > > arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable]
>      412 |         struct kprobe *p = NULL;
>          |                        ^
> 
> The variable 'p' is indeed unused since the commit fa5a24b16f94
> ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> 
> Remove that variable along with 'kprobe_opcode_t *addr' which also
> becomes unused after 'p' is removed.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/
> Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code")
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> 
> I don't know why the warning poped up only now, the code there didn't
> change for some time :/
> 
>  arch/sh/kernel/kprobes.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/sh/kernel/kprobes.c b/arch/sh/kernel/kprobes.c
> index 49c4ffd782d6..a250fb1b9420 100644
> --- a/arch/sh/kernel/kprobes.c
> +++ b/arch/sh/kernel/kprobes.c
> @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
>  				       unsigned long val, void *data)
>  {
> -	struct kprobe *p = NULL;
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
> -	kprobe_opcode_t *addr = NULL;
>  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>  
> -	addr = (kprobe_opcode_t *) (args->regs->pc);
>  	if (val == DIE_TRAP &&
>  	    args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) {
>  		if (!kprobe_running()) {
> @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
>  				ret = NOTIFY_DONE;
>  			}
>  		} else {
> -			p = get_kprobe(addr);
>  			if ((kcb->kprobe_status == KPROBE_HIT_SS) ||
>  			    (kcb->kprobe_status == KPROBE_REENTER)) {
>  				if (post_kprobe_handler(args->regs))

Thanks for catching this! Looks good to me!

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913