Re: [kvm-unit-tests PATCH] Always compile the kvm-unit-tests with -fno-common

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

 



On Wed, May 13, 2020 at 12:11:39PM +0200, Thomas Huth wrote:
> On 13/05/2020 12.05, Thomas Huth wrote:
> > On 12/05/2020 11.55, Thomas Huth wrote:
> >> The new GCC v10 uses -fno-common by default. To avoid that we commit
> >> code that declares global variables twice and thus fails to link with
> >> the latest version, we should also compile with -fno-common when using
> >> older versions of the compiler.
> >>
> >> Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
> >> ---
> >>  Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 754ed65..3ff2f91 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -49,7 +49,7 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> >>  cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> >>                > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> >>  
> >> -COMMON_CFLAGS += -g $(autodepend-flags) -fno-strict-aliasing
> >> +COMMON_CFLAGS += -g $(autodepend-flags) -fno-strict-aliasing -fno-common
> > 
> > Oh, wait, this breaks the non-x86 builds due to "extern-less" struct
> > auxinfo auxinfo in libauxinfo.h !
> > Drew, why isn't this declared in auxinfo.c instead?
> 
> Oh well, it's there ... so we're playing tricks with the linker here? I
> guess adding a "__attribute__((common, weak))" to auxinfo.h will be ok
> to fix this issue?

Right. In lib/auxinfo.h we have

/* No extern!  Define a common symbol.  */
struct auxinfo auxinfo;

Despite git-blame giving me credit for the 'No extern' comment (and the
missing 'extern'), I think Paolo made those changes when applying the
patch. I presume he did so to fix compilation on x86, for which I presume
the problem was that lib/argv.c references auxinfo, and that resulted
in an undefined symbol, since x86 doesn't link to auxinfo.o.

Unfortunately making the symbol weak won't work because if we add it
to the definition in auxinfo.h, then the linker prefers using its
own zero-initialized, global symbol. And, if we add the attribute
to the definition in auxinfo.c, then we still get the multiple
definition error.

So I'm not really sure what the best thing to do is. Maybe we
should just do this


diff --git a/lib/auxinfo.h b/lib/auxinfo.h
index 08b96f8ece4c..a46a1e6f6a62 100644
--- a/lib/auxinfo.h
+++ b/lib/auxinfo.h
@@ -13,7 +13,6 @@ struct auxinfo {
        unsigned long flags;
 };
 
-/* No extern!  Define a common symbol.  */
-struct auxinfo auxinfo;
+extern struct auxinfo auxinfo;
 #endif
 #endif
diff --git a/x86/Makefile.common b/x86/Makefile.common
index ab67ca0a9fda..2ea9c9f5bbcc 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -5,6 +5,7 @@ all: directories test_cases
 cflatobjs += lib/pci.o
 cflatobjs += lib/pci-edu.o
 cflatobjs += lib/alloc.o
+cflatobjs += lib/auxinfo.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc_page.o
 cflatobjs += lib/alloc_phys.o


Thanks,
drew




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux