Re: [Xen-devel] 4.4.0-rc2 tagged

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.01.14 at 10:44, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>>>> On 15.01.14 at 01:09, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 14/01/2014 22:49, Don Slutz wrote:
>>> On 01/14/14 11:29, Ian Campbell wrote:
>>>> We've just tagged 4.4.0-rc2, please test and report bugs.
>>>>
>>>> The tarball can be downloaded here:
>>>>
>>>> http://bits.xensource.com/oss-xen/release/4.4.0-rc2/xen-4.4.0-rc2.tar.gz 
>>>>
>>>> Ian.
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@xxxxxxxxxxxxx 
>>>> http://lists.xen.org/xen-devel 
>>>
>>> This tarball does not build on CentOS 5.10:
>>>
>>>
>>> gcc -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing
>>> -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement
>>> -I/home/don/xen-4.4.0-rc2/xen/include
>>> -I/home/don/xen-4.4.0-rc2/xen/include/asm-x86/mach-generic
>>> -I/home/don/xen-4.4.0-rc2/xen/include/asm-x86/mach-default
>>> -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs
>>> -DHAVE_GAS_VMX -mno-red-zone -mno-sse -fpic
>>> -fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE
>>> -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith
>>> -pipe -g -D__XEN__ -include
>>> /home/don/xen-4.4.0-rc2/xen/include/xen/config.h -nostdinc
>>> -iwithprefix include -fno-optimize-sibling-calls -DVERBOSE -DHAS_ACPI
>>> -DHAS_GDBSX -DHAS_PASSTHROUGH -DHAS_PCI -DHAS_IOPORTS
>>> -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER -MMD -MF .memory.o.d -c
>>> memory.c -o memory.o
>>> cc1: warnings being treated as errors
>>> memory.c: In function 'compat_memory_op':
>>> memory.c:213: warning: comparison is always true due to limited range
>>> of data type
>>> memory.c:214: warning: comparison is always true due to limited range
>>> of data type
>>> memory.c:215: warning: comparison is always true due to limited range
>>> of data type
>>> make[5]: *** [memory.o] Error 1
>>> make[5]: Leaving directory `/home/don/xen-4.4.0-rc2/xen/common/compat'
>>> make[4]: *** [compat/built_in.o] Error 2
>>> make[4]: Leaving directory `/home/don/xen-4.4.0-rc2/xen/common'
>>> make[3]: *** [/home/don/xen-4.4.0-rc2/xen/common/built_in.o] Error 2
>>> make[3]: Leaving directory `/home/don/xen-4.4.0-rc2/xen/arch/x86'
>>> make[2]: *** [/home/don/xen-4.4.0-rc2/xen/xen] Error 2
>>> make[2]: Leaving directory `/home/don/xen-4.4.0-rc2/xen'
>>> make[1]: *** [install] Error 2
>>> make[1]: Leaving directory `/home/don/xen-4.4.0-rc2/xen'
>>> make: *** [install-xen] Error 2
>>> dcs-xen-53:~/xen-4.4.0-rc2>uname -a
>>> Linux dcs-xen-53 2.6.18-371.el5xen #1 SMP Tue Oct 1 09:15:30 EDT 2013
>>> x86_64 x86_64 x86_64 GNU/Linux
>>> dcs-xen-53:~/xen-4.4.0-rc2>cat /etc/redhat-release
>>> CentOS release 5.10 (Final)
>>> dcs-xen-53:~/xen-4.4.0-rc2>gcc -v
>>> Using built-in specs.
>>> Target: x86_64-redhat-linux
>>> Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
>>> --infodir=/usr/share/info --enable-shared --enable-threads=posix
>>> --enable-checking=release --with-system-zlib --enable-__cxa_atexit
>>> --disable-libunwind-exceptions --enable-libgcj-multifile
>>> --enable-languages=c,c++,objc,obj-c++,java,fortran,ada
>>> --enable-java-awt=gtk --disable-dssi --disable-plugin
>>> --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre
>>> --with-cpu=generic --host=x86_64-redhat-linux
>>> Thread model: posix
>>> gcc version 4.1.2 20080704 (Red Hat 4.1.2-54)
>> 
>> I have also just encountered this build error, but am currently on the
>> fence as to whether it is a compiler bug in 4.1.2 or bad code.  Using
>> newer compilers causes the complaint to go away.  I would certainly like
>> to hope that compat_handle_ok() is correct, and does appear to be
>> correct from code inspection.
>> 
>> The if statement becomes gigantic after preprocessing, and I ran out of
>> effort today to sanitise the preprocessed output and check it for
>> correctness.  (At the very least, it would be kind to the compiler to
>> factor out the paging_mode_external(current->domain) check and degrade
>> the compat_handle_okay()s to compat_array_access_ok())
> 
> It's not that bad; breaking the if() up a little got me to quickly
> see that this is due to struct xen_add_to_physmap_batch's
> size field being uint16_t. Using a separate local variable to
> latch the structure value makes the problem go away. The
> warning isn't really a compiler bug, but also not very useful.
> 
> Question now is: Should we replace the checks with
> BUILD_BUG_ON()s (I wouldn't want to drop them altogether)
> or suppress the warning via intermediate variable?

And looking at the implications I'm much in favor of using
an intermediate variable (see below).

Jan

compat/memory: fix build with old gcc

struct xen_add_to_physmap_batch's size field being uint16_t causes old
compiler versions to warn about the pointless range check done inside
compat_handle_okay().

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -206,18 +206,20 @@ int compat_memory_op(unsigned int cmd, X
         {
             unsigned int limit = (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.atpb))
                                  / (sizeof(nat.atpb->idxs.p) + 
sizeof(nat.atpb->gpfns.p));
+            /* Use an intermediate variable to suppress warnings on old gcc: */
+            unsigned int size = cmp.atpb.size;
             xen_ulong_t *idxs = (void *)(nat.atpb + 1);
             xen_pfn_t *gpfns = (void *)(idxs + limit);
 
             if ( copy_from_guest(&cmp.atpb, compat, 1) ||
-                 !compat_handle_okay(cmp.atpb.idxs, cmp.atpb.size) ||
-                 !compat_handle_okay(cmp.atpb.gpfns, cmp.atpb.size) ||
-                 !compat_handle_okay(cmp.atpb.errs, cmp.atpb.size) )
+                 !compat_handle_okay(cmp.atpb.idxs, size) ||
+                 !compat_handle_okay(cmp.atpb.gpfns, size) ||
+                 !compat_handle_okay(cmp.atpb.errs, size) )
                 return -EFAULT;
 
             end_extent = start_extent + limit;
-            if ( end_extent > cmp.atpb.size )
-                end_extent = cmp.atpb.size;
+            if ( end_extent > size )
+                end_extent = size;
 
             idxs -= start_extent;
             gpfns -= start_extent;



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