Re: [PATCH] tracing: events_hist: avoid using excessive stack space

From: Arnd Bergmann
Date: Fri May 14 2021 - 17:15:27 EST


On Fri, May 14, 2021 at 10:19 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@xxxxxxxxxxxxxxxx> wrote:
>
> On Fri, May 14, 2021 at 7:16 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > On Fri, 14 May 2021 16:04:25 +0200
> > Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
> >
> > > From: Arnd Bergmann <arnd@xxxxxxxx>
> > >
> > > In some configurations, clang produces a warning about an overly large
> > > amount of stack space used in hist_trigger_print_key():
> > >
> > > kernel/trace/trace_events_hist.c:4594:13: error: stack frame size of 1248 bytes in function 'hist_trigger_print_key' [-Werror,-Wframe-larger-than=]
> > > static void hist_trigger_print_key(struct seq_file *m,
> > >
> > > Moving the 'str' variable into a more local scope in the two places
> > > where it gets used actually reduces the the used stack space here
> > > and gets it below the warning limit, because the compiler can now
> > > assume that it is safe to use the same stack slot that it has for
> > > the stack of any inline function.
> >
> > Thanks Arnd for the nice explanation of the rationale for this change.
> >
> > But I still find it too subtle to my liking that we need to move the
> > declaration like this (and duplicate it twice) for internal behavior of the
> > compiler (where it can't figure out itself by the use cases if it can
> > optimize the stack).
>
> Under which configurations this warning was observed wasn't specified,
> but I'd bet it's one of the sanitizers splitting this in two in order
> to tell which branch may have overflowed the buffer.

Right, I did not manage to find the exact conditions that trigger it.
I did *not* see it with KASAN enabled and I saw it with UBSAN both
on or off.

Arnd