[PATCH v2] arch: define weak abort

[PATCH v2] arch: define weak abort

[Sudip Mukherjee]

gcc toggle -fisolate-erroneous-paths-dereference (default at -O2
onwards) isolates faulty code paths such as null pointer access, divide
by zero etc. If gcc port doesnt implement __builtin_trap, an abort() is
generated which causes kernel link error.

In this case, gcc is generating abort due to 'divide by zero' in
lib/mpi/mpih-div.c.

Currently 'frv' and 'arc' are failing. Previously other arch was also
broken like m32r was fixed by d22e3d69ee1a ("m32r: fix build failure").

Lets define this weak function which is common for all arch and fix the
problem permanently. We can even remove the arch specific 'abort' after
this is done.

Cc: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---

Hi Alexey,
I was thinking of sending the m32r revert patch in few days. My m32r
builds are having a little problem and should be fixed by this weekend.
I can not test m32r before that. We can also send a patch to remove the
same code in arm and unicore32.

 kernel/exit.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/exit.c b/kernel/exit.c
index af6c245..90c6869 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1759,3 +1759,11 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
 	return -EFAULT;
 }
 #endif
+
+__weak void abort(void)
+{
+	BUG();
+
+	/* if that doesn't kill us, halt */
+	panic("Oops failed to kill thread");
+}
-- 
1.9.1

Re: [PATCH v2] arch: define weak abort

[Alexey Brodkin]

Hi Sudip,

On Tue, 2017-12-12 at 22:49 +0000, Sudip Mukherjee wrote:
> gcc toggle -fisolate-erroneous-paths-dereference (default at -O2
> onwards) isolates faulty code paths such as null pointer access, divide
> by zero etc. If gcc port doesnt implement __builtin_trap, an abort() is
> generated which causes kernel link error.
> 
> In this case, gcc is generating abort due to 'divide by zero' in
> lib/mpi/mpih-div.c.
> 
> Currently 'frv' and 'arc' are failing. Previously other arch was also
> broken like m32r was fixed by d22e3d69ee1a ("m32r: fix build failure").
> 
> Lets define this weak function which is common for all arch and fix the
> problem permanently. We can even remove the arch specific 'abort' after
> this is done.
> 
> Cc: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> ---
> 
> Hi Alexey,
> I was thinking of sending the m32r revert patch in few days. My m32r
> builds are having a little problem and should be fixed by this weekend.
> I can not test m32r before that.

Understood, that's fine by me.

> We can also send a patch to remove the
> same code in arm and unicore32.

Sure that would be really great!

-Alexey

Re: [PATCH v2] arch: define weak abort

[Vineet Gupta]

On 12/12/2017 02:49 PM, Sudip Mukherjee wrote:
> gcc toggle -fisolate-erroneous-paths-dereference (default at -O2
> onwards) isolates faulty code paths such as null pointer access, divide
> by zero etc. If gcc port doesnt implement __builtin_trap, an abort() is
> generated which causes kernel link error.
> 
> In this case, gcc is generating abort due to 'divide by zero' in
> lib/mpi/mpih-div.c.
> 
> Currently 'frv' and 'arc' are failing. Previously other arch was also
> broken like m32r was fixed by d22e3d69ee1a ("m32r: fix build failure").
> 
> Lets define this weak function which is common for all arch and fix the
> problem permanently. We can even remove the arch specific 'abort' after
> this is done.
> 
> Cc: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> ---
> 
> Hi Alexey,
> I was thinking of sending the m32r revert patch in few days. My m32r
> builds are having a little problem and should be fixed by this weekend.
> I can not test m32r before that. We can also send a patch to remove the
> same code in arm and unicore32.
> 
>   kernel/exit.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/exit.c b/kernel/exit.c
> index af6c245..90c6869 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1759,3 +1759,11 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
>   	return -EFAULT;
>   }
>   #endif
> +
> +__weak void abort(void)
> +{
> +	BUG();
> +
> +	/* if that doesn't kill us, halt */
> +	panic("Oops failed to kill thread");
> +}

Hmm, I realize there's a small gotcha with this, when testing with a different 
patch from Arnd. This needs an EXPORT_SYMBOL() as well !

-Vineet

Re: [PATCH v2] arch: define weak abort

[Andrew Morton]

On Tue, 19 Dec 2017 13:39:22 -0800 Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:

> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -1759,3 +1759,11 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
> >   	return -EFAULT;
> >   }
> >   #endif
> > +
> > +__weak void abort(void)
> > +{
> > +	BUG();
> > +
> > +	/* if that doesn't kill us, halt */
> > +	panic("Oops failed to kill thread");
> > +}
> 
> Hmm, I realize there's a small gotcha with this, when testing with a different 
> patch from Arnd. This needs an EXPORT_SYMBOL() as well !

Yup.  This?

From: Andrew Morton <akpm@linux-foundation.org>
Subject: kernel/exit.c: export abort() to modules

gcc -fisolate-erroneous-paths-dereference can generate calls to abort()
from modular code too.

Reported-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Alexey Brodkin" <Alexey.Brodkin@synopsys.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/exit.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN kernel/exit.c~a kernel/exit.c
--- a/kernel/exit.c~a
+++ a/kernel/exit.c
@@ -1763,3 +1763,4 @@ __weak void abort(void)
 	/* if that doesn't kill us, halt */
 	panic("Oops failed to kill thread");
 }
+EXPORT_SYMBOL(abort);
_

Re: [PATCH v2] arch: define weak abort

[Sudip Mukherjee]

On Tue, Dec 19, 2017 at 01:55:59PM -0800, Andrew Morton wrote:
> On Tue, 19 Dec 2017 13:39:22 -0800 Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
> 

<snip>

> > Hmm, I realize there's a small gotcha with this, when testing with a different 
> > patch from Arnd. This needs an EXPORT_SYMBOL() as well !
> 
> Yup.  This?

Yes, just faced this when I was testing with the reverted patch on m32r.

> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: kernel/exit.c: export abort() to modules
> 
> gcc -fisolate-erroneous-paths-dereference can generate calls to abort()
> from modular code too.
> 
> Reported-by: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: "Alexey Brodkin" <Alexey.Brodkin@synopsys.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---

Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

--
Regards
Sudip