Re: fixes for type-punning warnings in GCC 4.1

Alexandre Oliva
2006-02-13T18:58:01Z

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

On Feb 9, 2006, Daniel Jacobowitz <drow@false.org> wrote:

> On Thu, Feb 09, 2006 at 10:32:48PM -0200, Alexandre Oliva wrote:
>> On Feb 8, 2006, Daniel Jacobowitz <drow@false.org> wrote:
>>
>> > On Wed, Feb 08, 2006 at 12:48:11AM -0200, Alexandre Oliva wrote:
>> >> On Jan 22, 2006, Daniel Jacobowitz <drow@false.org> wrote:
>> >>
>> >> > The output is always a DOUBLEST. I don't know of any reason why we
>> >> > should enable HAVE_LONG_DOUBLE if we can't printf and scanf them; would
>> >> > this be simpler in that case? Don't make DOUBLEST something we can't
>> >> > scan or print.
>> >>
>> >> Sounds good to me. Ok to install?
>>
>> > Well, it's not right as-is; you need to look at the other uses of
>> > HAVE_LONG_DOUBLE.
>>
>> Did. The other uses of HAVE_LONG_DOUBLE are correct, since they do
>> not assume DOUBLEST is long double and they make no attempts at
>> printing long doubles directly.

> Disagree; did you read the bit of my message that you snipped?
> doublest.c jumps through unnecessary hoops casting to long double and
> back to handle a DOUBLEST if this is defined.

Yep. It didn't look like those two occurrences were such a big deal.
I don't see why we should refuse to handle long double at all just
because we can't scan them in as such. But it's not my call, I guess.

>> > Would you mind terribly fixing that, adding a changelog, and leaving
>> > out the tui-data change for now?

>> gdb won't build without the tui-data change. What's wrong with adding
>> the temporary fix now, such that it builds, until someone with a
>> better understanding can go ahead and re-engineer the data structure
>> correctly?

> Sorry, use -Wno-error if you're in that much of a hurry. I even
> offered to take care of it for you. If your fix goes in, it will never
> leave; we have plenty of experience with FIXMEs in GDB to back that up,
> I think.

Ok, I tried changing the type in the struct declaration and that
seems to have worked, so I went ahead and removed the now-redundant
type casts.

Ok to install?

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

for gdb/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>

* doublest.h (DOUBLEST): Use long double only if we can scan
it in. Undefine HAVE_LONG_DOUBLE otherwise.
(DOUBLEST_FORMAT): New.
* c-exp.y (parse_number): Use it.
* jv-exp.y (parse_number): Likewise.
* objc-exp.y (parse_number): Likewise.
* p-exp.y (parse_number): Likewise.
* varobj.c (free_variable): Silence type-punning warnings.
* tui/tui-data.h (struct tui_list): Change type of list member.
* tui/tui-data.c: Remove no-longer-needed type casts.
(source_windows): Silence type-punning warnings.
* tui/tui-stack.c, tui/tui-win.c, tui/tui-winsource.c: Likewise.

Index: gdb/c-exp.y
===================================================================
--- gdb/c-exp.y.orig 2006-02-13 16:33:29.000000000 -0200
+++ gdb/c-exp.y 2006-02-13 16:34:55.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 (p, DOUBLEST_FORMAT "%s",
+ &putithere->typed_val_float.dval, s);
p[len] = saved_char; /* restore the input stream */

if (num == 1)
Index: gdb/jv-exp.y
===================================================================
--- gdb/jv-exp.y.orig 2006-02-13 16:33:29.000000000 -0200
+++ gdb/jv-exp.y 2006-02-13 16:34:55.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 (p, DOUBLEST_FORMAT "%c",
+ &putithere->typed_val_float.dval, &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-02-13 16:33:29.000000000 -0200
+++ gdb/objc-exp.y 2006-02-13 16:34:55.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 (p, DOUBLEST_FORMAT "%c",
+ &putithere->typed_val_float.dval, &c);

/* See if it has `f' or `l' suffix (float or long double). */

Index: gdb/p-exp.y
===================================================================
--- gdb/p-exp.y.orig 2006-02-13 16:33:29.000000000 -0200
+++ gdb/p-exp.y 2006-02-13 16:34:55.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 (p, DOUBLEST_FORMAT "%c",
+ &putithere->typed_val_float.dval, &c);
p[len] = saved_char; /* restore the input stream */
if (num != 1) /* check scanf found ONLY a float ... */
return ERROR;
Index: gdb/varobj.c
===================================================================
--- gdb/varobj.c.orig 2006-02-13 16:33:29.000000000 -0200
+++ gdb/varobj.c 2006-02-13 16:34:55.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-02-13 16:33:29.000000000 -0200
+++ gdb/doublest.h 2006-02-13 16:34:55.000000000 -0200
@@ -48,10 +48,15 @@ struct floatformat;
host's `long double'. In general, we'll probably reduce the precision of
any such values and print a warning. */

-#ifdef HAVE_LONG_DOUBLE
+#if defined HAVE_LONG_DOUBLE && defined SCANF_HAS_LONG_DOUBLE
typedef long double DOUBLEST;
+# define DOUBLEST_FORMAT "%Lg"
#else
typedef double DOUBLEST;
+# define DOUBLEST_FORMAT "%g"
+/* If we can't scan or print long double, we don't want to use it
+ anywhere. */
+# undef HAVE_LONG_DOUBLE
#endif

extern void floatformat_to_doublest (const struct floatformat *,
Index: gdb/tui/tui-data.c
===================================================================
--- gdb/tui/tui-data.c.orig 2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-data.c 2006-02-13 16:52:07.000000000 -0200
@@ -44,7 +44,7 @@ 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};
+static struct tui_list source_windows = {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 =
@@ -182,7 +182,7 @@ tui_clear_source_windows_detail (void)
int i;

for (i = 0; i < (tui_source_windows ())->count; i++)
- tui_clear_win_detail ((struct tui_win_info *) (tui_source_windows ())->list[i]);
+ tui_clear_win_detail ((tui_source_windows ())->list[i]);
}

@@ -782,7 +782,7 @@ tui_free_all_source_wins_content (void)

for (i = 0; i < (tui_source_windows ())->count; i++)
{
- struct tui_win_info * win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+ struct tui_win_info * win_info = (tui_source_windows ())->list[i];

if (win_info != NULL)
{
Index: gdb/tui/tui-data.h
===================================================================
--- gdb/tui/tui-data.h.orig 2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-data.h 2006-02-13 16:52:07.000000000 -0200
@@ -110,7 +110,7 @@ enum tui_scroll_direction
/* General list struct. */
struct tui_list
{
- void **list;
+ struct tui_win_info **list;
int count;
};

Index: gdb/tui/tui-stack.c
===================================================================
--- gdb/tui/tui-stack.c.orig 2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-stack.c 2006-02-13 16:52:07.000000000 -0200
@@ -343,7 +343,7 @@ tui_show_frame_info (struct frame_info *
for (i = 0; i < (tui_source_windows ())->count; i++)
{
union tui_which_element *item;
- win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+ win_info = (tui_source_windows ())->list[i];

item = &((struct tui_win_element *) locator->content[0])->which_element;
if (win_info == TUI_SRC_WIN)
@@ -401,7 +401,7 @@ tui_show_frame_info (struct frame_info *
tui_show_locator_content ();
for (i = 0; i < (tui_source_windows ())->count; i++)
{
- win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+ win_info = (tui_source_windows ())->list[i];
tui_clear_source_content (win_info, EMPTY_SOURCE_PROMPT);
tui_update_exec_info (win_info);
}
Index: gdb/tui/tui-win.c
===================================================================
--- gdb/tui/tui-win.c.orig 2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-win.c 2006-02-13 16:52:07.000000000 -0200
@@ -1119,7 +1119,7 @@ tui_adjust_win_heights (struct tui_win_i
make_invisible_and_set_new_height (primary_win_info, new_height);
if (primary_win_info->generic.type == CMD_WIN)
{
- win_info = (struct tui_win_info *) (tui_source_windows ())->list[0];
+ win_info = (tui_source_windows ())->list[0];
src_win_info = win_info;
}
else
@@ -1148,7 +1148,7 @@ tui_adjust_win_heights (struct tui_win_i
else
{
first_win = TUI_DATA_WIN;
- second_win = (struct tui_win_info *) (tui_source_windows ())->list[0];
+ second_win = (tui_source_windows ())->list[0];
}
if (primary_win_info == TUI_CMD_WIN)
{ /*
@@ -1393,7 +1393,7 @@ new_height_ok (struct tui_win_info * pri
struct tui_win_info * win_info;

if (primary_win_info == TUI_CMD_WIN)
- win_info = (struct tui_win_info *) (tui_source_windows ())->list[0];
+ win_info = (tui_source_windows ())->list[0];
else
win_info = TUI_CMD_WIN;
ok = ((new_height +
@@ -1414,7 +1414,7 @@ new_height_ok (struct tui_win_info * pri
else
{
first_win = TUI_DATA_WIN;
- second_win = (struct tui_win_info *) (tui_source_windows ())->list[0];
+ second_win = (tui_source_windows ())->list[0];
}
/*
** We could simply add all the heights to obtain the same result
@@ -1543,7 +1543,7 @@ parse_scrolling_args (char *arg, struct
error (_("Invalid window specified. \n\
The window name specified must be valid and visible.\n"));
else if (*win_to_scroll == TUI_CMD_WIN)
- *win_to_scroll = (struct tui_win_info *) (tui_source_windows ())->list[0];
+ *win_to_scroll = (tui_source_windows ())->list[0];
}
xfree (buf);
}
Index: gdb/tui/tui-winsource.c
===================================================================
--- gdb/tui/tui-winsource.c.orig 2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-winsource.c 2006-02-13 16:52:07.000000000 -0200
@@ -160,7 +160,7 @@ tui_update_source_windows_with_addr (COR

for (i = 0; i < (tui_source_windows ())->count; i++)
{
- struct tui_win_info * win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+ struct tui_win_info * win_info = (tui_source_windows ())->list[i];

tui_clear_source_content (win_info, EMPTY_SOURCE_PROMPT);
tui_clear_exec_info_content (win_info);
@@ -384,7 +384,7 @@ tui_update_all_breakpoint_info (void)

for (i = 0; i < list->count; i++)
{
- struct tui_win_info * win = (struct tui_win_info *) list->list[i];
+ struct tui_win_info * win = list->list[i];

if (tui_update_breakpoint_info (win, FALSE))
{

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

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