Xen devel - Jan 2011 - [PATCH]: minios: eliminate compile warning - use of uninitialised variable

I''m not sure if this is a legit catch by gcc or a false positive due to
extra cleverness in gcc-4.5. Either way it causes the build to barf.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r c55b93f3f8cf extras/mini-os/lib/math.c
--- a/extras/mini-os/lib/math.c	Fri Jan 07 15:27:30 2011 +0000
+++ b/extras/mini-os/lib/math.c	Fri Jan 07 16:48:27 2011 +0000
@@ -157,6 +157,8 @@ __qdivrem(uint64_t uq, uint64_t vq, uint
 	int m, n, d, j, i;
 	digit uspace[5], vspace[5], qspace[5];
 
+	memset(&tmp, 0, sizeof(tmp));
+
 	/*
 	 * Take care of special cases: divide by zero, and u < v.
 	 */



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

Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit
:
> I''m not sure if this is a legit catch by gcc or a false positive
due to
> extra cleverness in gcc-4.5. Either way it causes the build to barf.
It seems a false positive to me. What is the warning message?

Samuel

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

On Fri, 2011-01-07 at 16:58 +0000, Samuel Thibault
wrote:
> Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit :
> > I''m not sure if this is a legit catch by gcc or a false
positive due to
> > extra cleverness in gcc-4.5. Either way it causes the build to barf.
> 
> It seems a false positive to me. What is the warning message?
lib/math.c:197:22: error: ‘tmp.ul[1]’ may be used uninitialized in this function
lib/math.c:153:11: note: ‘tmp.ul[1]’ was declared here

I figured it was a false positive but looking at it now, maybe not?

Gianni


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

On Fri, 2011-01-07 at 17:21 +0000, Gianni Tedesco wrote:
> On Fri, 2011-01-07 at 16:58 +0000, Samuel Thibault wrote:
> > Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit :
> > > I''m not sure if this is a legit catch by gcc or a false
positive due to
> > > extra cleverness in gcc-4.5. Either way it causes the build to
barf.
> > 
> > It seems a false positive to me. What is the warning message?
Actually looks like these are incorrect when compiled 64bit and not
needed anyway. Following patch does the job.

diff -r 4695fd70077d extras/mini-os/lib/math.c
--- a/extras/mini-os/lib/math.c	Fri Jan 07 17:13:53 2011 +0000
+++ b/extras/mini-os/lib/math.c	Fri Jan 07 17:40:20 2011 +0000
@@ -59,9 +59,9 @@
 #include <mini-os/lib.h>
 #include <mini-os/time.h>
 
-	/* On ia64 these functions lead to crashes. These are replaced by
+	/* These functions are incorrect on 64bit. On ia64 they are replaced by
 	 * assembler functions. */
-#if !defined(__ia64__)
+#if !defined(__ia64__) && !defined(__x86_64__)
 
 /*
  * Depending on the desired operation, we view a `long long'' (aka
quad_t) in



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

Gianni Tedesco, le Fri 07 Jan 2011 17:21:59 +0000, a écrit
:
> On Fri, 2011-01-07 at 16:58 +0000, Samuel Thibault wrote:
> > Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit :
> > > I''m not sure if this is a legit catch by gcc or a false
positive due to
> > > extra cleverness in gcc-4.5. Either way it causes the build to
barf.
> > 
> > It seems a false positive to me. What is the warning message?
> 
> lib/math.c:197:22: error: ‘tmp.ul[1]’ may be used uninitialized in this
function
> lib/math.c:153:11: note: ‘tmp.ul[1]’ was declared here
> 
> I figured it was a false positive but looking at it now, maybe not?
Oooh, is this perhaps on a 64bit machine? There is actually a bug: these
should be ints, not longs... Use this instead.

Samuel

Replace long/int/short sizes with proper exact-size types for 64bit
architectures.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

diff -r a33886146b45 extras/mini-os/lib/math.c
--- a/extras/mini-os/lib/math.c	Fri Oct 08 11:41:57 2010 +0100
+++ b/extras/mini-os/lib/math.c	Fri Jan 07 20:26:34 2011 +0100
@@ -70,8 +70,8 @@
 union uu {
         int64_t           q;              /* as a (signed) quad */
         int64_t          uq;             /* as an unsigned quad */
-        long           sl[2];          /* as two signed longs */
-        unsigned long  ul[2];          /* as two unsigned longs */
+        int32_t        sl[2];          /* as two signed ints */
+        uint32_t       ul[2];          /* as two unsigned ints */
 };
 /* XXX RN: Yuck hardcoded endianess :) */
 #define _QUAD_HIGHWORD 1
@@ -91,17 +91,17 @@
 #define CHAR_BIT        8               /* number of bits in a char */
 #endif
 #define QUAD_BITS       (sizeof(int64_t) * CHAR_BIT)
-#define LONG_BITS       (sizeof(long) * CHAR_BIT)
-#define HALF_BITS       (sizeof(long) * CHAR_BIT / 2)
+#define LONG_BITS       (sizeof(int32_t) * CHAR_BIT)
+#define HALF_BITS       (sizeof(int32_t) * CHAR_BIT / 2)
 
 /*
- * Extract high and low shortwords from longword, and move low shortword of
- * longword to upper half of long, i.e., produce the upper longword of
- * ((quad_t)(x) << (number_of_bits_in_long/2)).  (`x'' must
actually be u_long.)
+ * Extract high and low shortwords from intword, and move low shortword of
+ * intword to upper half of int32_t, i.e., produce the upper intword of
+ * ((quad_t)(x) << (number_of_bits_in_int/2)).  (`x'' must
actually be uint32_t.)
  *
- * These are used in the multiply code, to split a longword into upper
+ * These are used in the multiply code, to split a intword into upper
  * and lower halves, and to reassemble a product as a quad_t, shifted left
- * (sizeof(long)*CHAR_BIT/2).
+ * (sizeof(int32_t)*CHAR_BIT/2).
  */
 #define HHALF(x)        ((x) >> HALF_BITS)
 #define LHALF(x)        ((x) & ((1UL << HALF_BITS) - 1))
@@ -114,14 +114,10 @@
 #define	B	(1UL << HALF_BITS)	/* digit base */
 
 /* Combine two `digits'' to make a single two-digit number. */
-#define	COMBINE(a, b) (((u_long)(a) << HALF_BITS) | (b))
+#define	COMBINE(a, b) (((uint32_t)(a) << HALF_BITS) | (b))
 
-/* select a type for digits in base B: use unsigned short if they fit */
-#if ULONG_MAX == 0xffffffff && USHRT_MAX >= 0xffff
-typedef unsigned short digit;
-#else
-typedef u_long digit;
-#endif
+/* select a type for digits in base B: */
+typedef uint16_t digit;
 
 
 /*
@@ -143,7 +139,7 @@
  * __qdivrem(u, v, rem) returns u/v and, optionally, sets *rem to u%v.
  *
  * We do this in base 2-sup-HALF_BITS, so that all intermediate products
- * fit within u_long.  As a consequence, the maximum length dividend and
+ * fit within uint32_t.  As a consequence, the maximum length dividend and
  * divisor are 4 `digits'' in this base (they are shorter if they have
  * leading zeros).
  */
@@ -153,7 +149,7 @@
 	union uu tmp;
 	digit *u, *v, *q;
 	register digit v1, v2;
-	u_long qhat, rhat, t;
+	uint32_t qhat, rhat, t;
 	int m, n, d, j, i;
 	digit uspace[5], vspace[5], qspace[5];
 
@@ -204,7 +200,7 @@
 	v[4] = LHALF(tmp.ul[L]);
 	for (n = 4; v[1] == 0; v++) {
 		if (--n == 1) {
-			u_long rbj;	/* r*B+u[j] (not root boy jim) */
+			uint32_t rbj;	/* r*B+u[j] (not root boy jim) */
 			digit q1, q2, q3, q4;
 
 			/*
@@ -280,7 +276,7 @@
 			rhat = uj1;
 			goto qhat_too_big;
 		} else {
-			u_long nn = COMBINE(uj0, uj1);
+			uint32_t nn = COMBINE(uj0, uj1);
 			qhat = nn / v1;
 			rhat = nn % v1;
 		}

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

Gianni Tedesco, le Fri 07 Jan 2011 17:42:22 +0000, a écrit
:
> On Fri, 2011-01-07 at 17:21 +0000, Gianni Tedesco wrote:
> > On Fri, 2011-01-07 at 16:58 +0000, Samuel Thibault wrote:
> > > Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit :
> > > > I''m not sure if this is a legit catch by gcc or a
false positive due to
> > > > extra cleverness in gcc-4.5. Either way it causes the build
to barf.
> > > 
> > > It seems a false positive to me. What is the warning message?
> 
> Actually looks like these are incorrect when compiled 64bit and not
> needed anyway. Following patch does the job.
They _are_ needed when you do some particular divisions: gcc will emit
references in that case.

Samuel

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

On Fri, 2011-01-07 at 19:27 +0000, Samuel Thibault
wrote:
> Gianni Tedesco, le Fri 07 Jan 2011 17:21:59 +0000, a écrit :
> > On Fri, 2011-01-07 at 16:58 +0000, Samuel Thibault wrote:
> > > Gianni Tedesco, le Fri 07 Jan 2011 16:52:49 +0000, a écrit :
> > > > I''m not sure if this is a legit catch by gcc or a
false positive due to
> > > > extra cleverness in gcc-4.5. Either way it causes the build
to barf.
> > > 
> > > It seems a false positive to me. What is the warning message?
> > 
> > lib/math.c:197:22: error: ‘tmp.ul[1]’ may be used uninitialized in
this function
> > lib/math.c:153:11: note: ‘tmp.ul[1]’ was declared here
> > 
> > I figured it was a false positive but looking at it now, maybe not?
> 
> Oooh, is this perhaps on a 64bit machine? There is actually a bug: these
> should be ints, not longs... Use this instead.
> 
> Samuel
> 
> Replace long/int/short sizes with proper exact-size types for 64bit
> architectures.
> 
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> diff -r a33886146b45 extras/mini-os/lib/math.c
> --- a/extras/mini-os/lib/math.c	Fri Oct 08 11:41:57 2010 +0100
> +++ b/extras/mini-os/lib/math.c	Fri Jan 07 20:26:34 2011 +0100
Yep, works for me...

Gianni


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

Samuel Thibault writes ("Re: [Xen-devel] [PATCH]: minios: eliminate compile
warning - use of uninitialised variable"):
> Oooh, is this perhaps on a 64bit machine? There is actually a bug: these
> should be ints, not longs... Use this instead.
Thanks for looking at this and providing the patch.  I have applied
it.

Ian.

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