[12/20] Remove DW_ADDR

Message ID 20200328192208.11324-13-tom@tromey.com
State New
Headers show
Series
  • Make DWARF attribute references safe
Related show

Commit Message

Tom Tromey March 28, 2020, 7:22 p.m.
This removes DW_ADDR in favor of accessor methods.

gdb/ChangeLog
2020-03-28  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (read_attribute_reprocess, read_attribute_value)
	(dwarf2_const_value_attr, dump_die_shallow)
	(dwarf2_fetch_constant_bytes): Update.
	* dwarf2/attribute.h (struct attribute) <form_is_ref>: Update
	comment.
	<set_address>: New method.
	(DW_ADDR): Remove.
	* dwarf2/attribute.c (attribute::form_is_ref): Update comment.
---
 gdb/ChangeLog          | 11 +++++++++++
 gdb/dwarf2/attribute.c |  3 +--
 gdb/dwarf2/attribute.h | 17 ++++++++++++++---
 gdb/dwarf2/read.c      | 18 +++++++++++-------
 4 files changed, 37 insertions(+), 12 deletions(-)

-- 
2.17.2

Comments

Simon Marchi March 30, 2020, 3:40 p.m. | #1
On 2020-03-28 3:22 p.m., Tom Tromey wrote:
> @@ -202,6 +203,17 @@ struct attribute

>      requires_reprocessing = 1;

>    }

>  

> +  /* Set this attribute to an address.  */

> +  void set_address (CORE_ADDR addr)

> +  {

> +    gdb_assert (form == DW_FORM_addr

> +		|| ((form == DW_FORM_addrx

> +		     || form == DW_FORM_GNU_addr_index)

> +		    && requires_reprocessing));


I wonder if we should assert

  gdb_assert (!requires_reprocessing);

in attribute::address and attribute::string.  Otherwise, we could miss
reprocessing an attribute and we'd return a bogus value, it seems.

Simon
Tom Tromey April 4, 2020, 2:05 p.m. | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> I wonder if we should assert

Simon>   gdb_assert (!requires_reprocessing);

Simon> in attribute::address and attribute::string.  Otherwise, we could miss
Simon> reprocessing an attribute and we'd return a bogus value, it seems.

Makes sense, I did this.

Tom

Patch

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 73c1ef9f792..94bedf6dbd3 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -128,8 +128,7 @@  attribute::form_is_constant () const
     }
 }
 
-/* DW_ADDR is always stored already as sect_offset; despite for the forms
-   besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file.  */
+/* See attribute.h.  */
 
 bool
 attribute::form_is_ref () const
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index b96fdac5201..972beef9825 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -111,8 +111,9 @@  struct attribute
 
   bool form_is_constant () const;
 
-  /* DW_ADDR is always stored already as sect_offset; despite for the forms
-     besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file.  */
+  /* The address is always stored already as sect_offset; despite for
+     the forms besides DW_FORM_ref_addr it is stored as cu_offset in
+     the DWARF file.  */
 
   bool form_is_ref () const;
 
@@ -202,6 +203,17 @@  struct attribute
     requires_reprocessing = 1;
   }
 
+  /* Set this attribute to an address.  */
+  void set_address (CORE_ADDR addr)
+  {
+    gdb_assert (form == DW_FORM_addr
+		|| ((form == DW_FORM_addrx
+		     || form == DW_FORM_GNU_addr_index)
+		    && requires_reprocessing));
+    u.addr = addr;
+    requires_reprocessing = 0;
+  }
+
 
   ENUM_BITFIELD(dwarf_attribute) name : 15;
 
@@ -231,6 +243,5 @@  struct attribute
 /* Get at parts of an attribute structure.  */
 
 #define DW_UNSND(attr)     ((attr)->u.unsnd)
-#define DW_ADDR(attr)	   ((attr)->u.addr)
 
 #endif /* GDB_DWARF2_ATTRIBUTE_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a5c2c52375d..b010c9c3b01 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18314,7 +18314,8 @@  read_attribute_reprocess (const struct die_reader_specs *reader,
     {
       case DW_FORM_addrx:
       case DW_FORM_GNU_addr_index:
-        DW_ADDR (attr) = read_addr_index (cu, attr->get_unsigned_reprocess ());
+	attr->set_address (read_addr_index (cu,
+					    attr->get_unsigned_reprocess ()));
         break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
@@ -18373,9 +18374,12 @@  read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += bytes_read;
       break;
     case DW_FORM_addr:
-      DW_ADDR (attr) = cu->header.read_address (abfd, info_ptr, &bytes_read);
-      DW_ADDR (attr) = gdbarch_adjust_dwarf2_addr (gdbarch, DW_ADDR (attr));
-      info_ptr += bytes_read;
+      {
+	CORE_ADDR addr = cu->header.read_address (abfd, info_ptr, &bytes_read);
+	addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr);
+	attr->set_address (addr);
+	info_ptr += bytes_read;
+      }
       break;
     case DW_FORM_block2:
       blk = dwarf_alloc_block (cu);
@@ -20468,7 +20472,7 @@  dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
 
 	data[0] = DW_OP_addr;
 	store_unsigned_integer (&data[1], cu_header->addr_size,
-				byte_order, DW_ADDR (attr));
+				byte_order, attr->address ());
 	data[cu_header->addr_size + 1] = DW_OP_stack_value;
       }
       break;
@@ -21342,7 +21346,7 @@  dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_addrx:
 	case DW_FORM_GNU_addr_index:
 	  fprintf_unfiltered (f, "address: ");
-	  fputs_filtered (hex_string (DW_ADDR (&die->attrs[i])), f);
+	  fputs_filtered (hex_string (die->attrs[i].address ()), f);
 	  break;
 	case DW_FORM_block2:
 	case DW_FORM_block4:
@@ -21783,7 +21787,7 @@  dwarf2_fetch_constant_bytes (sect_offset sect_off,
 
 	*len = cu->header.addr_size;
 	tem = (gdb_byte *) obstack_alloc (obstack, *len);
-	store_unsigned_integer (tem, *len, byte_order, DW_ADDR (attr));
+	store_unsigned_integer (tem, *len, byte_order, attr->address ());
 	result = tem;
       }
       break;