* [REVIEW][PATCH 16/22] signal/sh: Use force_sig_fault where appropriate [not found] <87604mhrnb.fsf@xmission.com> @ 2018-04-20 14:38 ` Eric W. Biederman 2018-04-20 14:55 ` Rich Felker 0 siblings, 1 reply; 5+ messages in thread From: Eric W. Biederman @ 2018-04-20 14:38 UTC (permalink / raw) To: linux-arch Cc: linux-kernel, Eric W. Biederman, Yoshinori Sato, Rich Felker, linux-sh Filling in struct siginfo before calling force_sig_info a tedious and error prone process, where once in a great while the wrong fields are filled out, and siginfo has been inconsistently cleared. Simplify this process by using the helper force_sig_fault. Which takes as a parameters all of the information it needs, ensures all of the fiddly bits of filling in struct siginfo are done properly and then calls force_sig_info. In short about a 5 line reduction in code for every time force_sig_info is called, which makes the calling function clearer. Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Cc: linux-sh@vger.kernel.org Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/sh/kernel/traps_32.c | 19 +++++-------------- arch/sh/math-emu/math.c | 9 ++------- arch/sh/mm/fault.c | 10 +--------- 3 files changed, 8 insertions(+), 30 deletions(-) diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c index e85e59c3d6df..660a4bc17698 100644 --- a/arch/sh/kernel/traps_32.c +++ b/arch/sh/kernel/traps_32.c @@ -477,7 +477,6 @@ asmlinkage void do_address_error(struct pt_regs *regs, { unsigned long error_code = 0; mm_segment_t oldfs; - siginfo_t info; insn_size_t instruction; int tmp; @@ -537,12 +536,7 @@ asmlinkage void do_address_error(struct pt_regs *regs, "access (PC %lx PR %lx)\n", current->comm, regs->pc, regs->pr); - clear_siginfo(&info); - info.si_signo = SIGBUS; - info.si_errno = 0; - info.si_code = si_code; - info.si_addr = (void __user *)address; - force_sig_info(SIGBUS, &info, current); + force_sig_fault(SIGBUS, si_code, (void __user *)address, current); } else { inc_unaligned_kernel_access(); @@ -599,20 +593,17 @@ int is_dsp_inst(struct pt_regs *regs) #ifdef CONFIG_CPU_SH2A asmlinkage void do_divide_error(unsigned long r4) { - siginfo_t info; + int code; - clear_siginfo(&info); switch (r4) { case TRAP_DIVZERO_ERROR: - info.si_code = FPE_INTDIV; + code = FPE_INTDIV; break; case TRAP_DIVOVF_ERROR: - info.si_code = FPE_INTOVF; + code = FPE_INTOVF; break; } - - info.si_signo = SIGFPE; - force_sig_info(info.si_signo, &info, current); + force_sig_fault(SIGFPE, code, NULL, current); } #endif diff --git a/arch/sh/math-emu/math.c b/arch/sh/math-emu/math.c index d6d2213df078..a0fa8fc88739 100644 --- a/arch/sh/math-emu/math.c +++ b/arch/sh/math-emu/math.c @@ -507,7 +507,6 @@ static int ieee_fpe_handler(struct pt_regs *regs) unsigned short insn = *(unsigned short *)regs->pc; unsigned short finsn; unsigned long nextpc; - siginfo_t info; int nib[4] = { (insn >> 12) & 0xf, (insn >> 8) & 0xf, @@ -560,12 +559,8 @@ static int ieee_fpe_handler(struct pt_regs *regs) ~(FPSCR_CAUSE_MASK | FPSCR_FLAG_MASK); task_thread_info(tsk)->status |= TS_USEDFPU; } else { - clear_siginfo(&info); - info.si_signo = SIGFPE; - info.si_errno = 0; - info.si_code = FPE_FLTINV; - info.si_addr = (void __user *)regs->pc; - force_sig_info(SIGFPE, &info, tsk); + force_sig_fault(SIGFPE, FPE_FLTINV, + (void __user *)regs->pc, tsk); } regs->pc = nextpc; diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c index 4c98b6f20e02..b8e7bb84b6b1 100644 --- a/arch/sh/mm/fault.c +++ b/arch/sh/mm/fault.c @@ -42,15 +42,7 @@ static void force_sig_info_fault(int si_signo, int si_code, unsigned long address, struct task_struct *tsk) { - siginfo_t info; - - clear_siginfo(&info); - info.si_signo = si_signo; - info.si_errno = 0; - info.si_code = si_code; - info.si_addr = (void __user *)address; - - force_sig_info(si_signo, &info, tsk); + force_sig_fault(si_signo, si_code, (void __user *)address, tsk); } /* -- 2.14.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [REVIEW][PATCH 16/22] signal/sh: Use force_sig_fault where appropriate 2018-04-20 14:38 ` [REVIEW][PATCH 16/22] signal/sh: Use force_sig_fault where appropriate Eric W. Biederman @ 2018-04-20 14:55 ` Rich Felker 2018-05-28 9:19 ` Geert Uytterhoeven 0 siblings, 1 reply; 5+ messages in thread From: Rich Felker @ 2018-04-20 14:55 UTC (permalink / raw) To: Eric W. Biederman; +Cc: linux-arch, linux-kernel, Yoshinori Sato, linux-sh On Fri, Apr 20, 2018 at 09:38:05AM -0500, Eric W. Biederman wrote: > Filling in struct siginfo before calling force_sig_info a tedious and > error prone process, where once in a great while the wrong fields > are filled out, and siginfo has been inconsistently cleared. > > Simplify this process by using the helper force_sig_fault. Which > takes as a parameters all of the information it needs, ensures > all of the fiddly bits of filling in struct siginfo are done properly > and then calls force_sig_info. > > In short about a 5 line reduction in code for every time force_sig_info > is called, which makes the calling function clearer. > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > Cc: Rich Felker <dalias@libc.org> > Cc: linux-sh@vger.kernel.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > arch/sh/kernel/traps_32.c | 19 +++++-------------- > arch/sh/math-emu/math.c | 9 ++------- > arch/sh/mm/fault.c | 10 +--------- > 3 files changed, 8 insertions(+), 30 deletions(-) > > diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c > index e85e59c3d6df..660a4bc17698 100644 > --- a/arch/sh/kernel/traps_32.c > +++ b/arch/sh/kernel/traps_32.c > @@ -477,7 +477,6 @@ asmlinkage void do_address_error(struct pt_regs *regs, > { > unsigned long error_code = 0; > mm_segment_t oldfs; > - siginfo_t info; > insn_size_t instruction; > int tmp; > > @@ -537,12 +536,7 @@ asmlinkage void do_address_error(struct pt_regs *regs, > "access (PC %lx PR %lx)\n", current->comm, regs->pc, > regs->pr); > > - clear_siginfo(&info); > - info.si_signo = SIGBUS; > - info.si_errno = 0; > - info.si_code = si_code; > - info.si_addr = (void __user *)address; > - force_sig_info(SIGBUS, &info, current); > + force_sig_fault(SIGBUS, si_code, (void __user *)address, current); > } else { > inc_unaligned_kernel_access(); > > @@ -599,20 +593,17 @@ int is_dsp_inst(struct pt_regs *regs) > #ifdef CONFIG_CPU_SH2A > asmlinkage void do_divide_error(unsigned long r4) > { > - siginfo_t info; > + int code; > > - clear_siginfo(&info); > switch (r4) { > case TRAP_DIVZERO_ERROR: > - info.si_code = FPE_INTDIV; > + code = FPE_INTDIV; > break; > case TRAP_DIVOVF_ERROR: > - info.si_code = FPE_INTOVF; > + code = FPE_INTOVF; > break; > } > - > - info.si_signo = SIGFPE; > - force_sig_info(info.si_signo, &info, current); > + force_sig_fault(SIGFPE, code, NULL, current); > } > #endif > > diff --git a/arch/sh/math-emu/math.c b/arch/sh/math-emu/math.c > index d6d2213df078..a0fa8fc88739 100644 > --- a/arch/sh/math-emu/math.c > +++ b/arch/sh/math-emu/math.c > @@ -507,7 +507,6 @@ static int ieee_fpe_handler(struct pt_regs *regs) > unsigned short insn = *(unsigned short *)regs->pc; > unsigned short finsn; > unsigned long nextpc; > - siginfo_t info; > int nib[4] = { > (insn >> 12) & 0xf, > (insn >> 8) & 0xf, > @@ -560,12 +559,8 @@ static int ieee_fpe_handler(struct pt_regs *regs) > ~(FPSCR_CAUSE_MASK | FPSCR_FLAG_MASK); > task_thread_info(tsk)->status |= TS_USEDFPU; > } else { > - clear_siginfo(&info); > - info.si_signo = SIGFPE; > - info.si_errno = 0; > - info.si_code = FPE_FLTINV; > - info.si_addr = (void __user *)regs->pc; > - force_sig_info(SIGFPE, &info, tsk); > + force_sig_fault(SIGFPE, FPE_FLTINV, > + (void __user *)regs->pc, tsk); > } > > regs->pc = nextpc; > diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c > index 4c98b6f20e02..b8e7bb84b6b1 100644 > --- a/arch/sh/mm/fault.c > +++ b/arch/sh/mm/fault.c > @@ -42,15 +42,7 @@ static void > force_sig_info_fault(int si_signo, int si_code, unsigned long address, > struct task_struct *tsk) > { > - siginfo_t info; > - > - clear_siginfo(&info); > - info.si_signo = si_signo; > - info.si_errno = 0; > - info.si_code = si_code; > - info.si_addr = (void __user *)address; > - > - force_sig_info(si_signo, &info, tsk); > + force_sig_fault(si_signo, si_code, (void __user *)address, tsk); > } > > /* > -- > 2.14.1 Acked-by: Rich Felker <dalias@libc.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [REVIEW][PATCH 16/22] signal/sh: Use force_sig_fault where appropriate 2018-04-20 14:55 ` Rich Felker @ 2018-05-28 9:19 ` Geert Uytterhoeven 2018-05-29 15:00 ` [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error Eric W. Biederman 0 siblings, 1 reply; 5+ messages in thread From: Geert Uytterhoeven @ 2018-05-28 9:19 UTC (permalink / raw) To: Rich Felker Cc: Eric W. Biederman, Linux-Arch, Linux Kernel Mailing List, Yoshinori Sato, Linux-sh list On Fri, Apr 20, 2018 at 4:55 PM, Rich Felker <dalias@libc.org> wrote: > On Fri, Apr 20, 2018 at 09:38:05AM -0500, Eric W. Biederman wrote: >> Filling in struct siginfo before calling force_sig_info a tedious and >> error prone process, where once in a great while the wrong fields >> are filled out, and siginfo has been inconsistently cleared. >> >> Simplify this process by using the helper force_sig_fault. Which >> takes as a parameters all of the information it needs, ensures >> all of the fiddly bits of filling in struct siginfo are done properly >> and then calls force_sig_info. >> >> In short about a 5 line reduction in code for every time force_sig_info >> is called, which makes the calling function clearer. >> >> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> >> Cc: Rich Felker <dalias@libc.org> >> Cc: linux-sh@vger.kernel.org >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >> --- a/arch/sh/kernel/traps_32.c >> +++ b/arch/sh/kernel/traps_32.c >> @@ -599,20 +593,17 @@ int is_dsp_inst(struct pt_regs *regs) >> #ifdef CONFIG_CPU_SH2A >> asmlinkage void do_divide_error(unsigned long r4) >> { >> - siginfo_t info; >> + int code; >> >> - clear_siginfo(&info); >> switch (r4) { >> case TRAP_DIVZERO_ERROR: >> - info.si_code = FPE_INTDIV; >> + code = FPE_INTDIV; >> break; >> case TRAP_DIVOVF_ERROR: >> - info.si_code = FPE_INTOVF; >> + code = FPE_INTOVF; >> break; >> } >> - >> - info.si_signo = SIGFPE; >> - force_sig_info(info.si_signo, &info, current); >> + force_sig_fault(SIGFPE, code, NULL, current); /kisskb/src/arch/sh/kernel/traps_32.c:606:17: error: 'code' may be used uninitialized in this function [-Werror=uninitialized] http://kisskb.ellerman.id.au/kisskb/buildresult/13366052/ http://kisskb.ellerman.id.au/kisskb/buildresult/13366048/ http://kisskb.ellerman.id.au/kisskb/buildresult/13366072/ >> } > Acked-by: Rich Felker <dalias@libc.org> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error 2018-05-28 9:19 ` Geert Uytterhoeven @ 2018-05-29 15:00 ` Eric W. Biederman 2018-05-30 7:54 ` Sergei Shtylyov 0 siblings, 1 reply; 5+ messages in thread From: Eric W. Biederman @ 2018-05-29 15:00 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Rich Felker, Linux-Arch, Linux Kernel Mailing List, Yoshinori Sato, Linux-sh list Geert Uytterhoeven <geert@linux-m68k.org> reported: > HOSTLD scripts/mod/modpost > CC arch/sh/kernel/traps_32.o > arch/sh/kernel/traps_32.c: In function 'do_divide_error': > arch/sh/kernel/traps_32.c:606:17: error: 'code' may be used uninitialized in this function [-Werror=uninitialized] > cc1: all warnings being treated as errors It is clear from inspection that do_divide_error is only called with TRAP_DIVZERO_ERROR or TRAP_DIVOVF_ERROR, as that is the way set_exception_table_vec is called. So let gcc know the other cases should not be considered by returning in all other cases. This removes the warning and let's the code continue to build. Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Fixes: c65626c0cd4d ("signal/sh: Use force_sig_fault where appropriate") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- I am adding this fix to my tree to at least let the code build. arch/sh/kernel/traps_32.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c index 660a4bc17698..60709ad17fc7 100644 --- a/arch/sh/kernel/traps_32.c +++ b/arch/sh/kernel/traps_32.c @@ -602,6 +602,9 @@ asmlinkage void do_divide_error(unsigned long r4) case TRAP_DIVOVF_ERROR: code = FPE_INTOVF; break; + default: + /* Let gcc know unhandled cases don't make it past here */ + return; } force_sig_fault(SIGFPE, code, NULL, current); } -- 2.14.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error 2018-05-29 15:00 ` [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error Eric W. Biederman @ 2018-05-30 7:54 ` Sergei Shtylyov 0 siblings, 0 replies; 5+ messages in thread From: Sergei Shtylyov @ 2018-05-30 7:54 UTC (permalink / raw) To: Eric W. Biederman, Geert Uytterhoeven Cc: Rich Felker, Linux-Arch, Linux Kernel Mailing List, Yoshinori Sato, Linux-sh list Hello! On 5/29/2018 6:00 PM, Eric W. Biederman wrote: > Geert Uytterhoeven <geert@linux-m68k.org> reported: >> HOSTLD scripts/mod/modpost >> CC arch/sh/kernel/traps_32.o >> arch/sh/kernel/traps_32.c: In function 'do_divide_error': >> arch/sh/kernel/traps_32.c:606:17: error: 'code' may be used uninitialized in this function [-Werror=uninitialized] >> cc1: all warnings being treated as errors > > It is clear from inspection that do_divide_error is only called with > TRAP_DIVZERO_ERROR or TRAP_DIVOVF_ERROR, as that is the way > set_exception_table_vec is called. So let gcc know the other cases > should not be considered by returning in all other cases. > > This removes the warning and let's the code continue to build. Lets. :-) > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Fixes: c65626c0cd4d ("signal/sh: Use force_sig_fault where appropriate") > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> [...] MBR, Sergei ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-30 7:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <87604mhrnb.fsf@xmission.com>
2018-04-20 14:38 ` [REVIEW][PATCH 16/22] signal/sh: Use force_sig_fault where appropriate Eric W. Biederman
2018-04-20 14:55 ` Rich Felker
2018-05-28 9:19 ` Geert Uytterhoeven
2018-05-29 15:00 ` [PATCH] signal/sh: Stop gcc warning about an impossible case in do_divide_error Eric W. Biederman
2018-05-30 7:54 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).