Re: kunit stack usage, was: pmwg-ci report v5.5-rc4-147-gc62d43442481

From: Brendan Higgins
Date: Wed Jan 08 2020 - 09:41:47 EST


+Stephen Boyd

On Tue, Jan 7, 2020 at 4:37 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Mon, Dec 30, 2019 at 6:16 PM PMWG CI <pmwg-ci@xxxxxxxxxx> wrote:
> >
> >
> > The error/warning: 1 drivers/base/test/property-entry-test.c:214:1: warning: the frame size of 3128 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> > ... was introduced by commit:
> >
> > commit c032ace71c29d513bf9df64ace1885fe5ff24981
> > Author: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > Date: Wed Dec 4 10:53:15 2019 -0800
> >
> > software node: add basic tests for property entries
>
> This problem is a result of the KUNIT_ASSERTION() definition that puts
> a local struct on the stack interacting badly with the structleak_plugin
> when CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is set in
> allmodconfig:
>
> pe_test_uint_arrays() contains a couple of larger variables, plus 41
> instances of KUNIT_EXPECT_*() or KUNIT_ASSERT_*(), each one
> of these adds its own copy of the structure, eventually exceeding
> the warning limit.

Uh oh, sorry about that.

> We can work around this locally by splitting up the largest four
> functions in this file (pe_test_uints, pe_test_uint_arrays, pe_test_strings,
> and pe_test_reference) into smaller functions that stay below the
> warning limit, but it would be nice to find a way for kunit to not
> use as much stack space. Any suggestions?

Agreed. I have a couple ideas. The grossest idea, which I am guessing
most people won't like is to go back to the stream builder method,
basically build the message as we go on the heap; however, the current
method was created specifically to not do that which much help from
Stephen (CC'ed), so probably not want we want to do.

Another idea, the struct just exists to pack up data and ship it off
to a function which handles the expectation/assertion. Consequently,
the struct is only used inside the expectation/assertion macro; it is
not needed before the macro block and is not needed after it. So could
we maybe make some kind of expectation/assertion union so that we know
they are all the same size, and then somehow tag the stack allocation
so that we only ever have one copy in a stack frame? I am not sure if
that kind of compiler magic exists. I guess one way to accomplish this
is to make a dummy function in KUnit whose job it is to call the unit
test function, which allocates the object, and then calls the unit
test function with a reference to the object allocation; then we could
just reuse that allocation and we can avoid making a bunch of
piecemeal heap allocations.

What do people think? Any other ideas?

Also, sorry in advance for delayed responses: I am on vacation until
the 13th and then I will be at LCA until the 18th.

Cheers!