[PATCH] xtensa: Increase size of gcc stack frame check

[PATCH] xtensa: Increase size of gcc stack frame check

[Guenter Roeck]

xtensa frame size is larger than the frame size for almost all other
architectures. This results in more than 50 "the frame size of <n> is
larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.

Increase frame size for xtensa to 1536 bytes to avoid compile errors
due to frame size limits.

Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ed4a31e34098..afad11e57d6b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -346,7 +346,7 @@ config FRAME_WARN
 	int "Warn for stack frames larger than"
 	range 0 8192
 	default 2048 if GCC_PLUGIN_LATENT_ENTROPY
-	default 1536 if (!64BIT && PARISC)
+	default 1536 if (!64BIT && (PARISC || XTENSA))
 	default 1024 if (!64BIT && !PARISC)
 	default 2048 if 64BIT
 	help
-- 
2.33.0

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Max Filippov]

On Sat, Sep 11, 2021 at 7:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> xtensa frame size is larger than the frame size for almost all other
> architectures. This results in more than 50 "the frame size of <n> is
> larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.
>
> Increase frame size for xtensa to 1536 bytes to avoid compile errors
> due to frame size limits.
>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  lib/Kconfig.debug | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ed4a31e34098..afad11e57d6b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -346,7 +346,7 @@ config FRAME_WARN
>         int "Warn for stack frames larger than"
>         range 0 8192
>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> -       default 1536 if (!64BIT && PARISC)
> +       default 1536 if (!64BIT && (PARISC || XTENSA))
>         default 1024 if (!64BIT && !PARISC)

Shouldn't that line also be changed to
  default 1024 if (!64BIT && !(PARISC || XTENSA))
?

>         default 2048 if 64BIT
>         help
> --
> 2.33.0
>

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Guenter Roeck]

On 9/11/21 8:05 PM, Max Filippov wrote:
> On Sat, Sep 11, 2021 at 7:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> xtensa frame size is larger than the frame size for almost all other
>> architectures. This results in more than 50 "the frame size of <n> is
>> larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.
>>
>> Increase frame size for xtensa to 1536 bytes to avoid compile errors
>> due to frame size limits.
>>
>> Cc: Chris Zankel <chris@zankel.net>
>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   lib/Kconfig.debug | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index ed4a31e34098..afad11e57d6b 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -346,7 +346,7 @@ config FRAME_WARN
>>          int "Warn for stack frames larger than"
>>          range 0 8192
>>          default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>> -       default 1536 if (!64BIT && PARISC)
>> +       default 1536 if (!64BIT && (PARISC || XTENSA))
>>          default 1024 if (!64BIT && !PARISC)
> 
> Shouldn't that line also be changed to
>    default 1024 if (!64BIT && !(PARISC || XTENSA))
> ?

I could do that, but I tested it and it looks like the evaluation
is top-down, so it didn't seem necessary or useful. For example,
the default value is 2048 for 32-bit systems (such as arm, riscv32,
or i386) if GCC_PLUGIN_LATENT_ENTROPY is enabled, even though the
line with the default of 1024 matches as well.

Thanks,
Guenter

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Max Filippov]

On Sat, Sep 11, 2021 at 9:02 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 9/11/21 8:05 PM, Max Filippov wrote:
> > On Sat, Sep 11, 2021 at 7:52 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> xtensa frame size is larger than the frame size for almost all other
> >> architectures. This results in more than 50 "the frame size of <n> is
> >> larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.
> >>
> >> Increase frame size for xtensa to 1536 bytes to avoid compile errors
> >> due to frame size limits.
> >>
> >> Cc: Chris Zankel <chris@zankel.net>
> >> Cc: Max Filippov <jcmvbkbc@gmail.com>
> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >> ---
> >>   lib/Kconfig.debug | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> >> index ed4a31e34098..afad11e57d6b 100644
> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -346,7 +346,7 @@ config FRAME_WARN
> >>          int "Warn for stack frames larger than"
> >>          range 0 8192
> >>          default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> >> -       default 1536 if (!64BIT && PARISC)
> >> +       default 1536 if (!64BIT && (PARISC || XTENSA))
> >>          default 1024 if (!64BIT && !PARISC)
> >
> > Shouldn't that line also be changed to
> >    default 1024 if (!64BIT && !(PARISC || XTENSA))
> > ?
>
> I could do that, but I tested it and it looks like the evaluation
> is top-down, so it didn't seem necessary or useful. For example,
> the default value is 2048 for 32-bit systems (such as arm, riscv32,
> or i386) if GCC_PLUGIN_LATENT_ENTROPY is enabled, even though the
> line with the default of 1024 matches as well.

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

RE: [PATCH] xtensa: Increase size of gcc stack frame check

[David Laight]

From: Guenter Roeck
> Sent: 12 September 2021 03:53
> 
> xtensa frame size is larger than the frame size for almost all other
> architectures. This results in more than 50 "the frame size of <n> is
> larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.
> 
> Increase frame size for xtensa to 1536 bytes to avoid compile errors
> due to frame size limits.

Have you done anything to check what happens at run-time?
I'd guess that the deepest stack use is inside printk() in
some obscure error path.

In reality all these 1k+ stack frames need killing
rather than the limit for the compiler warning increased.

While it may be sensible for a system call entry function
so allocate a reasonable size buffer on stack (as poll()
and sendmsg() probably do) allocating big buffers way
down the call stack could easily cause stack overflow.
Even a 1k stack frame is huge.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Guenter Roeck]

On 9/13/21 8:57 AM, David Laight wrote:
> From: Guenter Roeck
>> Sent: 12 September 2021 03:53
>>
>> xtensa frame size is larger than the frame size for almost all other
>> architectures. This results in more than 50 "the frame size of <n> is
>> larger than 1024 bytes" errors when trying to build xtensa:allmodconfig.
>>
>> Increase frame size for xtensa to 1536 bytes to avoid compile errors
>> due to frame size limits.
> 
> Have you done anything to check what happens at run-time?
> I'd guess that the deepest stack use is inside printk() in
> some obscure error path.
> 
> In reality all these 1k+ stack frames need killing
> rather than the limit for the compiler warning increased.
> 
> While it may be sensible for a system call entry function
> so allocate a reasonable size buffer on stack (as poll()
> and sendmsg() probably do) allocating big buffers way
> down the call stack could easily cause stack overflow.
> Even a 1k stack frame is huge.
> 

The functions I checked typically have pretty large local data
(like, more than 700-800 bytes). The errors are only observed
with xtensa:allmodconfig test builds. While it may be arguable
if those functions really need that much data on the stack, it
is unreasonable to assume that all those errors (again, more
than 50) are ever going to get fixed, especially since the errors
are only seen with xtensa and not with any other architecture
(including parisc; setting a stack limit of 1024 works just fine
with that architecture, at least with gcc 11.x). So the realistic
options are:

1) accept this or a similar patch
2) stop build testing xtensa:allmodconfig
3) Manually disable CONFIG_WERROR when test building xtensa:allmodconfig

As it looks like, I'll probably implement option 3) in my test builds.
I planned to start doing that around v5.15-rc4/rc5, but I may do it
earlier if it is becoming obvious that the now-errors won't get fixed.

Guenter

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Max Filippov]

On Mon, Sep 13, 2021 at 9:11 AM Guenter Roeck <linux@roeck-us.net> wrote:
> The functions I checked typically have pretty large local data
> (like, more than 700-800 bytes). The errors are only observed
> with xtensa:allmodconfig test builds. While it may be arguable
> if those functions really need that much data on the stack, it
> is unreasonable to assume that all those errors (again, more
> than 50) are ever going to get fixed, especially since the errors
> are only seen with xtensa and not with any other architecture

That's not what I observe. If I build allmodconfig on v5.15-rc1
for arm with gcc-8.3 I get 69 of them.

> (including parisc; setting a stack limit of 1024 works just fine
> with that architecture, at least with gcc 11.x). So the realistic

This comparison is a bit biased because allmodconfig on xtensa
enables KASAN which is not supported by the parisc. Disabling
KASAN roughly halves the size of stack frames for a few
instances that I've checked.

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Guenter Roeck]

On 9/14/21 12:04 AM, Max Filippov wrote:
> On Mon, Sep 13, 2021 at 9:11 AM Guenter Roeck <linux@roeck-us.net> wrote:
>> The functions I checked typically have pretty large local data
>> (like, more than 700-800 bytes). The errors are only observed
>> with xtensa:allmodconfig test builds. While it may be arguable
>> if those functions really need that much data on the stack, it
>> is unreasonable to assume that all those errors (again, more
>> than 50) are ever going to get fixed, especially since the errors
>> are only seen with xtensa and not with any other architecture
> 
> That's not what I observe. If I build allmodconfig on v5.15-rc1
> for arm with gcc-8.3 I get 69 of them.
> 

Interesting. I had used gcc 11.2. Anyway, arm:allmodconfig sets
the limit to 2048 for me, for both gcc 8.3 and 11.2, due to
GCC_PLUGIN_LATENT_ENTROPY=y. But you are right, if I disable
GCC_PLUGIN_LATENT_ENTROPY and set the frame size to 1024,
I get lots of frame size errors on arm as well.

>> (including parisc; setting a stack limit of 1024 works just fine
>> with that architecture, at least with gcc 11.x). So the realistic
> 
> This comparison is a bit biased because allmodconfig on xtensa
> enables KASAN which is not supported by the parisc. Disabling
> KASAN roughly halves the size of stack frames for a few
> instances that I've checked.
> 

It wasn't meant to be biased or unbiased, just a (surprising)
observation. Maybe there is some configuration in parisc that
is not enabled with allmodconfig which increases the frame size.

I still don't think that those stack limit warnings (now errors)
will ever get fixed. Which is the point I was trying to make,
and your observation that the stack frames are really that large
because of KASAN just makes that argument stronger.

Other than that, it is not my call to make that to do with this
patch. If you think that it is inappropriate, by all means
reject it.

Thanks,
Guenter

Re: [PATCH] xtensa: Increase size of gcc stack frame check

[Max Filippov]

On Tue, Sep 14, 2021 at 7:02 AM Guenter Roeck <linux@roeck-us.net> wrote:
> I still don't think that those stack limit warnings (now errors)
> will ever get fixed. Which is the point I was trying to make,
> and your observation that the stack frames are really that large
> because of KASAN just makes that argument stronger.
>
> Other than that, it is not my call to make that to do with this
> patch. If you think that it is inappropriate, by all means
> reject it.

I like it, also for the reason that with KASAN enabled
kernel stack size on xtensa is 4 times bigger.

-- 
Thanks.
-- Max