gdb: replace function pointer with `void *` data with function_view

Message ID 20200807182916.1873936-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • gdb: replace function pointer with `void *` data with function_view
Related show

Commit Message

Kevin Buettner via Gdb-patches Aug. 7, 2020, 6:29 p.m.
Replace the function pointer + `void *` parameters of
dwarf2_fetch_die_loc_sect_off and dwarf2_fetch_die_loc_cu_off with a
function_view parameter.  Change call sites to use a lambda function.
This improves type-safety, so reduces the chances of errors.

gdb/ChangeLog:

	* read.h (dwarf2_fetch_die_loc_sect_off,
	dwarf2_fetch_die_loc_cu_off): Replace function pointer +
	`void *` parameter with function_view.
	* read.c (dwarf2_fetch_die_loc_sect_off,
	dwarf2_fetch_die_loc_cu_off): Likewise.
	* loc.c (get_frame_pc_for_per_cu_dwarf_call): Remove.
	(per_cu_dwarf_call): Adjust.
	(get_frame_address_in_block_wrapper): Remove.
	(indirect_synthetic_pointer): Adjust.
	(get_ax_pc): Remove.
	(dwarf2_compile_expr_to_ax): Adjust.

Change-Id: Ic9b6ced0c4128f2b75ca62e0ed638b0962a22859
---
 gdb/dwarf2/loc.c  | 46 ++++++++++++++++------------------------------
 gdb/dwarf2/read.c | 13 ++++++-------
 gdb/dwarf2/read.h |  8 ++++----
 3 files changed, 26 insertions(+), 41 deletions(-)

-- 
2.28.0

Comments

Tom Tromey Aug. 7, 2020, 8:54 p.m. | #1
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:


Simon> Replace the function pointer + `void *` parameters of
Simon> dwarf2_fetch_die_loc_sect_off and dwarf2_fetch_die_loc_cu_off with a
Simon> function_view parameter.  Change call sites to use a lambda function.
Simon> This improves type-safety, so reduces the chances of errors.

Simon> gdb/ChangeLog:

Simon> 	* read.h (dwarf2_fetch_die_loc_sect_off,
Simon> 	dwarf2_fetch_die_loc_cu_off): Replace function pointer +
Simon> 	`void *` parameter with function_view.
Simon> 	* read.c (dwarf2_fetch_die_loc_sect_off,
Simon> 	dwarf2_fetch_die_loc_cu_off): Likewise.
Simon> 	* loc.c (get_frame_pc_for_per_cu_dwarf_call): Remove.
Simon> 	(per_cu_dwarf_call): Adjust.
Simon> 	(get_frame_address_in_block_wrapper): Remove.
Simon> 	(indirect_synthetic_pointer): Adjust.
Simon> 	(get_ax_pc): Remove.
Simon> 	(dwarf2_compile_expr_to_ax): Adjust.

Looks good, thanks.

Tom
Kevin Buettner via Gdb-patches Aug. 9, 2020, 10:28 p.m. | #2
On 2020-08-07 4:54 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

> 

> Simon> Replace the function pointer + `void *` parameters of

> Simon> dwarf2_fetch_die_loc_sect_off and dwarf2_fetch_die_loc_cu_off with a

> Simon> function_view parameter.  Change call sites to use a lambda function.

> Simon> This improves type-safety, so reduces the chances of errors.

> 

> Simon> gdb/ChangeLog:

> 

> Simon> 	* read.h (dwarf2_fetch_die_loc_sect_off,

> Simon> 	dwarf2_fetch_die_loc_cu_off): Replace function pointer +

> Simon> 	`void *` parameter with function_view.

> Simon> 	* read.c (dwarf2_fetch_die_loc_sect_off,

> Simon> 	dwarf2_fetch_die_loc_cu_off): Likewise.

> Simon> 	* loc.c (get_frame_pc_for_per_cu_dwarf_call): Remove.

> Simon> 	(per_cu_dwarf_call): Adjust.

> Simon> 	(get_frame_address_in_block_wrapper): Remove.

> Simon> 	(indirect_synthetic_pointer): Adjust.

> Simon> 	(get_ax_pc): Remove.

> Simon> 	(dwarf2_compile_expr_to_ax): Adjust.

> 

> Looks good, thanks.

> 

> Tom

> 


Thanks, I pushed it.

Simon

Patch

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 866417eb1b10..72cf9261e0ad 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -600,23 +600,19 @@  func_get_frame_base_dwarf_block (struct symbol *framefunc, CORE_ADDR pc,
 	   framefunc->natural_name ());
 }
 
-static CORE_ADDR
-get_frame_pc_for_per_cu_dwarf_call (void *baton)
-{
-  dwarf_expr_context *ctx = (dwarf_expr_context *) baton;
-
-  return ctx->get_frame_pc ();
-}
-
 static void
 per_cu_dwarf_call (struct dwarf_expr_context *ctx, cu_offset die_offset,
 		   dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile)
 {
   struct dwarf2_locexpr_baton block;
 
+  auto get_frame_pc_from_ctx = [ctx] ()
+    {
+      return ctx->get_frame_pc ();
+    };
+
   block = dwarf2_fetch_die_loc_cu_off (die_offset, per_cu, per_objfile,
-				       get_frame_pc_for_per_cu_dwarf_call,
-				       ctx);
+				       get_frame_pc_from_ctx);
 
   /* DW_OP_call_ref is currently not supported.  */
   gdb_assert (block.per_cu == per_cu);
@@ -2001,14 +1997,6 @@  check_pieced_synthetic_pointer (const struct value *value, LONGEST bit_offset,
   return 1;
 }
 
-/* A wrapper function for get_frame_address_in_block.  */
-
-static CORE_ADDR
-get_frame_address_in_block_wrapper (void *baton)
-{
-  return get_frame_address_in_block ((struct frame_info *) baton);
-}
-
 /* Fetch a DW_AT_const_value through a synthetic pointer.  */
 
 static struct value *
@@ -2052,9 +2040,13 @@  indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
 			    bool resolve_abstract_p)
 {
   /* Fetch the location expression of the DIE we're pointing to.  */
+  auto get_frame_address_in_block_wrapper = [frame] ()
+    {
+     return get_frame_address_in_block (frame);
+    };
   struct dwarf2_locexpr_baton baton
     = dwarf2_fetch_die_loc_sect_off (die, per_cu, per_objfile,
-				     get_frame_address_in_block_wrapper, frame,
+				     get_frame_address_in_block_wrapper,
 				     resolve_abstract_p);
 
   /* Get type of pointed-to DIE.  */
@@ -2994,16 +2986,6 @@  access_memory (struct gdbarch *arch, struct agent_expr *expr, ULONGEST nbits)
     }
 }
 
-/* A helper function to return the frame's PC.  */
-
-static CORE_ADDR
-get_ax_pc (void *baton)
-{
-  struct agent_expr *expr = (struct agent_expr *) baton;
-
-  return expr->scope;
-}
-
 /* Compile a DWARF location expression to an agent expression.
    
    EXPR is the agent expression we are building.
@@ -3655,9 +3637,13 @@  dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc,
 	    uoffset = extract_unsigned_integer (op_ptr, size, byte_order);
 	    op_ptr += size;
 
+	    auto get_frame_pc_from_expr = [expr] ()
+	      {
+		return expr->scope;
+	      };
 	    cu_offset cuoffset = (cu_offset) uoffset;
 	    block = dwarf2_fetch_die_loc_cu_off (cuoffset, per_cu, per_objfile,
-						 get_ax_pc, expr);
+						 get_frame_pc_from_expr);
 
 	    /* DW_OP_call_ref is currently not supported.  */
 	    gdb_assert (block.per_cu == per_cu);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ff1c25b9487c..0ac8533263aa 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -22973,8 +22973,8 @@  struct dwarf2_locexpr_baton
 dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
 			       dwarf2_per_cu_data *per_cu,
 			       dwarf2_per_objfile *per_objfile,
-			       CORE_ADDR (*get_frame_pc) (void *baton),
-			       void *baton, bool resolve_abstract_p)
+			       gdb::function_view<CORE_ADDR ()> get_frame_pc,
+			       bool resolve_abstract_p)
 {
   struct die_info *die;
   struct attribute *attr;
@@ -23003,7 +23003,7 @@  dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
       && (per_objfile->per_bfd->abstract_to_concrete.find (die->sect_off)
 	  != per_objfile->per_bfd->abstract_to_concrete.end ()))
     {
-      CORE_ADDR pc = (*get_frame_pc) (baton);
+      CORE_ADDR pc = get_frame_pc ();
       CORE_ADDR baseaddr = objfile->text_section_offset ();
       struct gdbarch *gdbarch = objfile->arch ();
 
@@ -23044,7 +23044,7 @@  dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
   else if (attr->form_is_section_offset ())
     {
       struct dwarf2_loclist_baton loclist_baton;
-      CORE_ADDR pc = (*get_frame_pc) (baton);
+      CORE_ADDR pc = get_frame_pc ();
       size_t size;
 
       fill_in_loclist_baton (cu, &loclist_baton, attr);
@@ -23077,13 +23077,12 @@  struct dwarf2_locexpr_baton
 dwarf2_fetch_die_loc_cu_off (cu_offset offset_in_cu,
 			     dwarf2_per_cu_data *per_cu,
 			     dwarf2_per_objfile *per_objfile,
-			     CORE_ADDR (*get_frame_pc) (void *baton),
-			     void *baton)
+			     gdb::function_view<CORE_ADDR ()> get_frame_pc)
 {
   sect_offset sect_off = per_cu->sect_off + to_underlying (offset_in_cu);
 
   return dwarf2_fetch_die_loc_sect_off (sect_off, per_cu, per_objfile,
-					get_frame_pc, baton);
+					get_frame_pc);
 }
 
 /* Write a constant of a given type as target-ordered bytes into
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index cd57d9026ae6..fe5aab0e9a1f 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -28,6 +28,7 @@ 
 #include "filename-seen-cache.h"
 #include "gdb_obstack.h"
 #include "gdbsupport/hash_enum.h"
+#include "gdbsupport/function-view.h"
 #include "psympriv.h"
 
 /* Hold 'maintenance (set|show) dwarf' commands.  */
@@ -662,8 +663,8 @@  CORE_ADDR dwarf2_read_addr_index (dwarf2_per_cu_data *per_cu,
 struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off
   (sect_offset sect_off, dwarf2_per_cu_data *per_cu,
    dwarf2_per_objfile *per_objfile,
-   CORE_ADDR (*get_frame_pc) (void *baton),
-   void *baton, bool resolve_abstract_p = false);
+   gdb::function_view<CORE_ADDR ()> get_frame_pc,
+   bool resolve_abstract_p = false);
 
 /* Like dwarf2_fetch_die_loc_sect_off, but take a CU
    offset.  */
@@ -671,8 +672,7 @@  struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_sect_off
 struct dwarf2_locexpr_baton dwarf2_fetch_die_loc_cu_off
   (cu_offset offset_in_cu, dwarf2_per_cu_data *per_cu,
    dwarf2_per_objfile *per_objfile,
-   CORE_ADDR (*get_frame_pc) (void *baton),
-   void *baton);
+   gdb::function_view<CORE_ADDR ()> get_frame_pc);
 
 /* If the DIE at SECT_OFF in PER_CU has a DW_AT_const_value, return a
    pointer to the constant bytes and set LEN to the length of the