Random improvements to work around a build system issue

[libvirt PATCH 0/2] Random improvements to work around a build system issue

Posted by Tim Wiederhake 2 years, 7 months ago

This is an alternative to
https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.

When libvirt is build:
* with sanitizers enabled,
* buildtype explicitly set to "debug",
* on clang,

the build fails with:

    ../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616
    bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=]
    virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
    ^
    1 error generated.

    ../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312
    bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=]
    virDomainDefParseXML(xmlXPathContextPtr ctxt,
    ^
    1 error generated.

Note that this does not happen when "-Dbuildtype" is not specified, even
though "debug" is the default build type.

The patches in this series happen to make these errors go away.

Regards,
Tim

Tim Wiederhake (2):
  virDomainDefParseXML: Use automatic memory management
  virNWFilterRuleDefFixup: Replace macro with function

 src/conf/domain_conf.c   | 208 ++++++++++++++--------------
 src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++-------------------
 2 files changed, 245 insertions(+), 247 deletions(-)

-- 
2.31.1

Re: [libvirt PATCH 0/2] Random improvements to work around a build system issue

Posted by Michal Prívozník 2 years, 7 months ago

On 9/20/21 7:21 PM, Tim Wiederhake wrote:
> This is an alternative to
> https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.
> 
> When libvirt is build:
> * with sanitizers enabled,
> * buildtype explicitly set to "debug",
> * on clang,
> 
> the build fails with:
> 
>     ../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616
>     bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=]
>     virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
>     ^
>     1 error generated.
> 
>     ../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312
>     bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=]
>     virDomainDefParseXML(xmlXPathContextPtr ctxt,
>     ^
>     1 error generated.
> 
> Note that this does not happen when "-Dbuildtype" is not specified, even
> though "debug" is the default build type.
> 
> The patches in this series happen to make these errors go away.
> 
> Regards,
> Tim
> 
> Tim Wiederhake (2):
>   virDomainDefParseXML: Use automatic memory management
>   virNWFilterRuleDefFixup: Replace macro with function
> 
>  src/conf/domain_conf.c   | 208 ++++++++++++++--------------
>  src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++-------------------
>  2 files changed, 245 insertions(+), 247 deletions(-)
> 

Frankly, I don't understand how either of patches can size the frame
down, but they make sense regardless.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [libvirt PATCH 0/2] Random improvements to work around a build system issue

Posted by Daniel P. Berrangé 2 years, 7 months ago

On Thu, Sep 23, 2021 at 09:56:01AM +0200, Michal Prívozník wrote:
> On 9/20/21 7:21 PM, Tim Wiederhake wrote:
> > This is an alternative to
> > https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.
> > 
> > When libvirt is build:
> > * with sanitizers enabled,
> > * buildtype explicitly set to "debug",
> > * on clang,
> > 
> > the build fails with:
> > 
> >     ../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616
> >     bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=]
> >     virNWFilterRuleDefFixup(virNWFilterRuleDef *rule)
> >     ^
> >     1 error generated.
> > 
> >     ../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312
> >     bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=]
> >     virDomainDefParseXML(xmlXPathContextPtr ctxt,
> >     ^
> >     1 error generated.
> > 
> > Note that this does not happen when "-Dbuildtype" is not specified, even
> > though "debug" is the default build type.
> > 
> > The patches in this series happen to make these errors go away.
> > 
> > Regards,
> > Tim
> > 
> > Tim Wiederhake (2):
> >   virDomainDefParseXML: Use automatic memory management
> >   virNWFilterRuleDefFixup: Replace macro with function
> > 
> >  src/conf/domain_conf.c   | 208 ++++++++++++++--------------
> >  src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++-------------------
> >  2 files changed, 245 insertions(+), 247 deletions(-)
> > 
> 
> Frankly, I don't understand how either of patches can size the frame
> down, but they make sense regardless.

The first patch is fine, but I don't think the second is. It introduces
countless more pointless fnuction calls when a macro suffices.

We just need to increase the stack frame check limit for instrumented
builds. Enforcing stack frame only makes sense on normal builds when
stack usage is determinstic


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 :|