Re: fixes for type-punning warnings in GCC 4.1

[Alexandre Oliva <aoliva@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On Dec 19, 2005, Daniel Jacobowitz <drow@false.org> wrote:

>> > From: Alexandre Oliva <aoliva@redhat.com>
>> > Date: Mon, 19 Dec 2005 17:20:48 -0200

>> > -	num = sscanf (p, "%g%s", (float *) &putithere->typed_val_float.dval,s);
>> > +	num = sscanf (p, "%g%s", (float *) (void *) &putithere->typed_val_float.dval,s);

> We should fix it properly, in any case.  The right solution here is
> pretty apparent: delete the "float" case, and #define appropriate
> format characters for DOUBLEST in doublest.h, in the same place we
> typedef DOUBLEST.  Just like GCC's HOST_WIDE_INT_PRINT.

We can't quite do that because we may have to issue more complex
command patterns than simply a plain sscanf into the output location.
I've come up with this macro that defers to a static inline function
in the complex case.  Is this change acceptable?  Tested on
amd64-linux-gnu.

Another alternative that I found uglier would be to define a
DOUBLEST_SCAN macro and a DOUBLEST_SCAN_TYPE typedef, and always
declare a variable of DOUBLEST_SCAN_TYPE, scan into it, and then copy
the scanned value to the given output variable.  This enables people
to construct more complex scan patterns, but is it worth the ugliness
and extra copying for the sake of the uncommon `has long double but
can't scanf it' case?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-type-punning-2.patch --]
[-- Type: text/x-patch, Size: 7908 bytes --]

Index: gdb/ChangeLog
2005-12-19  Alexandre Oliva  <aoliva@redhat.com>

	* c-exp.y (parse_number): Silence type-punning warnings.
	* jv-exp.y (parse_number): Likewise.
	* objc-exp.y (parse_number): Likewise.
	* p-exp.y (parse_number): Likewise.
	* tui/tui-data.c (source_windows): Likewise.
	* varobj.c (free_variable): Likewise.

Index: gdb/c-exp.y
===================================================================
--- gdb/c-exp.y.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/c-exp.y	2006-01-16 16:09:10.000000000 -0200
@@ -1080,24 +1080,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%s", (float *) &putithere->typed_val_float.dval,s);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%s", (double *) &putithere->typed_val_float.dval,s);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%s", &putithere->typed_val_float.dval,s);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%s", &temp,s);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = SSCANF_DOUBLEST_AND_SUFFIX (p, putithere->typed_val_float.dval,
+					"%s", s);
       p[len] = saved_char;	/* restore the input stream */
 
       if (num == 1)
Index: gdb/jv-exp.y
===================================================================
--- gdb/jv-exp.y.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/jv-exp.y	2006-01-16 16:08:33.000000000 -0200
@@ -713,23 +713,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%c", (float *) &putithere->typed_val_float.dval, &c);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%c", (double *) &putithere->typed_val_float.dval, &c);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%c", &putithere->typed_val_float.dval, &c);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%c", &temp, &c);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = SSCANF_DOUBLEST_AND_SUFFIX (p, putithere->typed_val_float.dval,
+					"%c", &c);
       p[len] = saved_char;	/* restore the input stream */
       if (num != 1) 		/* check scanf found ONLY a float ... */
 	return ERROR;
Index: gdb/objc-exp.y
===================================================================
--- gdb/objc-exp.y.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/objc-exp.y	2006-01-16 16:08:50.000000000 -0200
@@ -1025,23 +1025,8 @@ parse_number (p, len, parsed_float, puti
 
       /* It's a float since it contains a point or an exponent.  */
 
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	sscanf (p, "%g", (float *)&putithere->typed_val_float.dval);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	sscanf (p, "%lg", (double *)&putithere->typed_val_float.dval);
-      else
-	{
-#ifdef PRINTF_HAS_LONG_DOUBLE
-	  sscanf (p, "%Lg", &putithere->typed_val_float.dval);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles.  */
-	  double temp;
-	  sscanf (p, "%lg", &temp);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      SSCANF_DOUBLEST_AND_SUFFIX (p, putithere->typed_val_float.dval,
+				  "%c", &c);
 
       /* See if it has `f' or `l' suffix (float or long double).  */
 
Index: gdb/p-exp.y
===================================================================
--- gdb/p-exp.y.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/p-exp.y	2006-01-16 16:09:00.000000000 -0200
@@ -799,23 +799,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%c", (float *) &putithere->typed_val_float.dval,&c);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%c", (double *) &putithere->typed_val_float.dval,&c);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%c", &putithere->typed_val_float.dval,&c);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%c", &temp,&c);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = SSCANF_DOUBLEST_AND_SUFFIX (p, putithere->typed_val_float.dval,
+					"%c", &c);
       p[len] = saved_char;	/* restore the input stream */
       if (num != 1) 		/* check scanf found ONLY a float ... */
 	return ERROR;
Index: gdb/tui/tui-data.c
===================================================================
--- gdb/tui/tui-data.c.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/tui/tui-data.c	2006-01-16 15:33:33.000000000 -0200
@@ -44,7 +44,13 @@ static int term_height, term_width;
 static struct tui_gen_win_info _locator;
 static struct tui_gen_win_info exec_info[2];
 static struct tui_win_info * src_win_list[2];
-static struct tui_list source_windows = {(void **) src_win_list, 0};
+/* The intermediate cast to void* silences a type-punning warning
+   issued by GCC.  The use appears to be safe, since we always access
+   source_windows.list with type void**, and whenever we access one of
+   the list members, we cast it to struct tui_win_info*.  The
+   interface of struct tui_list should probably be redesigned with
+   less type opacity to avoid type punning.  -aoliva */
+static struct tui_list source_windows = {(void **) (void *) src_win_list, 0};
 static int default_tab_len = DEFAULT_TAB_LEN;
 static struct tui_win_info * win_with_focus = (struct tui_win_info *) NULL;
 static struct tui_layout_def layout_def =
Index: gdb/varobj.c
===================================================================
--- gdb/varobj.c.orig	2006-01-16 15:30:39.000000000 -0200
+++ gdb/varobj.c	2006-01-16 15:33:33.000000000 -0200
@@ -1374,7 +1374,7 @@ free_variable (struct varobj *var)
   /* Free the expression if this is a root variable. */
   if (var->root->rootvar == var)
     {
-      free_current_contents ((char **) &var->root->exp);
+      free_current_contents (&var->root->exp);
       xfree (var->root);
     }
 
Index: gdb/doublest.h
===================================================================
--- gdb/doublest.h.orig	2006-01-16 15:57:33.000000000 -0200
+++ gdb/doublest.h	2006-01-16 16:08:27.000000000 -0200
@@ -50,8 +50,33 @@ struct floatformat;
 
 #ifdef HAVE_LONG_DOUBLE
 typedef long double DOUBLEST;
+
+# ifdef SCANF_HAS_LONG_DOUBLE
+#  define SSCANF_DOUBLEST_AND_SUFFIX(from, val, sufspec, suf)	\
+  (sscanf ((from), "%Lg" sufspec, &(val), (suf)))
+
+# else
+#  define SSCANF_DOUBLEST_AND_SUFFIX(from, val, sufspec, suf)	\
+  (sscanf_doublest_and_suffix ((from), "%lg" sufspec, &(val), (suf)))
+static inline int
+sscanf_doublest_and_suffix (const char *from, const char *spec,
+			    DOUBLEST *valp, void *sufp)
+{
+  int result;
+  double temp;
+
+  result = sscanf (from, spec, &temp, sufp);
+
+  *valp = temp;
+
+  return result;
+}
+# endif
+
 #else
 typedef double DOUBLEST;
+# define SSCANF_DOUBLEST_AND_SUFFIX(from, val, sufspec, suf)	\
+  (sscanf ((from), "%g" sufspec, &(val), &(suf)))
 #endif
 
 extern void floatformat_to_doublest (const struct floatformat *,

[-- Attachment #3: Type: text/plain, Size: 188 bytes --]


-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}