linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
@ 2018-09-21  0:15 Jason A. Donenfeld
  2018-09-21  1:50 ` Andy Lutomirski
  0 siblings, 1 reply; 11+ messages in thread
From: Jason A. Donenfeld @ 2018-09-21  0:15 UTC (permalink / raw)
  To: Arnd Bergmann, Ard Biesheuvel, LKML, Andrew Lutomirski; +Cc: Jason A. Donenfeld

It turns out that KASAN in general will bloat stack frames in unexpected
ways, not just KASAN_EXTRA. So, this patch trivially changes that
default to be associated with KASAN instead of KASAN_EXTRA.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..39078a080e29 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
 config FRAME_WARN
 	int "Warn for stack frames larger than (needs gcc 4.4)"
 	range 0 8192
-	default 3072 if KASAN_EXTRA
+	default 3072 if KASAN
 	default 2048 if GCC_PLUGIN_LATENT_ENTROPY
 	default 1280 if (!64BIT && PARISC)
 	default 1024 if (!64BIT && !PARISC)
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  0:15 [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN Jason A. Donenfeld
@ 2018-09-21  1:50 ` Andy Lutomirski
  2018-09-21  8:42   ` Dmitry Vyukov
  2018-09-21  9:25   ` Andrey Ryabinin
  0 siblings, 2 replies; 11+ messages in thread
From: Andy Lutomirski @ 2018-09-21  1:50 UTC (permalink / raw)
  To: Jason A. Donenfeld, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: Arnd Bergmann, Ard Biesheuvel, LKML, Andrew Lutomirski

This patch seems reasonable, but you emailed the wrong people :)

On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> It turns out that KASAN in general will bloat stack frames in unexpected
> ways, not just KASAN_EXTRA. So, this patch trivially changes that
> default to be associated with KASAN instead of KASAN_EXTRA.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  lib/Kconfig.debug | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 4966c4fbe7f7..39078a080e29 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
>  config FRAME_WARN
>         int "Warn for stack frames larger than (needs gcc 4.4)"
>         range 0 8192
> -       default 3072 if KASAN_EXTRA
> +       default 3072 if KASAN
>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>         default 1280 if (!64BIT && PARISC)
>         default 1024 if (!64BIT && !PARISC)
> --
> 2.19.0
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  1:50 ` Andy Lutomirski
@ 2018-09-21  8:42   ` Dmitry Vyukov
  2018-09-21 12:11     ` Andrey Konovalov
  2018-09-21  9:25   ` Andrey Ryabinin
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Vyukov @ 2018-09-21  8:42 UTC (permalink / raw)
  To: Andy Lutomirski, kasan-dev
  Cc: Jason A. Donenfeld, Andrey Ryabinin, Alexander Potapenko,
	Arnd Bergmann, Ard Biesheuvel, LKML

On Fri, Sep 21, 2018 at 3:50 AM, Andy Lutomirski <luto@kernel.org> wrote:
> This patch seems reasonable, but you emailed the wrong people :)
>
> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> It turns out that KASAN in general will bloat stack frames in unexpected
>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
>> default to be associated with KASAN instead of KASAN_EXTRA.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> ---
>>  lib/Kconfig.debug | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 4966c4fbe7f7..39078a080e29 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
>>  config FRAME_WARN
>>         int "Warn for stack frames larger than (needs gcc 4.4)"
>>         range 0 8192
>> -       default 3072 if KASAN_EXTRA
>> +       default 3072 if KASAN
>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>>         default 1280 if (!64BIT && PARISC)
>>         default 1024 if (!64BIT && !PARISC)


Looks fine to me.
KASAN always increases stack size too for exactly that reason, so it's
reasonable to allow larger frames rather than try to massage code
every here and there to fit into the normal frame size.

Acked-by: Dmitry Vyukov <dvyukov@google.com>

Thanks

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  1:50 ` Andy Lutomirski
  2018-09-21  8:42   ` Dmitry Vyukov
@ 2018-09-21  9:25   ` Andrey Ryabinin
  2018-09-21  9:45     ` Dmitry Vyukov
  1 sibling, 1 reply; 11+ messages in thread
From: Andrey Ryabinin @ 2018-09-21  9:25 UTC (permalink / raw)
  To: Andy Lutomirski, Jason A. Donenfeld, Alexander Potapenko, Dmitry Vyukov
  Cc: Arnd Bergmann, Ard Biesheuvel, LKML



On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
> This patch seems reasonable, but you emailed the wrong people :)
> 
> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> It turns out that KASAN in general will bloat stack frames in unexpected
>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
>> default to be associated with KASAN instead of KASAN_EXTRA.
>>

KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8

If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
I mean reduce stack usage.



>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> ---
>>  lib/Kconfig.debug | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 4966c4fbe7f7..39078a080e29 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
>>  config FRAME_WARN
>>         int "Warn for stack frames larger than (needs gcc 4.4)"
>>         range 0 8192
>> -       default 3072 if KASAN_EXTRA
>> +       default 3072 if KASAN
>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>>         default 1280 if (!64BIT && PARISC)
>>         default 1024 if (!64BIT && !PARISC)
>> --
>> 2.19.0
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  9:25   ` Andrey Ryabinin
@ 2018-09-21  9:45     ` Dmitry Vyukov
  2018-09-21  9:55       ` Nathan Chancellor
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Dmitry Vyukov @ 2018-09-21  9:45 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andy Lutomirski, Jason A. Donenfeld, Alexander Potapenko,
	Arnd Bergmann, Ard Biesheuvel, LKML, Nick Desaulniers

On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
>> This patch seems reasonable, but you emailed the wrong people :)
>>
>> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>>
>>> It turns out that KASAN in general will bloat stack frames in unexpected
>>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
>>> default to be associated with KASAN instead of KASAN_EXTRA.
>>>
>
> KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
> If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
>
> If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
> I mean reduce stack usage.


+Nick who is also hitting these warnings on clang/arm64 build. As far
as I understand the situation there is much worse.

I would be good to understand/fix the worst offenders. But the stack
size increase with KASAN is a real, inherent thing. So if we live very
close the edge, we can get people using different compilers and/or
versions of compilers constantly breaking each other. And clang hits
this warnings in lots of places today just because the current code
was tailored to gcc over long period, i.e. allowing more locals where
gcc happened to handle that better and having fewer locals where gcc
happened to handle it worse. But for another compiler all these
assumptions are significantly perturbed.

Nick, do you know what frame size limit eliminates the bulk of
warnings on clang? Is 3072 a reasonable limit allowing to fix the
remaining outliners?


>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>> ---
>>>  lib/Kconfig.debug | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 4966c4fbe7f7..39078a080e29 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
>>>  config FRAME_WARN
>>>         int "Warn for stack frames larger than (needs gcc 4.4)"
>>>         range 0 8192
>>> -       default 3072 if KASAN_EXTRA
>>> +       default 3072 if KASAN
>>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>>>         default 1280 if (!64BIT && PARISC)
>>>         default 1024 if (!64BIT && !PARISC)
>>> --
>>> 2.19.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  9:45     ` Dmitry Vyukov
@ 2018-09-21  9:55       ` Nathan Chancellor
  2018-09-21 17:59       ` Nick Desaulniers
  2018-09-22 14:56       ` Arnd Bergmann
  2 siblings, 0 replies; 11+ messages in thread
From: Nathan Chancellor @ 2018-09-21  9:55 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andrey Ryabinin, Andy Lutomirski, Jason A. Donenfeld,
	Alexander Potapenko, Arnd Bergmann, Ard Biesheuvel, LKML,
	Nick Desaulniers

On Fri, Sep 21, 2018 at 11:45:07AM +0200, Dmitry Vyukov wrote:
> On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
> > On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
> >> This patch seems reasonable, but you emailed the wrong people :)
> >>
> >> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >>>
> >>> It turns out that KASAN in general will bloat stack frames in unexpected
> >>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
> >>> default to be associated with KASAN instead of KASAN_EXTRA.
> >>>
> >
> > KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
> > If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
> >
> > If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
> > I mean reduce stack usage.
> 
> 
> +Nick who is also hitting these warnings on clang/arm64 build. As far
> as I understand the situation there is much worse.
> 
> I would be good to understand/fix the worst offenders. But the stack
> size increase with KASAN is a real, inherent thing. So if we live very
> close the edge, we can get people using different compilers and/or
> versions of compilers constantly breaking each other. And clang hits
> this warnings in lots of places today just because the current code
> was tailored to gcc over long period, i.e. allowing more locals where
> gcc happened to handle that better and having fewer locals where gcc
> happened to handle it worse. But for another compiler all these
> assumptions are significantly perturbed.
> 
> Nick, do you know what frame size limit eliminates the bulk of
> warnings on clang? Is 3072 a reasonable limit allowing to fix the
> remaining outliners?
> 

Hi Dmitry,

I know I'm not Nick and I hope I am not butting in but I've been
following this thread due to these warnings cropping up in Clang.
We've been tracking them on GitHub and judging from the values
there, I would argue that 3072 is a good starting value.

Link: https://github.com/ClangBuiltLinux/linux/issues/39

Cheers,
Nathan

> 
> >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >>> ---
> >>>  lib/Kconfig.debug | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> >>> index 4966c4fbe7f7..39078a080e29 100644
> >>> --- a/lib/Kconfig.debug
> >>> +++ b/lib/Kconfig.debug
> >>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
> >>>  config FRAME_WARN
> >>>         int "Warn for stack frames larger than (needs gcc 4.4)"
> >>>         range 0 8192
> >>> -       default 3072 if KASAN_EXTRA
> >>> +       default 3072 if KASAN
> >>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> >>>         default 1280 if (!64BIT && PARISC)
> >>>         default 1024 if (!64BIT && !PARISC)
> >>> --
> >>> 2.19.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  8:42   ` Dmitry Vyukov
@ 2018-09-21 12:11     ` Andrey Konovalov
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Konovalov @ 2018-09-21 12:11 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andy Lutomirski, kasan-dev, Jason A. Donenfeld, Andrey Ryabinin,
	Alexander Potapenko, Arnd Bergmann, Ard Biesheuvel, LKML

On Fri, Sep 21, 2018 at 10:42 AM, 'Dmitry Vyukov' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
> On Fri, Sep 21, 2018 at 3:50 AM, Andy Lutomirski <luto@kernel.org> wrote:
>> This patch seems reasonable, but you emailed the wrong people :)
>>
>> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>>
>>> It turns out that KASAN in general will bloat stack frames in unexpected
>>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
>>> default to be associated with KASAN instead of KASAN_EXTRA.
>>>
>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>> ---
>>>  lib/Kconfig.debug | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 4966c4fbe7f7..39078a080e29 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
>>>  config FRAME_WARN
>>>         int "Warn for stack frames larger than (needs gcc 4.4)"
>>>         range 0 8192
>>> -       default 3072 if KASAN_EXTRA
>>> +       default 3072 if KASAN
>>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>>>         default 1280 if (!64BIT && PARISC)
>>>         default 1024 if (!64BIT && !PARISC)
>
>
> Looks fine to me.
> KASAN always increases stack size too for exactly that reason, so it's
> reasonable to allow larger frames rather than try to massage code
> every here and there to fit into the normal frame size.

Do we need KASAN_EXTRA then at all?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  9:45     ` Dmitry Vyukov
  2018-09-21  9:55       ` Nathan Chancellor
@ 2018-09-21 17:59       ` Nick Desaulniers
  2018-09-21 18:17         ` Nick Desaulniers
  2018-09-22 14:56       ` Arnd Bergmann
  2 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2018-09-21 17:59 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andrey Ryabinin, luto, Jason, Alexander Potapenko, Arnd Bergmann,
	Ard Biesheuvel, LKML

On Fri, Sep 21, 2018 at 2:45 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
> > On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
> >> This patch seems reasonable, but you emailed the wrong people :)
> >>
> >> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >>>
> >>> It turns out that KASAN in general will bloat stack frames in unexpected
> >>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
> >>> default to be associated with KASAN instead of KASAN_EXTRA.
> >>>
> >
> > KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
> > If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
> >
> > If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
> > I mean reduce stack usage.
>
>
> +Nick who is also hitting these warnings on clang/arm64 build. As far
> as I understand the situation there is much worse.
>
> I would be good to understand/fix the worst offenders. But the stack
> size increase with KASAN is a real, inherent thing. So if we live very
> close the edge, we can get people using different compilers and/or
> versions of compilers constantly breaking each other. And clang hits
> this warnings in lots of places today just because the current code
> was tailored to gcc over long period, i.e. allowing more locals where
> gcc happened to handle that better and having fewer locals where gcc
> happened to handle it worse. But for another compiler all these
> assumptions are significantly perturbed.
>
> Nick, do you know what frame size limit eliminates the bulk of
> warnings on clang? Is 3072 a reasonable limit allowing to fix the
> remaining outliners?

Here's a plot of the distribution from an arm64 clang build with
allyesconfig minus LSE_ATOMICS, BIG_ENDIAN, and GCOV:
https://github.com/ClangBuiltLinux/linux/issues/39#issuecomment-423621655

A value of 3072 would ignore 79 out of the 128 instances (62%).

Some values are huge outliers.

>
>
> >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >>> ---
> >>>  lib/Kconfig.debug | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> >>> index 4966c4fbe7f7..39078a080e29 100644
> >>> --- a/lib/Kconfig.debug
> >>> +++ b/lib/Kconfig.debug
> >>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
> >>>  config FRAME_WARN
> >>>         int "Warn for stack frames larger than (needs gcc 4.4)"
> >>>         range 0 8192
> >>> -       default 3072 if KASAN_EXTRA
> >>> +       default 3072 if KASAN
> >>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> >>>         default 1280 if (!64BIT && PARISC)
> >>>         default 1024 if (!64BIT && !PARISC)
> >>> --
> >>> 2.19.0



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21 17:59       ` Nick Desaulniers
@ 2018-09-21 18:17         ` Nick Desaulniers
  0 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2018-09-21 18:17 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andrey Ryabinin, luto, Jason, Alexander Potapenko, Arnd Bergmann,
	Ard Biesheuvel, LKML

On Fri, Sep 21, 2018 at 10:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Sep 21, 2018 at 2:45 AM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
> > <aryabinin@virtuozzo.com> wrote:
> > > On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
> > >> This patch seems reasonable, but you emailed the wrong people :)
> > >>
> > >> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > >>>
> > >>> It turns out that KASAN in general will bloat stack frames in unexpected
> > >>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
> > >>> default to be associated with KASAN instead of KASAN_EXTRA.
> > >>>
> > >
> > > KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
> > > If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
> > >
> > > If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
> > > I mean reduce stack usage.
> >
> >
> > +Nick who is also hitting these warnings on clang/arm64 build. As far
> > as I understand the situation there is much worse.
> >
> > I would be good to understand/fix the worst offenders. But the stack
> > size increase with KASAN is a real, inherent thing. So if we live very
> > close the edge, we can get people using different compilers and/or
> > versions of compilers constantly breaking each other. And clang hits
> > this warnings in lots of places today just because the current code
> > was tailored to gcc over long period, i.e. allowing more locals where
> > gcc happened to handle that better and having fewer locals where gcc
> > happened to handle it worse. But for another compiler all these
> > assumptions are significantly perturbed.
> >
> > Nick, do you know what frame size limit eliminates the bulk of
> > warnings on clang? Is 3072 a reasonable limit allowing to fix the
> > remaining outliners?
>
> Here's a plot of the distribution from an arm64 clang build with
> allyesconfig minus LSE_ATOMICS, BIG_ENDIAN, and GCOV:
> https://github.com/ClangBuiltLinux/linux/issues/39#issuecomment-423621655
>
> A value of 3072 would ignore 79 out of the 128 instances (62%).

A value of 4614 would ignore 95% of the instances (2 standard
deviations of the distribution of current warnings).

>
> Some values are huge outliers.
>
> >
> >
> > >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > >>> ---
> > >>>  lib/Kconfig.debug | 2 +-
> > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > >>> index 4966c4fbe7f7..39078a080e29 100644
> > >>> --- a/lib/Kconfig.debug
> > >>> +++ b/lib/Kconfig.debug
> > >>> @@ -222,7 +222,7 @@ config ENABLE_MUST_CHECK
> > >>>  config FRAME_WARN
> > >>>         int "Warn for stack frames larger than (needs gcc 4.4)"
> > >>>         range 0 8192
> > >>> -       default 3072 if KASAN_EXTRA
> > >>> +       default 3072 if KASAN
> > >>>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> > >>>         default 1280 if (!64BIT && PARISC)
> > >>>         default 1024 if (!64BIT && !PARISC)
> > >>> --
> > >>> 2.19.0
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-21  9:45     ` Dmitry Vyukov
  2018-09-21  9:55       ` Nathan Chancellor
  2018-09-21 17:59       ` Nick Desaulniers
@ 2018-09-22 14:56       ` Arnd Bergmann
  2018-09-24  8:04         ` Dmitry Vyukov
  2 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2018-09-22 14:56 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Andrey Ryabinin, Andy Lutomirski, Jason A. Donenfeld,
	Alexander Potapenko, Ard Biesheuvel, Linux Kernel Mailing List,
	Nick Desaulniers

On Fri, Sep 21, 2018 at 2:45 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
> > On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
> >> This patch seems reasonable, but you emailed the wrong people :)
> >>
> >> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >>>
> >>> It turns out that KASAN in general will bloat stack frames in unexpected
> >>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
> >>> default to be associated with KASAN instead of KASAN_EXTRA.
> >>>
> >
> > KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
> > If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
> >
> > If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
> > I mean reduce stack usage.
>
>
> +Nick who is also hitting these warnings on clang/arm64 build. As far
> as I understand the situation there is much worse.
>
> I would be good to understand/fix the worst offenders. But the stack
> size increase with KASAN is a real, inherent thing. So if we live very
> close the edge, we can get people using different compilers and/or
> versions of compilers constantly breaking each other. And clang hits
> this warnings in lots of places today just because the current code
> was tailored to gcc over long period, i.e. allowing more locals where
> gcc happened to handle that better and having fewer locals where gcc
> happened to handle it worse. But for another compiler all these
> assumptions are significantly perturbed.
>
> Nick, do you know what frame size limit eliminates the bulk of
> warnings on clang? Is 3072 a reasonable limit allowing to fix the
> remaining outliners?

I do not consider 3072 a reasonable limit at all. For gcc, we managed to fix or
work around all the bugs that caused excessive stack usage. In almost all
cases there was something seriously wrong with code generation. I added
the KASAN_EXTRA option for the one thing that added an inherent significant
overhead to the stack usage.

llvm apparently has a similar bug to what we fixed in gcc. I created a
reduced test case for one of the file at:
https://bugs.llvm.org/show_bug.cgi?id=38809

Unfortunately, nobody has commented on that so far, but in the
meantime I think the best workaround would be to disable asan-stack
entirely when building with clang, and moving it to KASAN_EXTRA
there, like we did with the scope check on gcc.

        Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN
  2018-09-22 14:56       ` Arnd Bergmann
@ 2018-09-24  8:04         ` Dmitry Vyukov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Vyukov @ 2018-09-24  8:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrey Ryabinin, Andy Lutomirski, Jason A. Donenfeld,
	Alexander Potapenko, Ard Biesheuvel, Linux Kernel Mailing List,
	Nick Desaulniers

On Sat, Sep 22, 2018 at 4:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Sep 21, 2018 at 2:45 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>>
>> On Fri, Sep 21, 2018 at 11:25 AM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> wrote:
>> > On 09/21/2018 04:50 AM, Andy Lutomirski wrote:
>> >> This patch seems reasonable, but you emailed the wrong people :)
>> >>
>> >> On Thu, Sep 20, 2018 at 5:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> >>>
>> >>> It turns out that KASAN in general will bloat stack frames in unexpected
>> >>> ways, not just KASAN_EXTRA. So, this patch trivially changes that
>> >>> default to be associated with KASAN instead of KASAN_EXTRA.
>> >>>
>> >
>> > KASAN_EXTRA bloats stack more than just KASAN, that's why the limit is higher than just for KASAN.
>> > If want more details, tead the changelog from commit e7c52b84fb18f08ce49b6067ae6285aca79084a8
>> >
>> > If anything causes "stack frame > 2048" warning for KASAN we should at least try to fix it,
>> > I mean reduce stack usage.
>>
>>
>> +Nick who is also hitting these warnings on clang/arm64 build. As far
>> as I understand the situation there is much worse.
>>
>> I would be good to understand/fix the worst offenders. But the stack
>> size increase with KASAN is a real, inherent thing. So if we live very
>> close the edge, we can get people using different compilers and/or
>> versions of compilers constantly breaking each other. And clang hits
>> this warnings in lots of places today just because the current code
>> was tailored to gcc over long period, i.e. allowing more locals where
>> gcc happened to handle that better and having fewer locals where gcc
>> happened to handle it worse. But for another compiler all these
>> assumptions are significantly perturbed.
>>
>> Nick, do you know what frame size limit eliminates the bulk of
>> warnings on clang? Is 3072 a reasonable limit allowing to fix the
>> remaining outliners?
>
> I do not consider 3072 a reasonable limit at all. For gcc, we managed to fix or
> work around all the bugs that caused excessive stack usage. In almost all
> cases there was something seriously wrong with code generation. I added
> the KASAN_EXTRA option for the one thing that added an inherent significant
> overhead to the stack usage.
>
> llvm apparently has a similar bug to what we fixed in gcc. I created a
> reduced test case for one of the file at:
> https://bugs.llvm.org/show_bug.cgi?id=38809
>
> Unfortunately, nobody has commented on that so far, but in the
> meantime I think the best workaround would be to disable asan-stack
> entirely when building with clang, and moving it to KASAN_EXTRA
> there, like we did with the scope check on gcc.

Good point. I CCed more people on https://bugs.llvm.org/show_bug.cgi?id=38809

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-09-24  8:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-21  0:15 [PATCH] framewarn: expand KASAN_EXTRA exception to KASAN Jason A. Donenfeld
2018-09-21  1:50 ` Andy Lutomirski
2018-09-21  8:42   ` Dmitry Vyukov
2018-09-21 12:11     ` Andrey Konovalov
2018-09-21  9:25   ` Andrey Ryabinin
2018-09-21  9:45     ` Dmitry Vyukov
2018-09-21  9:55       ` Nathan Chancellor
2018-09-21 17:59       ` Nick Desaulniers
2018-09-21 18:17         ` Nick Desaulniers
2018-09-22 14:56       ` Arnd Bergmann
2018-09-24  8:04         ` Dmitry Vyukov

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