staging: libxl compile error in libxl__domain_save

staging: libxl compile error in libxl__domain_save

[Olaf Hering]

Current staging fails to compile:

[  276s] libxl_dom_save.c: In function 'libxl__domain_save':
[  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function

It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
errors?

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Andrew Cooper]

On 27/02/2016 08:14, Olaf Hering wrote:
> Current staging fails to compile:
>
> [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
>
> It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> errors?

-Wmaybe-uninitalised is quite distro-specific as to whether it is
enabled by default or not.

Travis being ubuntu-based, it appears not. 
https://travis-ci.org/xen-project/xen/builds/112092866

I am tempted to suggest explicitly enabling it.  I have lost count of
how many times I have found the build legitimately broken on CentOS, due
to their default of -Wmaybe-initialised.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Wei Liu]

On Sat, Feb 27, 2016 at 09:14:26AM +0100, Olaf Hering wrote:
> 
> Current staging fails to compile:
> 
> [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function

Thanks for the report. This looks like a simple enough bug to fix. I
will prepare a patch shortly.

Wei.

> 
> It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> errors?
> 
> Olaf
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Wei Liu]

On Sat, Feb 27, 2016 at 01:47:08PM +0000, Wei Liu wrote:
> On Sat, Feb 27, 2016 at 09:14:26AM +0100, Olaf Hering wrote:
> > 
> > Current staging fails to compile:
> > 
> > [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> > [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
> 
> Thanks for the report. This looks like a simple enough bug to fix. I
> will prepare a patch shortly.
> 

Hrm... Strange enough when I add -Wmaybe-uninitialized and
-Wuninitialized to my build environment, gcc doesn't complain about this
one.

Wei.

> Wei.
> 
> > 
> > It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> > 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> > errors?
> > 
> > Olaf
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Wei Liu]

On Sat, Feb 27, 2016 at 12:35:17PM +0000, Andrew Cooper wrote:
> On 27/02/2016 08:14, Olaf Hering wrote:
> > Current staging fails to compile:
> >
> > [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> > [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
> >
> > It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> > 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> > errors?
> 
> -Wmaybe-uninitalised is quite distro-specific as to whether it is
> enabled by default or not.
> 
> Travis being ubuntu-based, it appears not. 
> https://travis-ci.org/xen-project/xen/builds/112092866
> 
> I am tempted to suggest explicitly enabling it.  I have lost count of
> how many times I have found the build legitimately broken on CentOS, due
> to their default of -Wmaybe-initialised.
> 

It appears that libxl has -Wall specified. According to Debian gcc
manual, -Wmaybe-uninitialized is enabled by -Wall.

And fwiw even if I manually added that option to libxl build, gcc
doesn't complain.


Wei.

> ~Andrew
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Olaf Hering]

On Sat, Feb 27, Wei Liu wrote:

> And fwiw even if I manually added that option to libxl build, gcc
> doesn't complain.

Well, Debian is appearently not the first choice anymore...

My cmdline is like this, fails with gcc4.3 ... 5.3:

 gcc  -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall
-Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable
-Wno-unused-local-typedefs   -O0 -g3
-D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF
.libxl_dom_save.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-fno-optimize-sibling-calls -fmessage-length=0 -grecord-gcc-switches -O2 -Wall
-D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
-fasynchronous-unwind-tables -g -Werror -Wno-format-zero-length
-Wmissing-declarations -Wno-declaration-after-statement -Wformat-nonliteral -I.
-fPIC -pthread
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/toollog/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/evtchn/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libxc/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/toollog/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/foreignmemory/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-D__XEN_TOOLS__
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libxc/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/evtchn/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/libs/foreignmemory/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/xenstore/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/blktap2/control
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/blktap2/include
-I/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/include
-I/usr/include/libnl3  -Wshadow -include
/home/abuild/rpmbuild/BUILD/xen-4.7.20160226T190133.3dd926a/non-dbg/tools/libxl/../../tools/config.h
-c -o libxl_dom_save.o libxl_dom_save.c

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]

On 2/27/16 2:14 AM, Olaf Hering wrote:
> 
> Current staging fails to compile:
> 
> [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
> 
> It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> errors?
> 

Travis currently wouldn't catch this because I've got tools disabled
from building. The reason for this is because the configure script fails
to properly find Python due to problems with how AC_CHECK_LIB is called.
Down the road this doesn't matter because we never use that output from
the configure script and instead utilize setuptools to build the
extensions which do linking correctly.

The relevant patch is here and still waiting for an ACK:

http://lists.xen.org/archives/html/xen-devel/2016-02/msg02715.html

Once that gets merged and I can enable tools building on Travis then yes
it would catch it.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Olaf Hering]

On Sat, Feb 27, Olaf Hering wrote:

> -O2 -Wall
> -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables

I think the cause is the lack of a "maintainer mode", which should
enable realworld CFLAGS. Either a new configure knob gets added (and
used!) or the existing EXTRA_CFLAGS_* have to be used by commiters. See
"grep EXTRA_CFLAGS INSTALL".

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Andrew Cooper]

On 28/02/16 02:01, Doug Goldstein wrote:
> On 2/27/16 2:14 AM, Olaf Hering wrote:
>> Current staging fails to compile:
>>
>> [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
>> [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
>>
>> It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
>> 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
>> errors?
>>
> Travis currently wouldn't catch this because I've got tools disabled
> from building.

Oops yes - of course.

>  The reason for this is because the configure script fails
> to properly find Python due to problems with how AC_CHECK_LIB is called.
> Down the road this doesn't matter because we never use that output from
> the configure script and instead utilize setuptools to build the
> extensions which do linking correctly.
>
> The relevant patch is here and still waiting for an ACK:
>
> http://lists.xen.org/archives/html/xen-devel/2016-02/msg02715.html
>
> Once that gets merged and I can enable tools building on Travis then yes
> it would catch it.

There is still the HVMLoader errno issue, and the Clang (__align8__
uint64_t) issue.

What about SeaBIOS?  Is that fixed in the 1.9.1 tag, or are there
further issues luring?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Wei Liu]

On Sun, Feb 28, 2016 at 10:39:17AM +0100, Olaf Hering wrote:
> On Sat, Feb 27, Olaf Hering wrote:
> 
> > -O2 -Wall
> > -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
> 
> I think the cause is the lack of a "maintainer mode", which should
> enable realworld CFLAGS. Either a new configure knob gets added (and
> used!) or the existing EXTRA_CFLAGS_* have to be used by commiters. See
> "grep EXTRA_CFLAGS INSTALL".
> 

If the current set of compiler flags is not good enough, we should
improve it. I'm afraid having a third set of maintainer mode flags that
nobody else uses is going to cause us more headache.

Wei.

> Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Andrew Cooper]

On 28/02/16 15:47, Wei Liu wrote:
> On Sun, Feb 28, 2016 at 10:39:17AM +0100, Olaf Hering wrote:
>> On Sat, Feb 27, Olaf Hering wrote:
>>
>>> -O2 -Wall
>>> -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
>> I think the cause is the lack of a "maintainer mode", which should
>> enable realworld CFLAGS. Either a new configure knob gets added (and
>> used!) or the existing EXTRA_CFLAGS_* have to be used by commiters. See
>> "grep EXTRA_CFLAGS INSTALL".
>>
> If the current set of compiler flags is not good enough, we should
> improve it. I'm afraid having a third set of maintainer mode flags that
> nobody else uses is going to cause us more headache.

We should be using system-provided CFLAGS/LDFLAGS for all userspace,
appending our own local ones as appropriate.  Anything else is (as
demonstrated here) utterly fragile.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 1189 bytes --]

On 2/28/16 8:10 AM, Andrew Cooper wrote:
> On 28/02/16 02:01, Doug Goldstein wrote:
>> On 2/27/16 2:14 AM, Olaf Hering wrote:
>> The relevant patch is here and still waiting for an ACK:
>>
>> http://lists.xen.org/archives/html/xen-devel/2016-02/msg02715.html
>>
>> Once that gets merged and I can enable tools building on Travis then yes
>> it would catch it.
> 
> There is still the HVMLoader errno issue, and the Clang (__align8__
> uint64_t) issue.
> 
> What about SeaBIOS?  Is that fixed in the 1.9.1 tag, or are there
> further issues luring?
> 
> ~Andrew
> 

I forgot about the HVMLoader errno issue which I'm hitting here. This
branch has the fix I mentioned waiting to get ACKed but its hitting the
HVMLoader issue.

https://travis-ci.org/cardoe/xen/jobs/112358926

As far as SeaBIOS goes, I'm using the system package. You unfortunately
have to make a symlink from /usr/include/asm ->
/usr/include/asm-generic, which is what Ubuntu does to build their
SeaBIOS package.


Unfortunately ARM fails to cross compile on any Ubuntu box I've got with
the same errors as Travis:

https://travis-ci.org/cardoe/xen/jobs/112358935

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Konrad Rzeszutek Wilk]

On Sun, Feb 28, 2016 at 02:10:16PM +0000, Andrew Cooper wrote:
> On 28/02/16 02:01, Doug Goldstein wrote:
> > On 2/27/16 2:14 AM, Olaf Hering wrote:
> >> Current staging fails to compile:
> >>
> >> [  276s] libxl_dom_save.c: In function 'libxl__domain_save':
> >> [  276s] libxl_dom_save.c:328:9: error: 'rc' may be used uninitialized in this function
> >>
> >> It was introduced between abf8824fe530bcf060c757596f68663c87546a6a and
> >> 3dd926a25d866364ce6d46c21f9ac05a82fa7ffb. Would travis catch such
> >> errors?
> >>
> > Travis currently wouldn't catch this because I've got tools disabled
> > from building.
> 
> Oops yes - of course.
> 
> >  The reason for this is because the configure script fails
> > to properly find Python due to problems with how AC_CHECK_LIB is called.
> > Down the road this doesn't matter because we never use that output from
> > the configure script and instead utilize setuptools to build the
> > extensions which do linking correctly.
> >
> > The relevant patch is here and still waiting for an ACK:
> >
> > http://lists.xen.org/archives/html/xen-devel/2016-02/msg02715.html
> >
> > Once that gets merged and I can enable tools building on Travis then yes
> > it would catch it.
> 
> There is still the HVMLoader errno issue, and the Clang (__align8__
> uint64_t) issue.
> 
> What about SeaBIOS?  Is that fixed in the 1.9.1 tag, or are there
> further issues luring?

1.9.1 tag is in, and so is the change in the Config.mk to use that.

> 
> ~Andrew
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Olaf Hering]

On Sun, Feb 28, Wei Liu wrote:

> If the current set of compiler flags is not good enough, we should
> improve it. I'm afraid having a third set of maintainer mode flags that
> nobody else uses is going to cause us more headache.

There is nothing wrong with the CFLAGS. They are perfect for developers.
But the commiters hopefully do some sort of compile test before doing a
push. And this compile test must include -O2 to enable enough
diagnostic to catch developer errors.

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Andrew Cooper]

On 29/02/16 06:54, Olaf Hering wrote:
> On Sun, Feb 28, Wei Liu wrote:
>
>> If the current set of compiler flags is not good enough, we should
>> improve it. I'm afraid having a third set of maintainer mode flags that
>> nobody else uses is going to cause us more headache.
> There is nothing wrong with the CFLAGS. They are perfect for developers.
> But the commiters hopefully do some sort of compile test before doing a
> push. And this compile test must include -O2 to enable enough
> diagnostic to catch developer errors.

 gcc  -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing
-std=gnu99 -Wall
-Wstrict-prototypes -Wdeclaration-after-statement
-Wno-unused-but-set-variable
-Wno-unused-local-typedefs   -O0 -g3
-D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF
.libxl_dom_save.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-fno-optimize-sibling-calls -fmessage-length=0 -grecord-gcc-switches -O2
-Wall
-D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables
-fasynchronous-unwind-tables -g -Werror -Wno-format-zero-length
-Wmissing-declarations -Wno-declaration-after-statement
-Wformat-nonliteral -I.
-fPIC -pthread

This set of options is very messy.  We have both an -O1, an -O0 and an
-O2, as well as three different -g's

Frankly, at no point ever should -O0 be used, even for debugging.  -Og
if available or -O1 if not.

In this case, the -O2, being latest, should take priority.  However, it
would be useful to identify which flags are coming from where, and see
if we usefully reduce them.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: staging: libxl compile error in libxl__domain_save

[Olaf Hering]

On Mon, Feb 29, Andrew Cooper wrote:

> This set of options is very messy.  We have both an -O1, an -O0 and an
> -O2, as well as three different -g's

-O1 is a global CFLAG, -O0 is appended in tools/, and -O2 comes from
RPM_OPT_FLAGS.

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel