[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] test: fix build errors with gcc 4.7.0 and -O0



On 04/05/2012 02:31 PM, Laine Stump wrote:
> When building on Fedora 17 (which uses gcc 4.7.0) with -O0 in CFLAGS,
> three of the tests failed to compile.
> 
> cputest.c and qemuxml2argvtest.c had non-static structs defined
> inside the macro that was being repeatedly invoked. Due to some so-far
> unidentified change in gcc, the stack space used by variables defined
> inside { } is not recovered/re-used when the block ends, so all these
> structs have become additive (this is the same problem worked around
> in commit cf57d345b). Fortunately, these two files could be fixed with
> a single line addition of "static" to the struct definition in the
> macro.
> 
> virnettlscontexttest.c was a bit different, though. The structs were
> all defined inline as static already, but for some reason now take up
> space on the stack. The only reasonable solution here was to move all
> of the struct definitions outside of the function.
> 
> In an ideal world, none of these changes should be necessary, but not
> knowing how long it will be until the gcc regressions are fixed, and
> since the code is just as correct after this patch as before, it makes
> sense to fix libvirt's build for -O0 while also reporting the gcc
> problem.

It would be nice to include a link to the gcc bug that we end up filing
before pushing this.

> +++ b/tests/virnettlscontexttest.c
> @@ -734,21 +734,6 @@ cleanup:
>      return ret;
>  }
>  
> -
> -static int
> -mymain(void)
> -{
> -    int ret = 0;
> -    if (asn1_array2tree(pkix_asn1_tab, &pkix_asn1, NULL) != ASN1_SUCCESS)
> -        abort();
> -
> -    gnutls_global_init();
> -
> -    privkey = testTLSLoadKey();
> -
> -    if (virFileWriteStr(keyfile, PRIVATE_KEY, 0600) < 0)
> -        return EXIT_FAILURE;
> -
>  # define DO_CTX_TEST(isServer, caReq, certReq, expectFail)              \
>      do {                                                                \
>          struct testTLSContextData data = {                              \

Did you try adding a 'static' here?

It looks like your patch is a reasonable workaround, but please give me
24 hours to see if I can reproduce the issue on my F17 VM and come up
with any alternative patch for virnettlsconttexttest.c that uses fewer
lines changed.  And hopefully the gcc folks will admit that they have
introduced a layout regression, and fix it (the code is still
technically correct, but any compiler that chews up unnecessary stack
space, especially when it didn't used to do so, is not optimal).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]