On Mon, Jan 22, 2018 at 01:54:28PM +0100, 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.
Is it really worth the effort though? It is important for the core library
because we have a unimaginable set of code paths that are hard to validate,
so keeping stack use low is key to minimize risk fo stack exhaustion. In the
test suite, however, we have basically 1-3 call frames and stack exhaustion
is a non-issue - the test would merely crash & not have any bad consequences.
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|