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]

Re: [patch] Fix crash on DWARF C++ forward reference


>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> the problem is based on the testcase contained in GCC PR debug/28767
Jan> by Daniel J. with specially crafted forward-reference resulting in
Jan> a double-entry to quirk_gcc_member_function_pointer for the same DIE which
Jan> crashes GDB.  Problem has been seen in practice.

See http://sourceware.org/bugzilla/show_bug.cgi?id=11199

Jan> I understand the solution is not nice but I hope the DWARF reading
Jan> functions do not access much the content of referenced TYPEs.

I also came up with a patch.  What do you think of the appended?

My concern with your patch is what happens if
quirk_gcc_member_function_pointer bails out after it sets the DIE type?
It seems to me that if __pfn refers to its enclosing struct type, then
it could refer to the wrong type in the end.

My patch also suffers from the (IMO low) risk of the DWARF code seeing a
struct type at one point and a member function at another.  Also,
smashing the type seems pretty ugly.

Tom

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index db51653..3bc72ac 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4892,68 +4892,48 @@ is_vtable_name (const char *name, struct dwarf2_cu *cu)
 }
 
 /* GCC outputs unnamed structures that are really pointers to member
-   functions, with the ABI-specified layout.  If DIE (from CU) describes
-   such a structure, set its type, and return nonzero.  Otherwise return
-   zero.
+   functions, with the ABI-specified layout.  If TYPE describes
+   such a structure, smash it into a member function type.
 
    GCC shouldn't do this; it should just output pointer to member DIEs.
    This is GCC PR debug/28767.  */
 
-static struct type *
-quirk_gcc_member_function_pointer (struct die_info *die, struct dwarf2_cu *cu)
+static void
+quirk_gcc_member_function_pointer (struct type *type, struct objfile *objfile)
 {
-  struct objfile *objfile = cu->objfile;
-  struct type *type;
-  struct die_info *pfn_die, *delta_die;
-  struct attribute *pfn_name, *delta_name;
-  struct type *pfn_type, *domain_type;
+  struct type *pfn_type, *domain_type, *new_type;
 
   /* Check for a structure with no name and two children.  */
-  if (die->tag != DW_TAG_structure_type
-      || dwarf2_attr (die, DW_AT_name, cu) != NULL
-      || die->child == NULL
-      || die->child->sibling == NULL
-      || (die->child->sibling->sibling != NULL
-	  && die->child->sibling->sibling->tag != DW_TAG_padding))
-    return NULL;
+  if (TYPE_CODE (type) != TYPE_CODE_STRUCT || TYPE_NFIELDS (type) != 2)
+    return;
 
   /* Check for __pfn and __delta members.  */
-  pfn_die = die->child;
-  pfn_name = dwarf2_attr (pfn_die, DW_AT_name, cu);
-  if (pfn_die->tag != DW_TAG_member
-      || pfn_name == NULL
-      || DW_STRING (pfn_name) == NULL
-      || strcmp ("__pfn", DW_STRING (pfn_name)) != 0)
-    return NULL;
-
-  delta_die = pfn_die->sibling;
-  delta_name = dwarf2_attr (delta_die, DW_AT_name, cu);
-  if (delta_die->tag != DW_TAG_member
-      || delta_name == NULL
-      || DW_STRING (delta_name) == NULL
-      || strcmp ("__delta", DW_STRING (delta_name)) != 0)
-    return NULL;
+  if (TYPE_FIELD_NAME (type, 0) == NULL
+      || strcmp (TYPE_FIELD_NAME (type, 0), "__pfn") != 0
+      || TYPE_FIELD_NAME (type, 1) == NULL
+      || strcmp (TYPE_FIELD_NAME (type, 1), "__delta") != 0)
+    return;
 
   /* Find the type of the method.  */
-  pfn_type = die_type (pfn_die, cu);
+  pfn_type = TYPE_FIELD_TYPE (type, 0);
   if (pfn_type == NULL
       || TYPE_CODE (pfn_type) != TYPE_CODE_PTR
       || TYPE_CODE (TYPE_TARGET_TYPE (pfn_type)) != TYPE_CODE_FUNC)
-    return NULL;
+    return;
 
   /* Look for the "this" argument.  */
   pfn_type = TYPE_TARGET_TYPE (pfn_type);
   if (TYPE_NFIELDS (pfn_type) == 0
+      /* || TYPE_FIELD_TYPE (pfn_type, 0) == NULL */
       || TYPE_CODE (TYPE_FIELD_TYPE (pfn_type, 0)) != TYPE_CODE_PTR)
-    return NULL;
+    return;
 
   domain_type = TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (pfn_type, 0));
-  type = alloc_type (objfile);
-  smash_to_method_type (type, domain_type, TYPE_TARGET_TYPE (pfn_type),
+  new_type = alloc_type (objfile);
+  smash_to_method_type (new_type, domain_type, TYPE_TARGET_TYPE (pfn_type),
 			TYPE_FIELDS (pfn_type), TYPE_NFIELDS (pfn_type),
 			TYPE_VARARGS (pfn_type));
-  type = lookup_methodptr_type (type);
-  return set_die_type (die, type, cu);
+  smash_to_methodptr_type (type, new_type);
 }
 
 /* Called when we find the DIE that starts a structure or union scope
@@ -4981,10 +4961,6 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
   char *name;
   struct cleanup *back_to = make_cleanup (null_cleanup, 0);
 
-  type = quirk_gcc_member_function_pointer (die, cu);
-  if (type)
-    return type;
-
   /* If the definition of this type lives in .debug_types, read that type.
      Don't follow DW_AT_specification though, that will take us back up
      the chain and we want to go down.  */
@@ -5163,6 +5139,8 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
 	}
     }
 
+  quirk_gcc_member_function_pointer (type, cu->objfile);
+
   do_cleanups (back_to);
   return type;
 }
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 117606a..2ff8647 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -673,10 +673,7 @@ lookup_methodptr_type (struct type *to_type)
   struct type *mtype;
 
   mtype = alloc_type_copy (to_type);
-  TYPE_TARGET_TYPE (mtype) = to_type;
-  TYPE_DOMAIN_TYPE (mtype) = TYPE_DOMAIN_TYPE (to_type);
-  TYPE_LENGTH (mtype) = cplus_method_ptr_size (to_type);
-  TYPE_CODE (mtype) = TYPE_CODE_METHODPTR;
+  smash_to_methodptr_type (mtype, to_type);
   return mtype;
 }
 
@@ -981,6 +978,22 @@ smash_to_memberptr_type (struct type *type, struct type *domain,
   TYPE_CODE (type) = TYPE_CODE_MEMBERPTR;
 }
 
+/* Smash TYPE to be a type of pointer to methods type TO_TYPE.
+
+   When "smashing" the type, we preserve the objfile that the old type
+   pointed to, since we aren't changing where the type is actually
+   allocated.  */
+
+void
+smash_to_methodptr_type (struct type *type, struct type *to_type)
+{
+  smash_type (type);
+  TYPE_TARGET_TYPE (type) = to_type;
+  TYPE_DOMAIN_TYPE (type) = TYPE_DOMAIN_TYPE (to_type);
+  TYPE_LENGTH (type) = cplus_method_ptr_size (to_type);
+  TYPE_CODE (type) = TYPE_CODE_METHODPTR;
+}
+
 /* Smash TYPE to be a type of method of DOMAIN with type TO_TYPE.
    METHOD just means `function that gets an extra "this" argument'.
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 380f72a..72968a5 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1312,6 +1312,8 @@ extern void smash_to_method_type (struct type *type, struct type *domain,
 extern void smash_to_memberptr_type (struct type *, struct type *,
 				     struct type *);
 
+extern void smash_to_methodptr_type (struct type *, struct type *);
+
 extern struct type *allocate_stub_method (struct type *);
 
 extern char *type_name_no_tag (const struct type *);


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