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])
# Extra special flags
dnl -fstack-protector stuff passes gl_WARN_ADD with gcc
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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. > # Extra special flags > dnl -fstack-protector stuff passes gl_WARN_ADD with gcc >-- >2.13.6 > >-- >libvir-list mailing list >libvir-list@redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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.
>Michal
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jan 22, 2018 at 12:57:31PM +0000, Daniel P. Berrange wrote:
>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.
>
There are two points for this. 1) It can drive someone to start contributing to
libvirt by starting off easily, and 2) it can then help with assessing ways how
we can make the library frame sizes smaller.
So if there is no point in this for tests, as you said, we're back to my
original question. Why to have this when we just randomly increase it?
>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 :|
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jan 22, 2018 at 02:28:42PM +0100, Martin Kletzander wrote:
> On Mon, Jan 22, 2018 at 12:57:31PM +0000, Daniel P. Berrange wrote:
> > 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.
> >
>
> There are two points for this. 1) It can drive someone to start contributing to
> libvirt by starting off easily, and 2) it can then help with assessing ways how
> we can make the library frame sizes smaller.
>
> So if there is no point in this for tests, as you said, we're back to my
> original question. Why to have this when we just randomly increase it?
As I said, I don't see any real point it in - we might as well just use
the -Wno-frame-larger-than flag.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/22/2018 02:34 PM, Daniel P. Berrange wrote:
> On Mon, Jan 22, 2018 at 02:28:42PM +0100, Martin Kletzander wrote:
>> On Mon, Jan 22, 2018 at 12:57:31PM +0000, Daniel P. Berrange wrote:
>>> 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.
>>>
>>
>> There are two points for this. 1) It can drive someone to start contributing to
>> libvirt by starting off easily, and 2) it can then help with assessing ways how
>> we can make the library frame sizes smaller.
>>
>> So if there is no point in this for tests, as you said, we're back to my
>> original question. Why to have this when we just randomly increase it?
>
> As I said, I don't see any real point it in - we might as well just use
> the -Wno-frame-larger-than flag.
>
Agreed. tests/ shouldn't use -Wframe-lager-than. Nor examples/ perhaps
(although those are so small that they are never going to hit the limit).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jan 22, 2018 at 02:01:27PM +0100, Michal Privoznik wrote:
>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).
>
Really? Well, looks like you're not the only one, so feel free to remove that.
I just thought that would be a really easy starting point for someone.
>Michal
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.