[1/2] Make gdb property batons type-safe

Message ID 20230301-submit-baton-stuff-v1-1-567d536a8245@adacore.com
State Committed
Commit 802dace16f8c6dfa3200381669015a7dccbd5e28
Headers
Series Two minor dynamic property fixes |

Commit Message

Tom Tromey March 1, 2023, 3:18 p.m. UTC
  gdbtypes treats dynamic property batons as 'void *', but in actuality
the only users all use dwarf2_property_baton.  This patch changes this
code to be type-safe.  If a new type is needed here, it seems like
that too could be done in a type-safe way.
---
 gdb/dwarf2/loc.c | 12 ++++--------
 gdb/gdbtypes.h   | 11 ++++++-----
 2 files changed, 10 insertions(+), 13 deletions(-)
  

Comments

Simon Marchi March 1, 2023, 6:44 p.m. UTC | #1
On 3/1/23 10:18, Tom Tromey via Gdb-patches wrote:
> gdbtypes treats dynamic property batons as 'void *', but in actuality
> the only users all use dwarf2_property_baton.  This patch changes this
> code to be type-safe.  If a new type is needed here, it seems like
> that too could be done in a type-safe way.

I don't mind doing this, because in practice the DWARF reader is the
only one to use that data pointer.  But just wondering, what would be
the "right" way to implement this pattern in a type-safe way, if
multiple debug info readers wanted to use that field?

Simon
  
Tom Tromey March 1, 2023, 10:28 p.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 3/1/23 10:18, Tom Tromey via Gdb-patches wrote:
>> gdbtypes treats dynamic property batons as 'void *', but in actuality
>> the only users all use dwarf2_property_baton.  This patch changes this
>> code to be type-safe.  If a new type is needed here, it seems like
>> that too could be done in a type-safe way.

Simon> I don't mind doing this, because in practice the DWARF reader is the
Simon> only one to use that data pointer.  But just wondering, what would be
Simon> the "right" way to implement this pattern in a type-safe way, if
Simon> multiple debug info readers wanted to use that field?

There's several possibilities but one would be to just add a new element
to the union and new setters/getters.

Tom
  

Patch

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 4727651027b..bf582bcfeff 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1649,8 +1649,7 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
     {
     case PROP_LOCEXPR:
       {
-	const struct dwarf2_property_baton *baton
-	  = (const struct dwarf2_property_baton *) prop->baton ();
+	const struct dwarf2_property_baton *baton = prop->baton ();
 	gdb_assert (baton->property_type != NULL);
 
 	bool is_reference = baton->locexpr.is_reference;
@@ -1692,8 +1691,7 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
 
     case PROP_LOCLIST:
       {
-	struct dwarf2_property_baton *baton
-	  = (struct dwarf2_property_baton *) prop->baton ();
+	struct dwarf2_property_baton *baton = prop->baton ();
 	CORE_ADDR pc;
 	const gdb_byte *data;
 	struct value *val;
@@ -1724,8 +1722,7 @@  dwarf2_evaluate_property (const struct dynamic_prop *prop,
 
     case PROP_ADDR_OFFSET:
       {
-	struct dwarf2_property_baton *baton
-	  = (struct dwarf2_property_baton *) prop->baton ();
+	struct dwarf2_property_baton *baton = prop->baton ();
 	const struct property_addr_info *pinfo;
 	struct value *val;
 
@@ -1775,8 +1772,7 @@  dwarf2_compile_property_to_c (string_file *stream,
 			      CORE_ADDR pc,
 			      struct symbol *sym)
 {
-  struct dwarf2_property_baton *baton
-    = (struct dwarf2_property_baton *) prop->baton ();
+  struct dwarf2_property_baton *baton = prop->baton ();
   const gdb_byte *data;
   size_t size;
   dwarf2_per_cu_data *per_cu;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c2253310666..701a64d457a 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -64,6 +64,7 @@  struct value_print_options;
 struct language_defn;
 struct dwarf2_per_cu_data;
 struct dwarf2_per_objfile;
+struct dwarf2_property_baton;
 
 /* Some macros for char-based bitfields.  */
 
@@ -289,7 +290,7 @@  union dynamic_prop_data
 
   /* Storage for dynamic property.  */
 
-  void *baton;
+  dwarf2_property_baton *baton;
 
   /* Storage of variant parts for a type.  A type with variant parts
      has all its fields "linearized" -- stored in a single field
@@ -339,7 +340,7 @@  struct dynamic_prop
     m_data.const_val = const_val;
   }
 
-  void *baton () const
+  dwarf2_property_baton *baton () const
   {
     gdb_assert (m_kind == PROP_LOCEXPR
 		|| m_kind == PROP_LOCLIST
@@ -348,19 +349,19 @@  struct dynamic_prop
     return m_data.baton;
   }
 
-  void set_locexpr (void *baton)
+  void set_locexpr (dwarf2_property_baton *baton)
   {
     m_kind = PROP_LOCEXPR;
     m_data.baton = baton;
   }
 
-  void set_loclist (void *baton)
+  void set_loclist (dwarf2_property_baton *baton)
   {
     m_kind = PROP_LOCLIST;
     m_data.baton = baton;
   }
 
-  void set_addr_offset (void *baton)
+  void set_addr_offset (dwarf2_property_baton *baton)
   {
     m_kind = PROP_ADDR_OFFSET;
     m_data.baton = baton;