xen-unstable stubdom build-failure when debug=n

xen-unstable stubdom build-failure when debug=n

[Sander Eikelenboom]

Hi,

Today i tried to do a debug=n build of xen-unstable and ran into the build error 
below.

--
Sander

gcc -mno-red-zone -O2 -fomit-frame-pointer  -m64 -mno-red-zone -fno-reorder-blocks -fno-asynchronous-unwind-tables -m64 -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -fno-stack-protector -fno-exceptions -DCONFIG_SPARSE_BSS -DCONFIG_QEMU_XS_ARGS -DCONFIG_PCIFRONT -DCONFIG_BLKFRONT -DCONFIG_NETFRONT -DCONFIG_KBDFRONT -DCONFIG_FBFRONT -DCONFIG_CONSFRONT -DCONFIG_XENBUS -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls -fno-stack-protector -fgnu89-inline -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline -O3 -D__INSIDE_MINIOS__ -m64 -mno-red-zone -fno-reorder-blocks -fno-asynchronous-unwind-tables -isystem /usr/src/new/xen-unstable/stubdom/../extras/mini-os/include -D__MINIOS__ -DHAVE_LIBC -isystem /usr/src/new/xen-unstable/stubdom/../extras/mini-os/include/posix -isystem /usr/src/new/xen-unstable/stubdom/../tools/xenstore  -isystem /usr/src/new/xen-unstable/stubdom/../extras/mini-os/include/x86 -isystem /usr/src/new/xen-unstable/stubdom/../extras/mini-os/include/x86/x86_64 -U __linux__ -U __FreeBSD__ -U __sun__ -nostdinc -isystem /usr/src/new/xen-unstable/stubdom/../extras/mini-os/include/posix -isystem /usr/src/new/xen-unstable/stubdom/cross-root-x86_64/x86_64-xen-elf/include -isystem /usr/lib/gcc/x86_64-linux-gnu/4.7/include -isystem /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/include -isystem /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/include/ipv4 -I/usr/src/new/xen-unstable/stubdom/include -I/usr/src/new/xen-unstable/stubdom/../xen/include -isystem /usr/src/new/xen-unstable/extras/mini-os/../../extras/mini-os/include -D__MINIOS__ -DHAVE_LIBC -isystem /usr/src/new/xen-unstable/extras/mini-os/../../extras/mini-os/include/posix -isystem /usr/src/new/xen-unstable/extras/mini-os/../../tools/xenstore -DHAVE_LWIP -isystem /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/include -isystem /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/include/ipv4 -D__XEN_INTERFACE_VERSION__=0x00030205  -isystem /usr/src/new/xen-unstable/extras/mini-os/../../extras/mini-os/include/x86 -isystem /usr/src/new/xen-unstable/extras/mini-os/../../extras/mini-os/include/x86/x86_64  -c -o /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.o /usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c: In function ‘dhcp_create_request’:
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]
cc1: all warnings being treated as errors
make[2]: *** [/usr/src/new/xen-unstable/stubdom/lwip-x86_64/src/core/dhcp.o] Error 1
make[2]: Leaving directory `/usr/src/new/xen-unstable/extras/mini-os'
make[1]: *** [ioemu-stubdom] Error 2
make[1]: Leaving directory `/usr/src/new/xen-unstable/stubdom'
make: *** [install-stubdom] Error 2


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

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Thu, 2014-07-17 at 10:27 +0200, Sander Eikelenboom wrote:
> Hi,
> 
> Today i tried to do a debug=n build of xen-unstable and ran into the build error 
> below.

Yes, I see something similar:

stubdom/../extras/mini-os/include/mini-os/tpm_tis.h: In function ‘tpm_tis_request_locality.part.6’:
tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]
cc1: all warnings being treated as errors

Not sure why it should be debug=n only though.

In the case I've got the code is:

        s->loc[locty].ints &= ~(val & INTERRUPTS_SUPPORTED);

where locty is a uint8_t, so how it can be *below* the bounds I'm not sure.

dhcp.c:1359 in my copy (assuming it is similar to yours) is
    dhcp->msg_out->chaddr[i] = (i < netif->hwaddr_len) ? netif->hwaddr[i] : 0/* pad byte*/;
where i is a u16_t. But this is an above array bounds error, so
presumably the compiler thinks it knows something about the size of
chaddr or hwaddr vs hwaddr_len.


I'm not seeing anything in the logs for mini-os or stubdom since 4.4.0
which cry out to me as anything related. Except perhaps:

commit e6e9178431725c369aeac117badc546edf18ab07
Author: Thomas Leonard <talex5@gmail.com>
Date:   Thu Jun 26 12:28:22 2014 +0100

    mini-os: made off_t type signed
    
    POSIX requires this.
    
    Signed-off-by: Thomas Leonard <talex5@gmail.com>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>
    Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

But there doesn't seem to be any size_t's involved at either site.

*Confused*

Ian.



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

Re: xen-unstable stubdom build-failure when debug=n

[Sander Eikelenboom]

Thursday, July 17, 2014, 4:13:11 PM, you wrote:


> On Thu, 2014-07-17 at 10:27 +0200, Sander Eikelenboom wrote:
>> Hi,
>> 
>> Today i tried to do a debug=n build of xen-unstable and ran into the build error 
>> below.

> Yes, I see something similar:

> stubdom/../extras/mini-os/include/mini-os/tpm_tis.h: In function ‘tpm_tis_request_locality.part.6’:
> tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]
> cc1: all warnings being treated as errors

> Not sure why it should be debug=n only though.

> In the case I've got the code is:

>         s->loc[locty].ints &= ~(val & INTERRUPTS_SUPPORTED);

> where locty is a uint8_t, so how it can be *below* the bounds I'm not sure.

> dhcp.c:1359 in my copy (assuming it is similar to yours) is
>     dhcp->msg_out->chaddr[i] = (i < netif->hwaddr_len) ? netif->hwaddr[i] : 0/* pad byte*/;
> where i is a u16_t. But this is an above array bounds error, so
> presumably the compiler thinks it knows something about the size of
> chaddr or hwaddr vs hwaddr_len.


> I'm not seeing anything in the logs for mini-os or stubdom since 4.4.0
> which cry out to me as anything related. Except perhaps:

> commit e6e9178431725c369aeac117badc546edf18ab07
> Author: Thomas Leonard <talex5@gmail.com>
> Date:   Thu Jun 26 12:28:22 2014 +0100

>     mini-os: made off_t type signed
>     
>     POSIX requires this.
>     
>     Signed-off-by: Thomas Leonard <talex5@gmail.com>
>     Acked-by: Ian Campbell <ian.campbell@citrix.com>
>     Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> But there doesn't seem to be any size_t's involved at either site.

> *Confused*

Hrmmm i only changed 2 things that made it build .. 
1) debug=n to debug=y
2) implicit:
        - the first build was after a "make mrproper", so that destroyed and redownloaded all git sub repo stuff
        - the second build was after a "make clean" after the first build (and the change from debug=n and debug=y)

So it could also be it doesn't build on the first build due too an ordering 
issue of something that doesn't get cleaned by a make clean ?


> Ian.





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

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Thu, 2014-07-17 at 16:25 +0200, Sander Eikelenboom wrote:
> Thursday, July 17, 2014, 4:13:11 PM, you wrote:
> 
> 
> > On Thu, 2014-07-17 at 10:27 +0200, Sander Eikelenboom wrote:
> >> Hi,
> >> 
> >> Today i tried to do a debug=n build of xen-unstable and ran into the build error 
> >> below.
> 
> > Yes, I see something similar:
> 
> > stubdom/../extras/mini-os/include/mini-os/tpm_tis.h: In function ‘tpm_tis_request_locality.part.6’:
> > tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]
> > cc1: all warnings being treated as errors
> 
> > Not sure why it should be debug=n only though.
> 
> > In the case I've got the code is:
> 
> >         s->loc[locty].ints &= ~(val & INTERRUPTS_SUPPORTED);
> 
> > where locty is a uint8_t, so how it can be *below* the bounds I'm not sure.
> 
> > dhcp.c:1359 in my copy (assuming it is similar to yours) is
> >     dhcp->msg_out->chaddr[i] = (i < netif->hwaddr_len) ? netif->hwaddr[i] : 0/* pad byte*/;
> > where i is a u16_t. But this is an above array bounds error, so
> > presumably the compiler thinks it knows something about the size of
> > chaddr or hwaddr vs hwaddr_len.
> 
> 
> > I'm not seeing anything in the logs for mini-os or stubdom since 4.4.0
> > which cry out to me as anything related. Except perhaps:
> 
> > commit e6e9178431725c369aeac117badc546edf18ab07
> > Author: Thomas Leonard <talex5@gmail.com>
> > Date:   Thu Jun 26 12:28:22 2014 +0100
> 
> >     mini-os: made off_t type signed
> >     
> >     POSIX requires this.
> >     
> >     Signed-off-by: Thomas Leonard <talex5@gmail.com>
> >     Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >     Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> > But there doesn't seem to be any size_t's involved at either site.
> 
> > *Confused*
> 
> Hrmmm i only changed 2 things that made it build .. 
> 1) debug=n to debug=y
> 2) implicit:
>         - the first build was after a "make mrproper", so that destroyed and redownloaded all git sub repo stuff
>         - the second build was after a "make clean" after the first build (and the change from debug=n and debug=y)
> 
> So it could also be it doesn't build on the first build due too an ordering 
> issue of something that doesn't get cleaned by a make clean ?

If that were the case then I'd expect
        git clean -ffffdqx && ./configure && make debug=n ; make debug=n
to work as well, which it doesn't seem to.

My guest is that turning off debug increases the optimisation level
which somehow makes gcc decide this code is now wrong.

Ian.



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

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Ian Campbell wrote:

> If that were the case then I'd expect
>         git clean -ffffdqx && ./configure && make debug=n ; make debug=n
> to work as well, which it doesn't seem to.
> 
> My guest is that turning off debug increases the optimisation level
> which somehow makes gcc decide this code is now wrong.

How does stubdom the build tests anyway?

I see "... -Wall -Werror -Wno-unused-paramters ... -Wextra ...", and
stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:52 fails to compile.

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Mon, 2014-07-21 at 18:21 +0200, Olaf Hering wrote:
> On Mon, Jul 21, Ian Campbell wrote:
> 
> > If that were the case then I'd expect
> >         git clean -ffffdqx && ./configure && make debug=n ; make debug=n
> > to work as well, which it doesn't seem to.
> > 
> > My guest is that turning off debug increases the optimisation level
> > which somehow makes gcc decide this code is now wrong.
> 
> How does stubdom the build tests anyway?

Pardon?

Are you asking if osstest builds stubdoms? I think so and
http://www.chiark.greenend.org.uk/~xensrcts/logs/29421/build-amd64/5.ts-xen-build.log seems to contain evidence of it happening.

I think there is a certain amount of dependence on the gcc version in
this stuff. osstest uses gcc 4.7.2 from Debian 7.0 (Wheezy, current
stable)

Ian.
> I see "... -Wall -Werror -Wno-unused-paramters ... -Wextra ...", and
> stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:52 fails to compile.
> 
> Olaf
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Ian Campbell wrote:

> I think there is a certain amount of dependence on the gcc version in
> this stuff. osstest uses gcc 4.7.2 from Debian 7.0 (Wheezy, current
> stable)

Would it be much hassle to run a SLES11/SLES12 VM for automated compile
tests of the staging branch? I think that will catch some of the errors
early.

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Olaf Hering wrote:

> On Mon, Jul 21, Ian Campbell wrote:
> 
> > If that were the case then I'd expect
> >         git clean -ffffdqx && ./configure && make debug=n ; make debug=n
> > to work as well, which it doesn't seem to.
> > 
> > My guest is that turning off debug increases the optimisation level
> > which somehow makes gcc decide this code is now wrong.
> 
> How does stubdom the build tests anyway?

Here is the actual log:

[  6%] Building C object tpm/CMakeFiles/tpm.dir/tpm_credentials.o
cd /home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/build/tpm && /usr/bin/gcc   -std=c99 -DTPM_NO_EXTERN -isystem /home/olaf/work/github/olafhering/xen/stubdom/../extras/mini-os/include -D__MINIOS__ -DHAVE_LIBC -isystem /home/olaf/work/github/olafhering/xen/stubdom/../extras/mini-os/include/posix -isystem /home/olaf/work/github/olafhering/xen/stubdom/../tools/xenstore  -isystem /home/olaf/work/github/olafhering/xen/stubdom/../extras/mini-os/include/x86 -isystem /home/olaf/work/github/olafhering/xen/stubdom/../extras/mini-os/include/x86/x86_64 -U __linux__ -U __FreeBSD__ -U __sun__ -nostdinc -isystem /home/olaf/work/github/olafhering/xen/stubdom/../extras/mini-os/include/posix -isystem /home/olaf/work/github/olafhering/xen/stubdom/cross-root-x86_64/x86_64-xen-elf/include -i
 system /usr/lib64/gcc/x86_64-suse-linux/4.3/include -isystem /home/olaf/work/github/olafhering/xen/stubdom/lwip-x86_64/src/include -isystem /home/olaf/work/github/olafhering/xen/stubdom/lwip-x86_64/src/include/ipv4 -I/home/olaf/work/github/olafhering/xen/stubdom/include -I/home/olaf/work/github/olafhering/xen/stubdom/../xen/include -mno-red-zone -O2 -O1 -fomit-frame-pointer  -m64 -mno-red-zone -fno-reorder-blocks -fno-asynchronous-unwind-tables -m64 -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement   -fno-stack-protector -fno-exceptions -Wno-declaration-after-statement -I/opt/local/include -I/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64 -I/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/build   -Wall -Werror -Wno-
 unused-parameter -Wpointer-arith -Wcast-align -Wwrite-strings -Wextra -o CMakeFiles/tpm.dir/tpm_credentials.o   -c /home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c
cc1: warnings being treated as errors
/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c: In function 'TPM_CreateEndorsementKeyPair':
/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:52: error: unused parameter 'antiReplay'
/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:53: error: unused parameter 'keyInfo'
/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:54: error: unused parameter 'pubEndorsementKey'
/home/olaf/work/github/olafhering/xen/stubdom/tpm_emulator-x86_64/tpm/tpm_credentials.c:55: error: unused parameter 'checksum'
make[5]: *** [tpm/CMakeFiles/tpm.dir/tpm_credentials.o] Error 1


Why does that only fail for me?
Is "-Wall -Werror -Wextra" different in sles11?

Looks like it should fail everywhere:
stubdom/tpm_emulator-x86_64/CMakeLists.txt
 43 add_definitions(-Wall -Werror -Wno-unused-parameter -Wpointer-arith -Wcast-align -Wwrite-strings)
 44 if("${CMAKE_SYSTEM}" MATCHES "Linux")
 45     add_definitions(-Wextra)
 46 endif()

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Olaf Hering wrote:

> Is "-Wall -Werror -Wextra" different in sles11?

Looks like '-Wall -Werror -Wextra -Wno-unused-parameter' behaves
different than '-Wall -Werror -Wno-unused-parameter -Wextra'.
I think for the time being the -Wextra should be patched out.

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Mon, 2014-07-21 at 18:43 +0200, Olaf Hering wrote:
> On Mon, Jul 21, Olaf Hering wrote:
> 
> > Is "-Wall -Werror -Wextra" different in sles11?
> 
> Looks like '-Wall -Werror -Wextra -Wno-unused-parameter' behaves
> different than '-Wall -Werror -Wno-unused-parameter -Wextra'.

If Wextra enables -Wunused-parameter than that is what I would expect.
Ordering matters there.

> I think for the time being the -Wextra should be patched out.

xen.git$ git grep Wextra
xen.git$

Are you sure that isn't something you've added locally (or perhaps
your .src.rpm).

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Ian Campbell wrote:

> Are you sure that isn't something you've added locally (or perhaps
> your .src.rpm).

Its in the stubdom downloads, tpm_manager.tar.gz
I will see what else needs changing, then I will propose a patch.


Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Mon, 2014-07-21 at 17:13 +0100, Ian Campbell wrote:

> My guest is that turning off debug increases the optimisation level
> which somehow makes gcc decide this code is now wrong.

Well, I was right, but not how I thought...

Turns out there are two tpm_tis.c's in our source base and I was looking
at the wrong one ;-)

The code in extras/mini-os/tpm_tis.c rather than
stubdom/ioemu/hw/tpm_tis.c does look more like it might plausibly
generate the error I'm seeing this is:

tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]

Line 618 does appear to include the use of a signed variable as an array
offset, but I'm not sure how or why gcc is proving that it is negative.

The code in question appears to be identical in 4.4.0 . I'm not sure
what has changed.

Perhaps Daniel (CCd) has some idea what is going on here.

There's still the question of the error Sander is seeing which is
stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]

Sander, did this used to work? If so can you identify when the
regression occurred?

Ian.

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Ian Campbell wrote:

> tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]
> 
> Line 618 does appear to include the use of a signed variable as an array
> offset, but I'm not sure how or why gcc is proving that it is negative.

It can not prove that tpm->locality remains positive.

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Daniel De Graaf]

On 07/21/2014 01:43 PM, Olaf Hering wrote:
> On Mon, Jul 21, Ian Campbell wrote:
>
>> tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]
>>
>> Line 618 does appear to include the use of a signed variable as an array
>> offset, but I'm not sure how or why gcc is proving that it is negative.
>
> It can not prove that tpm->locality remains positive.
>
> Olaf

In that case, shouldn't the error be phrased less severely (something like "might be
below array bounds")?  Anyway, the actual value is checked by locality_enabled
prior to the array access, and that check is effectively (0x1F & (1 << locality)),
limiting the actual value to 0..4.  I can't reproduce the warning with my version
of GCC, so I'm only looking at the code, but it looks like GCC is being overly
conservative after not detecting the range limit and this is just a false positive.

-- 
Daniel De Graaf
National Security Agency

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 21, Daniel De Graaf wrote:

> In that case, shouldn't the error be phrased less severely (something like "might be
> below array bounds")?  Anyway, the actual value is checked by locality_enabled
> prior to the array access, and that check is effectively (0x1F & (1 << locality)),
> limiting the actual value to 0..4.  I can't reproduce the warning with my version
> of GCC, so I'm only looking at the code, but it looks like GCC is being overly
> conservative after not detecting the range limit and this is just a false positive.

So how should we proceed here? Adjust the code to use unsigned array
indices, or drop support for gcc versions as shipped with sle11 (gcc 4.3)?
Of adjust the code like this:

diff --git a/extras/mini-os/tpm_tis.c b/extras/mini-os/tpm_tis.c
index e8ca69f..dc4134a 100644
--- a/extras/mini-os/tpm_tis.c
+++ b/extras/mini-os/tpm_tis.c
@@ -611,7 +611,7 @@ s_time_t tpm_calc_ordinal_duration(struct tpm_chip *chip,
 
 
 static int locality_enabled(struct tpm_chip* tpm, int l) {
-   return tpm->enabled_localities & (1 << l);
+   return l >= 0 && tpm->enabled_localities & (1 << l);
 }
 
 static int check_locality(struct tpm_chip* tpm, int l) {

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Sander Eikelenboom]

Monday, July 21, 2014, 6:24:33 PM, you wrote:

> On Mon, 2014-07-21 at 17:13 +0100, Ian Campbell wrote:

>> My guest is that turning off debug increases the optimisation level
>> which somehow makes gcc decide this code is now wrong.

> Well, I was right, but not how I thought...

> Turns out there are two tpm_tis.c's in our source base and I was looking
> at the wrong one ;-)

> The code in extras/mini-os/tpm_tis.c rather than
> stubdom/ioemu/hw/tpm_tis.c does look more like it might plausibly
> generate the error I'm seeing this is:

> tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]

> Line 618 does appear to include the use of a signed variable as an array
> offset, but I'm not sure how or why gcc is proving that it is negative.

> The code in question appears to be identical in 4.4.0 . I'm not sure
> what has changed.

> Perhaps Daniel (CCd) has some idea what is going on here.

> There's still the question of the error Sander is seeing which is
> stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]

> Sander, did this used to work? If so can you identify when the
> regression occurred?

Hmm it's a pretty large window .. (essentially 4.4.0-release till now),
it was only due to Konrad's reporting of the apparent major performance regression that
ended up being a difference between debug=y and debug=n build, that made me wondering if
i would notice that effect .. so i tried a debug=n build.

If the build-test-system still has some slack, perhaps add a (weekly or only on a push) test that does a debug=n build ?
(instead of waiting for the end of the release cycle and somewhere in the RC's to fixup the things that
happen to turn up with debug=n (if my memory doesn't fail me too much, this has happened before)

(BTW i also use Debian Wheezy)
--
Sander

> Ian.

Re: xen-unstable stubdom build-failure when debug=n

[Sander Eikelenboom]

Tuesday, July 22, 2014, 9:11:06 AM, you wrote:


> Monday, July 21, 2014, 6:24:33 PM, you wrote:

>> On Mon, 2014-07-21 at 17:13 +0100, Ian Campbell wrote:

>>> My guest is that turning off debug increases the optimisation level
>>> which somehow makes gcc decide this code is now wrong.

>> Well, I was right, but not how I thought...

>> Turns out there are two tpm_tis.c's in our source base and I was looking
>> at the wrong one ;-)

>> The code in extras/mini-os/tpm_tis.c rather than
>> stubdom/ioemu/hw/tpm_tis.c does look more like it might plausibly
>> generate the error I'm seeing this is:

>> tpm_tis.c:618:71: error: array subscript is below array bounds [-Werror=array-bounds]

>> Line 618 does appear to include the use of a signed variable as an array
>> offset, but I'm not sure how or why gcc is proving that it is negative.

>> The code in question appears to be identical in 4.4.0 . I'm not sure
>> what has changed.

>> Perhaps Daniel (CCd) has some idea what is going on here.

>> There's still the question of the error Sander is seeing which is
>> stubdom/lwip-x86_64/src/core/dhcp.c:1359:71: error: array subscript is above array bounds [-Werror=array-bounds]

>> Sander, did this used to work? If so can you identify when the
>> regression occurred?

> Hmm it's a pretty large window .. (essentially 4.4.0-release till now),
> it was only due to Konrad's reporting of the apparent major performance regression that
> ended up being a difference between debug=y and debug=n build, that made me wondering if
> i would notice that effect .. so i tried a debug=n build.

> If the build-test-system still has some slack, perhaps add a (weekly or only on a push) test that does a debug=n build ?
> (instead of waiting for the end of the release cycle and somewhere in the RC's to fixup the things that
> happen to turn up with debug=n (if my memory doesn't fail me too much, this has happened before)

> (BTW i also use Debian Wheezy)
> --
> Sander

>> Ian.

Hi Ian,

Just tested some more with a strange outcome:

When doing:
~# git checkout  RELEASE-4.4.0
~# make mrproper && ./configure && make debug=n

It also fails with the error in dhcp i mentioned before.

However if i do:
~# git checkout  RELEASE-4.4.0
~# make mrproper && ./configure && make

It builds fine .. (and release-4.4.0 should have the implicit debug=n)
So what is different when explicitly giving debug=n ?
--
Sander

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Sat, 2014-07-26 at 17:14 +0200, Sander Eikelenboom wrote:
> Just tested some more with a strange outcome:
> 
> When doing:
> ~# git checkout  RELEASE-4.4.0
> ~# make mrproper && ./configure && make debug=n
> 
> It also fails with the error in dhcp i mentioned before.
> 
> However if i do:
> ~# git checkout  RELEASE-4.4.0
> ~# make mrproper && ./configure && make
> 
> It builds fine .. (and release-4.4.0 should have the implicit debug=n)
> So what is different when explicitly giving debug=n ?

The only thing I can thing of would be that specifying debug= on the
make command line makes it enter the environment or MAKEFLAGS or
something, whereas setting it in the Makefile or .config etc doesn't.
(Perhaps e.g. "export debug" in Config.mk would make these two behave
the same?)

Anyway, that having happened the debug could then filter down and get
picked up by the configure or build system of some sub-component in a
way we aren't expecting.

That's all 100% speculation though.

Ian.

Re: xen-unstable stubdom build-failure when debug=n

[Sander Eikelenboom]

Monday, July 28, 2014, 11:09:35 AM, you wrote:

> On Sat, 2014-07-26 at 17:14 +0200, Sander Eikelenboom wrote:
>> Just tested some more with a strange outcome:
>> 
>> When doing:
>> ~# git checkout  RELEASE-4.4.0
>> ~# make mrproper && ./configure && make debug=n
>> 
>> It also fails with the error in dhcp i mentioned before.
>> 
>> However if i do:
>> ~# git checkout  RELEASE-4.4.0
>> ~# make mrproper && ./configure && make
>> 
>> It builds fine .. (and release-4.4.0 should have the implicit debug=n)
>> So what is different when explicitly giving debug=n ?

> The only thing I can thing of would be that specifying debug= on the
> make command line makes it enter the environment or MAKEFLAGS or
> something, whereas setting it in the Makefile or .config etc doesn't.
> (Perhaps e.g. "export debug" in Config.mk would make these two behave
> the same?)

> Anyway, that having happened the debug could then filter down and get
> picked up by the configure or build system of some sub-component in a
> way we aren't expecting.

> That's all 100% speculation though.

Well just setting it only in the Config.mk works fine.

Could be due to the mixing, i was used to not changing my Config.mk and just 
specifying "make debug=(y|n)" on the commandline.
So Config.mk had debug=y, while the make had debug=n.
However some googling didn't show much if that was even ever supported,
although it used to work fine ;-)

Should there just be a ./configure --debug=y ?

> Ian.

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Mon, 2014-07-28 at 11:47 +0200, Sander Eikelenboom wrote:
> Monday, July 28, 2014, 11:09:35 AM, you wrote:
> 
> > On Sat, 2014-07-26 at 17:14 +0200, Sander Eikelenboom wrote:
> >> Just tested some more with a strange outcome:
> >> 
> >> When doing:
> >> ~# git checkout  RELEASE-4.4.0
> >> ~# make mrproper && ./configure && make debug=n
> >> 
> >> It also fails with the error in dhcp i mentioned before.
> >> 
> >> However if i do:
> >> ~# git checkout  RELEASE-4.4.0
> >> ~# make mrproper && ./configure && make
> >> 
> >> It builds fine .. (and release-4.4.0 should have the implicit debug=n)
> >> So what is different when explicitly giving debug=n ?
> 
> > The only thing I can thing of would be that specifying debug= on the
> > make command line makes it enter the environment or MAKEFLAGS or
> > something, whereas setting it in the Makefile or .config etc doesn't.
> > (Perhaps e.g. "export debug" in Config.mk would make these two behave
> > the same?)
> 
> > Anyway, that having happened the debug could then filter down and get
> > picked up by the configure or build system of some sub-component in a
> > way we aren't expecting.
> 
> > That's all 100% speculation though.
> 
> Well just setting it only in the Config.mk works fine.
> 
> Could be due to the mixing, i was used to not changing my Config.mk and just 
> specifying "make debug=(y|n)" on the commandline.
> So Config.mk had debug=y, while the make had debug=n.
> However some googling didn't show much if that was even ever supported,
> although it used to work fine ;-)

I must confess that I can never remember how make prioritises these
things.

> Should there just be a ./configure --debug=y ?

I don't think so -- debug is supposed to affect the hypervisor too
(mostly even, I think) and that should not require a configure.

Ian.

Re: xen-unstable stubdom build-failure when debug=n

[Olaf Hering]

On Mon, Jul 28, Sander Eikelenboom wrote:

> Should there just be a ./configure --debug=y ?

No. But perhaps a --enable-debug_symbols or --enable-debuginfo, which
sets CFLAGS+=-g in a single place instead of having -g in many
Makefiles as it is done now.

Olaf

Re: xen-unstable stubdom build-failure when debug=n

[Ian Campbell]

On Tue, 2014-07-22 at 09:11 +0200, Sander Eikelenboom wrote:
> If the build-test-system still has some slack, perhaps add a (weekly or only on a push) test that does a debug=n build ?

There isn't a lot of slack, but this is something that the new test 
hardware setup should address. A weekly test doesn't seem unreasonable
to me.

Bandwidth for someone to actually set this up on the other hand...

> (instead of waiting for the end of the release cycle and somewhere in the RC's to fixup the things that
> happen to turn up with debug=n (if my memory doesn't fail me too much, this has happened before)
> 
> (BTW i also use Debian Wheezy)
> --
> Sander
> 
> > Ian.
> 
> 
>