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

On 01/22/2018 01:54 PM, Martin Kletzander wrote: > 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. I don't think that's a good idea. It's working around broken compiler. Just like the original patch (which we unfortunately have to have).

Michal -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list