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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
