This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA-v2] Fix bug report 11479


> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé?: Thursday, April 22, 2010 3:11 AM
> À?: Pierre Muller
> Cc?: gdb-patches@sourceware.org
> Objet?: Re: [RFC] Fix bug report 11479
> 
> > > I think that the proper solution would be to enhance function
> > > cleanup_undefined_types_1 to also look at symbols inside function
> > > symbols.  That way, the problem should be fixed for all kinds of
> > > types, not just structs...
> >
> >  I do not think that this is enough: the problem is more that
> >  TYPE_STUB macro refers to the flag_stub of main_type, which means
> >  that once the main_type flag_stub has been cleared, all types having
> >  the same main_type will also return zero for TYPE_STUB.
> 
> It's definitely something i got myself confused over. In the stabs I
> posted:
> 
>         .stabs  "t:p(0,21)=*(0,22)=k(0,23)=xsdummy:",160,0,28,-24
>         .stabs
> "dummy:T(0,23)=s16x:(0,1),0,32;y:(0,1),32,32;b:(0,13),64,64;;",128,0,0,
> 0
> 
> Type dummy is numbered (0,23), whereas I talked myself into thinking
> that type (0,23) would the be constant version of type dummy.  With
> that confusion cleared, we do indeed clear the stub condition on our
> type at the time when we read the second stabs line above. So, by the
> time we reach the cleanup_undefined_types_1 stage, the type is actually
> no longer undefined.

  We agree here.
 
> >   I still think that we should cycle over the type_chain when the
> type
> >   is parsed.
> 
> I am afraid this might be the only solution indeed. For some reason,
> even after our pretty comprehensive investigating, I am still pretty
> reluctant about it, and I think it is because it looks wrong to
> perform this relatively unintuitive operation, and just do it for
> structs. Conceivably, you'd need to do the same for enums as well,
> right?

  You are perfectly right, and I only understood now
what you meant.
  The patch below does now handle enumerations as well,
 and the new testsuite exp file also checks it.
 The previous version indeed only handled struct and not enums ...
are there any other types that can be opaque?
 Unions are also handled inside read_struct_type
so this should be OK.

> I think we can reduce my anxiety by:
>   - Adding detailed comments explaining what and why;
>   - Putting the code inside a function so that we can reuse it
> elsewhere

  That is what I did.
 
> Do you think that this would be an acceptable approach?

  Yes, I hope this patch handles it as you 
wanted.
 
> Sorry if I am slow, but stabsread, and stabs in general, are two
> horrible
> deprecated pieces of technology - so it's hard to remain current.

  Thanks for the suggestions,

  Here is the new version,
tested on gcc16, one significant difference was
2 of the tests inside gdb.stabs/gdb11479.exp 
that FAIL without the patch and pass with the patch.
  
Is this OK?

Pierre Muller


2010-04-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR stabs/11479.
	* stabsread.c (set_length_in_type_chain): New function.
	(read_struct_type): Call set_length_in_type_chain function.
	(read_enum_type): Idem.

Testsuite ChangeLog entry:

2010-04-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	PR stabs/11479.
	* gdb.stabs/gdb11479.exp: New file.
	* gdb.stabs/gdb11479.c: New file.


Index: src/gdb/stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.124
diff -u -p -r1.124 stabsread.c
--- src/gdb/stabsread.c	5 Apr 2010 22:43:47 -0000	1.124
+++ src/gdb/stabsread.c	22 Apr 2010 07:29:06 -0000
@@ -3393,6 +3393,23 @@ complain_about_struct_wipeout (struct ty
 	     _("struct/union type gets multiply defined: %s%s"), kind,
name);
 }
 
+/* Set the length for all variants of a same main_type, which are
+   connected in the closed chain.  */
+
+static void
+set_length_in_type_chain (struct type * type)
+{
+  struct type *ntype = TYPE_CHAIN (type);
+
+  while (ntype != type)
+    {
+      if (TYPE_LENGTH(ntype) == 0)
+	TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+      else
+        complain_about_struct_wipeout (ntype);
+      ntype = TYPE_CHAIN (ntype);
+    }
+}
 
 /* Read the description of a structure (or union type) and return an object
    describing the type.
@@ -3451,6 +3468,7 @@ read_struct_type (char **pp, struct type
     TYPE_LENGTH (type) = read_huge_number (pp, 0, &nbits, 0);
     if (nbits != 0)
       return error_type (pp, objfile);
+    set_length_in_type_chain (type);
   }
 
   /* Now read the baseclasses, if any, read the regular C struct or C++
@@ -3615,6 +3633,7 @@ read_enum_type (char **pp, struct type *
   /* Now fill in the fields of the type-structure.  */
 
   TYPE_LENGTH (type) = gdbarch_int_bit (gdbarch) / HOST_CHAR_BIT;
+  set_length_in_type_chain (type);
   TYPE_CODE (type) = TYPE_CODE_ENUM;
   TYPE_STUB (type) = 0;
   if (unsigned_enum)
Index: src/gdb/testsuite/gdb.stabs/gdb11479.c
===================================================================
RCS file: testsuite/gdb.stabs/gdb11479.c
diff -N src/gdb/testsuite/gdb.stabs/gdb11479.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.stabs/gdb11479.c	22 Apr 2010 07:42:19 -0000
@@ -0,0 +1,69 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   Contributed by Pierre Muller.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Please email any bugs, comments, and/or additions to this file to:
+   bug-gdb@gnu.org  */
+
+/* Qualifiers of forward types are not resolved correctly with stabs.  */
+
+struct dummy;
+
+enum dummy_enum;
+
+const void *
+test (const struct dummy *t)
+{
+  const struct dummy *tt;
+  enum dummy_enum *e;
+  tt = t;
+  return t;
+}
+
+void *
+test2 (struct dummy *t)
+{
+  struct dummy *tt;
+  const enum dummy_enum *e;
+  tt = t;
+  return t;
+}
+
+
+struct dummy {
+ int x;
+ int y;
+ double b;
+} tag_dummy;
+
+enum dummy_enum {
+  enum1,
+  enum2
+};
+
+int
+main ()
+{
+  struct dummy tt;
+  tt.x = 5;
+  tt.y = 25;
+  tt.b = 2.5;
+  test2 (&tt);
+  test (&tt);
+  return 0;
+}
Index: src/gdb/testsuite/gdb.stabs/gdb11479.exp
===================================================================
RCS file: testsuite/gdb.stabs/gdb11479.exp
diff -N src/gdb/testsuite/gdb.stabs/gdb11479.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gdb/testsuite/gdb.stabs/gdb11479.exp	22 Apr 2010 09:49:24 -0000
@@ -0,0 +1,59 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# Test GDB stabs problem with qualified parameter of forward types.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "gdb11479"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
{debug additional_flags=-gstabs}] != "" } {
+    if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable {debug}] != "" } {
+	untested "couldn't compile ${srcdir}/${subdir}/${srcfile}"
+	return -1
+    }
+}
+
+# Start with a fresh gdb.
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Regression test for a cleanup bug in the charset code.
+gdb_test "rb test" "" "Set breakpoints"
+gdb_test "run" "Breakpoint .* test2 .*" "Stop at first breakpoint"
+# Check that the struct is read in correctly
+gdb_test "print *t" ".*\{x = 5, y = 25, b = 2.5\}.*" "Inspect t in test2"
+# Check that the enum type length has been set to a non-zero value
+gdb_test "print sizeof (*e)" "= \[1-9\]*" "sizeof (e) in test2"
+gdb_test "continue" "Breakpoint .* test .*" "Stop at first breakpoint"
+gdb_test "print *t" ".*\{x = 5, y = 25, b = 2.5\}.*" "Inspect t in test"
+# Check that the enum type length has been set to a non-zero value
+gdb_test "print sizeof (*e)" "= \[1-9\]*" "sizeof (e) in test"
+
+gdb_exit
+


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]