Patchwork [RFA,5/6] Remove objfile argument from add_dyn_prop

login
register
mail settings
Submitter Tom Tromey
Date Jan. 6, 2018, 12:26 a.m.
Message ID <20180106002621.21099-6-tom@tromey.com>
Download mbox | patch
Permalink /patch/25241/
State New
Headers show

Comments

Tom Tromey - Jan. 6, 2018, 12:26 a.m.
The objfile argument to add_dyn_prop is redundant, so this patch
removes it.

2018-01-05  Tom Tromey  <tom@tromey.com>

	* gdbtypes.h (add_dyn_prop): Remove objfile parameter.
	* gdbtypes.c (add_dyn_prop): Remove objfile parameter.
	(create_array_type_with_stride): Update.
	* dwarf2read.c (set_die_type): Update.
---
 gdb/ChangeLog    | 7 +++++++
 gdb/dwarf2read.c | 6 +++---
 gdb/gdbtypes.c   | 7 +++----
 gdb/gdbtypes.h   | 5 ++---
 4 files changed, 15 insertions(+), 10 deletions(-)
Yao Qi - Jan. 10, 2018, 1:01 p.m.
Tom Tromey <tom@tromey.com> writes:

> @@ -25116,7 +25116,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>    if (attr_form_is_block (attr))
>      {
>        if (attr_to_dynamic_prop (attr, die, cu, &prop))
> -        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type, objfile);
> +        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
>      }

Before the patch, the objfile is
cu->per_cu->dwarf2_per_objfile->objfile, but after the patch, the
objfile is TYPE_OBJFILE (type), are they equivalent?
Tom Tromey - Jan. 10, 2018, 4:32 p.m.
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> Tom Tromey <tom@tromey.com> writes:
>> @@ -25116,7 +25116,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>> if (attr_form_is_block (attr))
>> {
>> if (attr_to_dynamic_prop (attr, die, cu, &prop))
>> -        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type, objfile);
>> +        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
>> }

Yao> Before the patch, the objfile is
Yao> cu-> per_cu->dwarf2_per_objfile->objfile, but after the patch, the
Yao> objfile is TYPE_OBJFILE (type), are they equivalent?

If they were not the same, then that was already a bug, because
types generally have the restriction that they can only point to static
data or to other objects allocated on the same objfile.  Cross-objfile
pointers are not allowed, to avoid dangling pointers when an objfile is
destroyed.

Now, a given invocation of the DWARF reader generally deals with a
single objfile.  So I believe they were the same.  But if they were not,
then this patch actually improves the situation.

Tom
Simon Marchi - Jan. 16, 2018, 3:32 p.m.
On 2018-01-05 07:26 PM, Tom Tromey wrote:
> The objfile argument to add_dyn_prop is redundant, so this patch
> removes it.
> 
> 2018-01-05  Tom Tromey  <tom@tromey.com>
> 
> 	* gdbtypes.h (add_dyn_prop): Remove objfile parameter.
> 	* gdbtypes.c (add_dyn_prop): Remove objfile parameter.
> 	(create_array_type_with_stride): Update.
> 	* dwarf2read.c (set_die_type): Update.

Hi Tom,

> @@ -2305,13 +2304,13 @@ get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
>  
>  void
>  add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
> -              struct type *type, struct objfile *objfile)
> +              struct type *type)
>  {
>    struct dynamic_prop_list *temp;
>  
>    gdb_assert (TYPE_OBJFILE_OWNED (type));
>  
> -  temp = XOBNEW (&objfile->objfile_obstack, struct dynamic_prop_list);
> +  temp = XOBNEW (&TYPE_OBJFILE (type)->objfile_obstack, struct dynamic_prop_list);

This line is now too long.

Otherwise, LGTM.

Simon
Yao Qi - Jan. 16, 2018, 4 p.m.
On Wed, Jan 10, 2018 at 4:32 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> Tom Tromey <tom@tromey.com> writes:
>>> @@ -25116,7 +25116,7 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>>> if (attr_form_is_block (attr))
>>> {
>>> if (attr_to_dynamic_prop (attr, die, cu, &prop))
>>> -        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type, objfile);
>>> +        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
>>> }
>
> Yao> Before the patch, the objfile is
> Yao> cu-> per_cu->dwarf2_per_objfile->objfile, but after the patch, the
> Yao> objfile is TYPE_OBJFILE (type), are they equivalent?
>
> If they were not the same, then that was already a bug, because
> types generally have the restriction that they can only point to static
> data or to other objects allocated on the same objfile.  Cross-objfile
> pointers are not allowed, to avoid dangling pointers when an objfile is
> destroyed.
>
> Now, a given invocation of the DWARF reader generally deals with a
> single objfile.  So I believe they were the same.  But if they were not,
> then this patch actually improves the situation.
>

OK, patch is good to me then.
Tom Tromey - Jan. 17, 2018, 5:16 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

>> +  temp = XOBNEW (&TYPE_OBJFILE (type)->objfile_obstack, struct dynamic_prop_list);

Simon> This line is now too long.

I fixed this locally.

Tom

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0c878d9aba..e9b6023047 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@ 
 2018-01-05  Tom Tromey  <tom@tromey.com>
 
+	* gdbtypes.h (add_dyn_prop): Remove objfile parameter.
+	* gdbtypes.c (add_dyn_prop): Remove objfile parameter.
+	(create_array_type_with_stride): Update.
+	* dwarf2read.c (set_die_type): Update.
+
+2018-01-05  Tom Tromey  <tom@tromey.com>
+
 	* dwarf2read.c (delayed_method_info): Remove typedef.
 	(dwarf2_cu::method_info): Now a std::vector.
 	(add_to_method_list): Update.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 998c8479ea..3807251970 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -25116,7 +25116,7 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   if (attr_form_is_block (attr))
     {
       if (attr_to_dynamic_prop (attr, die, cu, &prop))
-        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type, objfile);
+        add_dyn_prop (DYN_PROP_ALLOCATED, prop, type);
     }
   else if (attr != NULL)
     {
@@ -25131,7 +25131,7 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   if (attr_form_is_block (attr))
     {
       if (attr_to_dynamic_prop (attr, die, cu, &prop))
-        add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type, objfile);
+        add_dyn_prop (DYN_PROP_ASSOCIATED, prop, type);
     }
   else if (attr != NULL)
     {
@@ -25144,7 +25144,7 @@  set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   /* Read DW_AT_data_location and set in type.  */
   attr = dwarf2_attr (die, DW_AT_data_location, cu);
   if (attr_to_dynamic_prop (attr, die, cu, &prop))
-    add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type, objfile);
+    add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
 
   if (dwarf2_per_objfile->die_type_hash == NULL)
     {
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 7ba62df474..c438696217 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1166,8 +1166,7 @@  create_array_type_with_stride (struct type *result_type,
     (struct field *) TYPE_ZALLOC (result_type, sizeof (struct field));
   TYPE_INDEX_TYPE (result_type) = range_type;
   if (byte_stride_prop != NULL)
-    add_dyn_prop (DYN_PROP_BYTE_STRIDE, *byte_stride_prop, result_type,
-		  TYPE_OBJFILE (result_type));
+    add_dyn_prop (DYN_PROP_BYTE_STRIDE, *byte_stride_prop, result_type);
   else if (bit_stride > 0)
     TYPE_FIELD_BITSIZE (result_type, 0) = bit_stride;
 
@@ -2305,13 +2304,13 @@  get_dyn_prop (enum dynamic_prop_node_kind prop_kind, const struct type *type)
 
 void
 add_dyn_prop (enum dynamic_prop_node_kind prop_kind, struct dynamic_prop prop,
-              struct type *type, struct objfile *objfile)
+              struct type *type)
 {
   struct dynamic_prop_list *temp;
 
   gdb_assert (TYPE_OBJFILE_OWNED (type));
 
-  temp = XOBNEW (&objfile->objfile_obstack, struct dynamic_prop_list);
+  temp = XOBNEW (&TYPE_OBJFILE (type)->objfile_obstack, struct dynamic_prop_list);
   temp->prop_kind = prop_kind;
   temp->prop = prop;
   temp->next = TYPE_DYN_PROP_LIST (type);
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 5942b5ad48..712b16b698 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1841,11 +1841,10 @@  extern struct dynamic_prop *get_dyn_prop
 /* * Given a dynamic property PROP of a given KIND, add this dynamic
    property to the given TYPE.
 
-   This function assumes that TYPE is objfile-owned, and that OBJFILE
-   is the TYPE's objfile.  */
+   This function assumes that TYPE is objfile-owned.  */
 extern void add_dyn_prop
   (enum dynamic_prop_node_kind kind, struct dynamic_prop prop,
-   struct type *type, struct objfile *objfile);
+   struct type *type);
 
 extern void remove_dyn_prop (enum dynamic_prop_node_kind prop_kind,
                              struct type *type);