* [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).