Re: [libvirt] [PATCH] Raise the frame limit for tests

On Mon, Jan 22, 2018 at 01:47:24PM +0100, Michal Privoznik wrote: > On 01/22/2018 01:22 PM, Daniel P. Berrange wrote: >> On Mon, Jan 22, 2018 at 12:49:12PM +0100, Martin Kletzander wrote: >>> On Thu, Jan 18, 2018 at 10:16:55AM +0100, Ján Tomko wrote: >>>> After the latest CPU additions, the build fails with clang: >>>> cputest.c:905:1: error: stack frame size of 26136 bytes >>>>  in function 'mymain' [-Werror,-Wframe-larger-than=] >>>> >>>> Raise the relaxed limit which is used for tests. >>>> --- >>>> m4/virt-compile-warnings.m4 | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> Pushed as a build breaker fix >>>> >>>> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 >>>> index f18a08a8f..b9c974842 100644 >>>> --- a/m4/virt-compile-warnings.m4 >>>> +++ b/m4/virt-compile-warnings.m4 >>>> @@ -200,7 +200,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ >>>>     # but using 1024 bytes sized buffers (mostly for virStrerror) >>>>     # stops us from going down further >>>>     gl_WARN_ADD([-Wframe-larger-than=4096], >>>> [STRICT_FRAME_LIMIT_CFLAGS]) >>>> -    gl_WARN_ADD([-Wframe-larger-than=25600], >>>> [RELAXED_FRAME_LIMIT_CFLAGS]) >>>> +    gl_WARN_ADD([-Wframe-larger-than=32768], >>>> [RELAXED_FRAME_LIMIT_CFLAGS]) >>>> >>> >>> Remind me again why don't we do -Wno-frame-larger-than (or something >>> to that >>> effect) for tests?  Is it just because "We should fix it at some >>> point"?  I >>> can't really recall the reasoning behind that (and if it is still >>> valid) even >>> though I already asked for it. >> >> I don't think there's a strong reason, given the way we currently write >> tests with huge amounts of stack variables. >> >> For -Wframe-larger-than to be useful, we'd need to move all the big data >> blobs to be static, global variables. > Or simply use compiler that honours variable lifetime. If a variable is > defined only in a block, compiler should be able to just reuse the > stack. I mean for the following case: > > do { >  int x; > } while (0); > > do { >  int y; > } while (0); > > I don't see any compelling reason for compiler to reserve two ints on > the stack. Or if it does, count it as one when comparing agains > -Wframe-larger-than. > We can do that ourselves, even though it's not really great thing to do.  Just reset the one struct and reuse it.  I added it (and future research) as an idea to GSoC ideas.  Let's see if someone rewrites that.