[PATCH] Use enum for gdbarch_call_dummy_location

[PATCH] Use enum for gdbarch_call_dummy_location

[Tom Tromey]

While debugging, I noticed that gdbarch_call_dummy_location is an
'int', and that the values are two defines.  Since I had to look up
the value by hand, I figured it would make sense to replace these with
an enum.

gdb/ChangeLog
2021-03-04  Tom Tromey  <tromey@adacore.com>

	* gdbarch.h, gdbarch.c: Rebuild.
	* inferior.h (ON_STACK, AT_ENTRY_POINT): Remove.
	* gdbarch.sh (enum call_dummy_location): New.
	(call_dummy_location): Change type.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/gdbarch.c  |  8 ++++----
 gdb/gdbarch.h  | 14 ++++++++++++--
 gdb/gdbarch.sh | 12 +++++++++++-
 gdb/inferior.h |  4 ----
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 43d1c154a7f..0ba99cf884c 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -207,7 +207,7 @@ struct gdbarch
   gdbarch_dummy_id_ftype *dummy_id;
   int deprecated_fp_regnum;
   gdbarch_push_dummy_call_ftype *push_dummy_call;
-  int call_dummy_location;
+  enum call_dummy_location call_dummy_location;
   gdbarch_push_dummy_code_ftype *push_dummy_code;
   gdbarch_code_of_frame_writable_ftype *code_of_frame_writable;
   gdbarch_print_registers_info_ftype *print_registers_info;
@@ -2419,7 +2419,7 @@ set_gdbarch_push_dummy_call (struct gdbarch *gdbarch,
   gdbarch->push_dummy_call = push_dummy_call;
 }
 
-int
+enum call_dummy_location
 gdbarch_call_dummy_location (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
@@ -2431,7 +2431,7 @@ gdbarch_call_dummy_location (struct gdbarch *gdbarch)
 
 void
 set_gdbarch_call_dummy_location (struct gdbarch *gdbarch,
-                                 int call_dummy_location)
+                                 enum call_dummy_location call_dummy_location)
 {
   gdbarch->call_dummy_location = call_dummy_location;
 }
@@ -5314,7 +5314,7 @@ struct gdbarch_data_registry
   struct gdbarch_data_registration *registrations;
 };
 
-static struct gdbarch_data_registry gdbarch_data_registry =
+struct gdbarch_data_registry gdbarch_data_registry =
 {
   0, NULL,
 };
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 2fef567c06f..77c1b4f32de 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -117,6 +117,16 @@ enum function_call_return_method
   return_method_struct,
 };
 
+/* Possible values for gdbarch_call_dummy_location.  */
+
+enum call_dummy_location
+{
+  /* Put the call dummy on the stack.  */
+  ON_STACK,
+  /* Put the call dummy at the entry point.  */
+  AT_ENTRY_POINT,
+};
+
 
 
 /* The following are pre-initialized by GDBARCH.  */
@@ -420,8 +430,8 @@ typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, stru
 extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr);
 extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call);
 
-extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch);
-extern void set_gdbarch_call_dummy_location (struct gdbarch *gdbarch, int call_dummy_location);
+extern enum call_dummy_location gdbarch_call_dummy_location (struct gdbarch *gdbarch);
+extern void set_gdbarch_call_dummy_location (struct gdbarch *gdbarch, enum call_dummy_location call_dummy_location);
 
 extern bool gdbarch_push_dummy_code_p (struct gdbarch *gdbarch);
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index e7c96159241..3796f097438 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -459,7 +459,7 @@ m;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame;;default_dum
 v;int;deprecated_fp_regnum;;;-1;-1;;0
 
 M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, return_method, struct_addr
-v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0
+v;enum call_dummy_location;call_dummy_location;;;;AT_ENTRY_POINT;;0
 M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
 
 # Return true if the code of FRAME is writable.
@@ -1362,6 +1362,16 @@ enum function_call_return_method
   return_method_struct,
 };
 
+/* Possible values for gdbarch_call_dummy_location.  */
+
+enum call_dummy_location
+{
+  /* Put the call dummy on the stack.  */
+  ON_STACK,
+  /* Put the call dummy at the entry point.  */
+  AT_ENTRY_POINT,
+};
+
 EOF
 
 # function typedef's
diff --git a/gdb/inferior.h b/gdb/inferior.h
index b8d5ff94fc5..16c179061fa 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -277,10 +277,6 @@ enum stop_kind
   };
 
 \f
-/* Possible values for gdbarch_call_dummy_location.  */
-#define ON_STACK 1
-#define AT_ENTRY_POINT 4
-
 /* Base class for target-specific inferior data.  */
 
 struct private_inferior
-- 
2.26.2

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> While debugging, I noticed that gdbarch_call_dummy_location is an
Tom> 'int', and that the values are two defines.  Since I had to look up
Tom> the value by hand, I figured it would make sense to replace these with
Tom> an enum.

Looking at this a little more, I wonder if we should simply remove this
from gdbarch and decide what to do based on gdbarch_push_dummy_code_p.
From what I can tell, this is the only thing this is used for.
And, it's only used in a single spot in infcall.c.

Tom

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2021-03-04 12:49:52 -0700]:

> While debugging, I noticed that gdbarch_call_dummy_location is an
> 'int', and that the values are two defines.  Since I had to look up
> the value by hand, I figured it would make sense to replace these with
> an enum.
> 
> gdb/ChangeLog
> 2021-03-04  Tom Tromey  <tromey@adacore.com>
> 
> 	* gdbarch.h, gdbarch.c: Rebuild.
> 	* inferior.h (ON_STACK, AT_ENTRY_POINT): Remove.
> 	* gdbarch.sh (enum call_dummy_location): New.
> 	(call_dummy_location): Change type.

LGTM.  I know you're thinking about rewriting this.  But if you don't
this patch looks like a good improvement.

Thanks,
Andrew


> ---
>  gdb/ChangeLog  |  7 +++++++
>  gdb/gdbarch.c  |  8 ++++----
>  gdb/gdbarch.h  | 14 ++++++++++++--
>  gdb/gdbarch.sh | 12 +++++++++++-
>  gdb/inferior.h |  4 ----
>  5 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 43d1c154a7f..0ba99cf884c 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -207,7 +207,7 @@ struct gdbarch
>    gdbarch_dummy_id_ftype *dummy_id;
>    int deprecated_fp_regnum;
>    gdbarch_push_dummy_call_ftype *push_dummy_call;
> -  int call_dummy_location;
> +  enum call_dummy_location call_dummy_location;
>    gdbarch_push_dummy_code_ftype *push_dummy_code;
>    gdbarch_code_of_frame_writable_ftype *code_of_frame_writable;
>    gdbarch_print_registers_info_ftype *print_registers_info;
> @@ -2419,7 +2419,7 @@ set_gdbarch_push_dummy_call (struct gdbarch *gdbarch,
>    gdbarch->push_dummy_call = push_dummy_call;
>  }
>  
> -int
> +enum call_dummy_location
>  gdbarch_call_dummy_location (struct gdbarch *gdbarch)
>  {
>    gdb_assert (gdbarch != NULL);
> @@ -2431,7 +2431,7 @@ gdbarch_call_dummy_location (struct gdbarch *gdbarch)
>  
>  void
>  set_gdbarch_call_dummy_location (struct gdbarch *gdbarch,
> -                                 int call_dummy_location)
> +                                 enum call_dummy_location call_dummy_location)
>  {
>    gdbarch->call_dummy_location = call_dummy_location;
>  }
> @@ -5314,7 +5314,7 @@ struct gdbarch_data_registry
>    struct gdbarch_data_registration *registrations;
>  };
>  
> -static struct gdbarch_data_registry gdbarch_data_registry =
> +struct gdbarch_data_registry gdbarch_data_registry =
>  {
>    0, NULL,
>  };
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 2fef567c06f..77c1b4f32de 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -117,6 +117,16 @@ enum function_call_return_method
>    return_method_struct,
>  };
>  
> +/* Possible values for gdbarch_call_dummy_location.  */
> +
> +enum call_dummy_location
> +{
> +  /* Put the call dummy on the stack.  */
> +  ON_STACK,
> +  /* Put the call dummy at the entry point.  */
> +  AT_ENTRY_POINT,
> +};
> +
>  
>  
>  /* The following are pre-initialized by GDBARCH.  */
> @@ -420,8 +430,8 @@ typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, stru
>  extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr);
>  extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call);
>  
> -extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch);
> -extern void set_gdbarch_call_dummy_location (struct gdbarch *gdbarch, int call_dummy_location);
> +extern enum call_dummy_location gdbarch_call_dummy_location (struct gdbarch *gdbarch);
> +extern void set_gdbarch_call_dummy_location (struct gdbarch *gdbarch, enum call_dummy_location call_dummy_location);
>  
>  extern bool gdbarch_push_dummy_code_p (struct gdbarch *gdbarch);
>  
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index e7c96159241..3796f097438 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -459,7 +459,7 @@ m;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame;;default_dum
>  v;int;deprecated_fp_regnum;;;-1;-1;;0
>  
>  M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, return_method, struct_addr
> -v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0
> +v;enum call_dummy_location;call_dummy_location;;;;AT_ENTRY_POINT;;0
>  M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
>  
>  # Return true if the code of FRAME is writable.
> @@ -1362,6 +1362,16 @@ enum function_call_return_method
>    return_method_struct,
>  };
>  
> +/* Possible values for gdbarch_call_dummy_location.  */
> +
> +enum call_dummy_location
> +{
> +  /* Put the call dummy on the stack.  */
> +  ON_STACK,
> +  /* Put the call dummy at the entry point.  */
> +  AT_ENTRY_POINT,
> +};
> +
>  EOF
>  
>  # function typedef's
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index b8d5ff94fc5..16c179061fa 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -277,10 +277,6 @@ enum stop_kind
>    };
>  
>  \f
> -/* Possible values for gdbarch_call_dummy_location.  */
> -#define ON_STACK 1
> -#define AT_ENTRY_POINT 4
> -
>  /* Base class for target-specific inferior data.  */
>  
>  struct private_inferior
> -- 
> 2.26.2
> 

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Tom Tromey]

>> * gdbarch.h, gdbarch.c: Rebuild.
>> * inferior.h (ON_STACK, AT_ENTRY_POINT): Remove.
>> * gdbarch.sh (enum call_dummy_location): New.
>> (call_dummy_location): Change type.

Andrew> LGTM.  I know you're thinking about rewriting this.

I looked at all the callers of set_gdbarch_push_dummy_code, and I
discovered that arc and cris set this, but don't use ON_STACK.

I guess we could delete these if I do the rewrite, to preserve existing
behavior.

Tom

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Tom Tromey]

Andrew> LGTM.  I know you're thinking about rewriting this.

Tom> I looked at all the callers of set_gdbarch_push_dummy_code, and I
Tom> discovered that arc and cris set this, but don't use ON_STACK.

Tom> I guess we could delete these if I do the rewrite, to preserve existing
Tom> behavior.

Here's what the other approach looks like.  This found some latent
bugs -- basically the current code isn't "DRY"-ready so of course some
divergences crept in.

Let me know what you think.

Tom

commit 1fae1e13c704ac536bf50c6644978e69e3d95975
Author: Tom Tromey <tromey@adacore.com>
Date:   Thu Mar 4 12:46:45 2021 -0700

    Remove gdbarch_call_dummy_location
    
    I noticed that gdbarch_call_dummy_location is only used in one spot.
    If it returns ON_STACK, then gdbarch_push_dummy_code is used;
    otherwise, gdbarch_push_dummy_call is used.
    
    Rather than requiring the two settings to be synchronized, it seemed
    better to use the presence or absence of gdbarch_push_dummy_code to
    decide.  This patch removes gdbarch_call_dummy_location in favor of
    this approach.
    
    Doing this revealed some latent bugs:
    
    * arc and cris set the push_dummy_code method, but did not use
      ON_STACK.  Therefore, push_dummy_code was never used.  This patch
      removes these functions, to preserve current behavior.
    
    * bpf and microblaze set ON_STACK but do not set push_dummy_code.  I
      believe this should result in an assertion failure in
      push_dummy_code.  This patch removes the ON_STACK setting -- the
      result can't be any worse.
    
    Also, sparc64-tdep.c sets gdbarch_push_dummy_code to NULL.  I wasn't
    sure if this is needed (if sparc32_gdbarch_init is called first, it
    is), so I've left this call in place.
    
    gdb/ChangeLog
    2021-03-05  Tom Tromey  <tromey@adacore.com>
    
            * infcall.c (push_dummy_code): Remove.
            (call_function_by_hand_dummy): Update.
            * dicos-tdep.c (dicos_init_abi): Update comment.
            * sparc-tdep.c (sparc32_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * or1k-tdep.c (or1k_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * score-tdep.c (score_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * riscv-tdep.c (riscv_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * mips-tdep.c (mips_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * i386-tdep.c (i386_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * bpf-tdep.c (bpf_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * microblaze-tdep.c (microblaze_gdbarch_init): Don't call
            set_gdbarch_call_dummy_location.
            * sparc64-tdep.c (sparc64_init_abi): Don't call
            set_gdbarch_push_dummy_code.
            * cris-tdep.c (cris_push_dummy_code): Remove.
            (cris_gdbarch_init): Update.
            * arc-tdep.c (arc_push_dummy_code): Remove.
            (arc_gdbarch_init): Update.
            * gdbarch.h, gdbarch.c: Rebuild.
            * inferior.h (ON_STACK, AT_ENTRY_POINT): Remove.
            * gdbarch.sh (call_dummy_location): Remove.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0126083241b..0567fd4ca96 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,34 @@
+2021-03-05  Tom Tromey  <tromey@adacore.com>
+
+	* infcall.c (push_dummy_code): Remove.
+	(call_function_by_hand_dummy): Update.
+	* dicos-tdep.c (dicos_init_abi): Update comment.
+	* sparc-tdep.c (sparc32_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* or1k-tdep.c (or1k_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* score-tdep.c (score_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* riscv-tdep.c (riscv_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* mips-tdep.c (mips_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* i386-tdep.c (i386_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* bpf-tdep.c (bpf_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* microblaze-tdep.c (microblaze_gdbarch_init): Don't call
+	set_gdbarch_call_dummy_location.
+	* sparc64-tdep.c (sparc64_init_abi): Don't call
+	set_gdbarch_push_dummy_code.
+	* cris-tdep.c (cris_push_dummy_code): Remove.
+	(cris_gdbarch_init): Update.
+	* arc-tdep.c (arc_push_dummy_code): Remove.
+	(arc_gdbarch_init): Update.
+	* gdbarch.h, gdbarch.c: Rebuild.
+	* inferior.h (ON_STACK, AT_ENTRY_POINT): Remove.
+	* gdbarch.sh (call_dummy_location): Remove.
+
 2021-03-04  Tom Tromey  <tromey@adacore.com>
 
 	* ada-lang.c (struct match_data) <found_sym>: Now bool.
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index 8a1da1a9868..c6738901d9e 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -826,33 +826,6 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   return sp;
 }
 
-/* Implement the "push_dummy_code" gdbarch method.
-
-   We don't actually push any code.  We just identify where a breakpoint can
-   be inserted to which we are can return and the resume address where we
-   should be called.
-
-   ARC does not necessarily have an executable stack, so we can't put the
-   return breakpoint there.  Instead we put it at the entry point of the
-   function.  This means the SP is unchanged.
-
-   SP is a current stack pointer FUNADDR is an address of the function to be
-   called.  ARGS is arguments to pass.  NARGS is a number of args to pass.
-   VALUE_TYPE is a type of value returned.  REAL_PC is a resume address when
-   the function is called.  BP_ADDR is an address where breakpoint should be
-   set.  Returns the updated stack pointer.  */
-
-static CORE_ADDR
-arc_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr,
-		     struct value **args, int nargs, struct type *value_type,
-		     CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
-		     struct regcache *regcache)
-{
-  *real_pc = funaddr;
-  *bp_addr = entry_point_address ();
-  return sp;
-}
-
 /* Implement the "cannot_fetch_register" gdbarch method.  */
 
 static int
@@ -2327,7 +2300,6 @@ arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fp0_regnum (gdbarch, -1);	/* No FPU registers.  */
 
   set_gdbarch_push_dummy_call (gdbarch, arc_push_dummy_call);
-  set_gdbarch_push_dummy_code (gdbarch, arc_push_dummy_code);
 
   set_gdbarch_cannot_fetch_register (gdbarch, arc_cannot_fetch_register);
   set_gdbarch_cannot_store_register (gdbarch, arc_cannot_store_register);
diff --git a/gdb/bpf-tdep.c b/gdb/bpf-tdep.c
index 352a22198e9..6ade6c17816 100644
--- a/gdb/bpf-tdep.c
+++ b/gdb/bpf-tdep.c
@@ -336,7 +336,6 @@ bpf_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, bpf_dwarf2_reg_to_regnum);
 
   /* Call dummy code.  */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_dummy_id (gdbarch, bpf_dummy_id);
   set_gdbarch_push_dummy_call (gdbarch, bpf_push_dummy_call);
 
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index e1c141865cc..89537e4065b 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -769,23 +769,6 @@ cris_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
   return sp & ~3;
 }
 
-static CORE_ADDR
-cris_push_dummy_code (struct gdbarch *gdbarch,
-		      CORE_ADDR sp, CORE_ADDR funaddr,
-		      struct value **args, int nargs,
-		      struct type *value_type,
-		      CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
-		      struct regcache *regcache)
-{
-  /* Allocate space sufficient for a breakpoint.  */
-  sp = (sp - 4) & ~3;
-  /* Store the address of that breakpoint */
-  *bp_addr = sp;
-  /* CRIS always starts the call at the callee's entry point.  */
-  *real_pc = funaddr;
-  return sp;
-}
-
 static CORE_ADDR
 cris_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		      struct regcache *regcache, CORE_ADDR bp_addr,
@@ -4030,7 +4013,6 @@ cris_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Dummy frame functions (shared between CRISv10 and CRISv32 since they
      have the same ABI).  */
-  set_gdbarch_push_dummy_code (gdbarch, cris_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, cris_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, cris_frame_align);
   set_gdbarch_skip_prologue (gdbarch, cris_skip_prologue);
diff --git a/gdb/dicos-tdep.c b/gdb/dicos-tdep.c
index f9408224f1e..9fc11a6612e 100644
--- a/gdb/dicos-tdep.c
+++ b/gdb/dicos-tdep.c
@@ -42,7 +42,7 @@ dicos_init_abi (struct gdbarch *gdbarch)
   /* There's no (standard definition of) entry point or a guaranteed
      text location with a symbol where to place the call dummy, so we
      need it on the stack.  Rely on i386_gdbarch_init used also for
-     amd64 to set up ON_STACK inferior calls.  */
+     amd64 to set up on-stack inferior calls.  */
 
   /* DICOS rewinds the PC itself.  */
   set_gdbarch_decr_pc_after_break (gdbarch, 0);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 43d1c154a7f..57f47574368 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -207,7 +207,6 @@ struct gdbarch
   gdbarch_dummy_id_ftype *dummy_id;
   int deprecated_fp_regnum;
   gdbarch_push_dummy_call_ftype *push_dummy_call;
-  int call_dummy_location;
   gdbarch_push_dummy_code_ftype *push_dummy_code;
   gdbarch_code_of_frame_writable_ftype *code_of_frame_writable;
   gdbarch_print_registers_info_ftype *print_registers_info;
@@ -410,7 +409,6 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->dwarf2_reg_to_regnum = no_op_reg_to_regnum;
   gdbarch->dummy_id = default_dummy_id;
   gdbarch->deprecated_fp_regnum = -1;
-  gdbarch->call_dummy_location = AT_ENTRY_POINT;
   gdbarch->code_of_frame_writable = default_code_of_frame_writable;
   gdbarch->print_registers_info = default_print_registers_info;
   gdbarch->print_float_info = default_print_float_info;
@@ -581,7 +579,6 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of dummy_id, invalid_p == 0 */
   /* Skip verify of deprecated_fp_regnum, invalid_p == 0 */
   /* Skip verify of push_dummy_call, has predicate.  */
-  /* Skip verify of call_dummy_location, invalid_p == 0 */
   /* Skip verify of push_dummy_code, has predicate.  */
   /* Skip verify of code_of_frame_writable, invalid_p == 0 */
   /* Skip verify of print_registers_info, invalid_p == 0 */
@@ -839,9 +836,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: byte_order_for_code = %s\n",
                       plongest (gdbarch->byte_order_for_code));
-  fprintf_unfiltered (file,
-                      "gdbarch_dump: call_dummy_location = %s\n",
-                      plongest (gdbarch->call_dummy_location));
   fprintf_unfiltered (file,
                       "gdbarch_dump: cannot_fetch_register = <%s>\n",
                       host_address_to_string (gdbarch->cannot_fetch_register));
@@ -2419,23 +2413,6 @@ set_gdbarch_push_dummy_call (struct gdbarch *gdbarch,
   gdbarch->push_dummy_call = push_dummy_call;
 }
 
-int
-gdbarch_call_dummy_location (struct gdbarch *gdbarch)
-{
-  gdb_assert (gdbarch != NULL);
-  /* Skip verify of call_dummy_location, invalid_p == 0 */
-  if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_call_dummy_location called\n");
-  return gdbarch->call_dummy_location;
-}
-
-void
-set_gdbarch_call_dummy_location (struct gdbarch *gdbarch,
-                                 int call_dummy_location)
-{
-  gdbarch->call_dummy_location = call_dummy_location;
-}
-
 bool
 gdbarch_push_dummy_code_p (struct gdbarch *gdbarch)
 {
@@ -5314,7 +5291,7 @@ struct gdbarch_data_registry
   struct gdbarch_data_registration *registrations;
 };
 
-static struct gdbarch_data_registry gdbarch_data_registry =
+struct gdbarch_data_registry gdbarch_data_registry =
 {
   0, NULL,
 };
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 2fef567c06f..26e9590f2f9 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -420,9 +420,6 @@ typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, stru
 extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr);
 extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call);
 
-extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch);
-extern void set_gdbarch_call_dummy_location (struct gdbarch *gdbarch, int call_dummy_location);
-
 extern bool gdbarch_push_dummy_code_p (struct gdbarch *gdbarch);
 
 typedef CORE_ADDR (gdbarch_push_dummy_code_ftype) (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index e7c96159241..844f2721b43 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -459,7 +459,6 @@ m;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame;;default_dum
 v;int;deprecated_fp_regnum;;;-1;-1;;0
 
 M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, function_call_return_method return_method, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, return_method, struct_addr
-v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0
 M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
 
 # Return true if the code of FRAME is writable.
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 6386716c3b8..5a2ecca433e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8550,7 +8550,6 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_get_longjmp_target (gdbarch, i386_get_longjmp_target);
 
   /* Call dummy code.  */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, i386_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, i386_frame_align);
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 2332f293a1e..e3679f4a1e7 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -342,25 +342,6 @@ find_function_addr (struct value *function,
   return funaddr + gdbarch_deprecated_function_start_offset (gdbarch);
 }
 
-/* For CALL_DUMMY_ON_STACK, push a breakpoint sequence that the called
-   function returns to.  */
-
-static CORE_ADDR
-push_dummy_code (struct gdbarch *gdbarch,
-		 CORE_ADDR sp, CORE_ADDR funaddr,
-		 gdb::array_view<value *> args,
-		 struct type *value_type,
-		 CORE_ADDR *real_pc, CORE_ADDR *bp_addr,
-		 struct regcache *regcache)
-{
-  gdb_assert (gdbarch_push_dummy_code_p (gdbarch));
-
-  return gdbarch_push_dummy_code (gdbarch, sp, funaddr,
-				  args.data (), args.size (),
-				  value_type, real_pc, bp_addr,
-				  regcache);
-}
-
 /* See infcall.h.  */
 
 void
@@ -955,61 +936,57 @@ call_function_by_hand_dummy (struct value *function,
   gdb::observers::inferior_call_pre.notify (inferior_ptid, funaddr);
 
   /* Determine the location of the breakpoint (and possibly other
-     stuff) that the called function will return to.  The SPARC, for a
-     function returning a structure or union, needs to make space for
-     not just the breakpoint but also an extra word containing the
-     size (?) of the structure being passed.  */
+     stuff) that the called function will return to.  */
 
-  switch (gdbarch_call_dummy_location (gdbarch))
+  if (gdbarch_push_dummy_code_p (gdbarch))
     {
-    case ON_STACK:
-      {
-	const gdb_byte *bp_bytes;
-	CORE_ADDR bp_addr_as_address;
-	int bp_size;
-
-	/* Be careful BP_ADDR is in inferior PC encoding while
-	   BP_ADDR_AS_ADDRESS is a plain memory address.  */
-
-	sp = push_dummy_code (gdbarch, sp, funaddr, args,
-			      target_values_type, &real_pc, &bp_addr,
-			      get_current_regcache ());
-
-	/* Write a legitimate instruction at the point where the infcall
-	   breakpoint is going to be inserted.  While this instruction
-	   is never going to be executed, a user investigating the
-	   memory from GDB would see this instruction instead of random
-	   uninitialized bytes.  We chose the breakpoint instruction
-	   as it may look as the most logical one to the user and also
-	   valgrind 3.7.0 needs it for proper vgdb inferior calls.
-
-	   If software breakpoints are unsupported for this target we
-	   leave the user visible memory content uninitialized.  */
-
-	bp_addr_as_address = bp_addr;
-	bp_bytes = gdbarch_breakpoint_from_pc (gdbarch, &bp_addr_as_address,
-					       &bp_size);
-	if (bp_bytes != NULL)
-	  write_memory (bp_addr_as_address, bp_bytes, bp_size);
-      }
-      break;
-    case AT_ENTRY_POINT:
-      {
-	CORE_ADDR dummy_addr;
+      /* If gdbarch_push_dummy_code is defined, use it.  This means
+	 the call dummy is on the stack.  */
+
+      const gdb_byte *bp_bytes;
+      CORE_ADDR bp_addr_as_address;
+      int bp_size;
+
+      /* Be careful BP_ADDR is in inferior PC encoding while
+	 BP_ADDR_AS_ADDRESS is a plain memory address.  */
+
+
+      sp = gdbarch_push_dummy_code (gdbarch, sp, funaddr,
+				    args.data (), args.size (),
+				    target_values_type, &real_pc, &bp_addr,
+				    get_current_regcache ());
+
+      /* Write a legitimate instruction at the point where the infcall
+	 breakpoint is going to be inserted.  While this instruction
+	 is never going to be executed, a user investigating the
+	 memory from GDB would see this instruction instead of random
+	 uninitialized bytes.  We chose the breakpoint instruction
+	 as it may look as the most logical one to the user and also
+	 valgrind 3.7.0 needs it for proper vgdb inferior calls.
+
+	 If software breakpoints are unsupported for this target we
+	 leave the user visible memory content uninitialized.  */
+
+      bp_addr_as_address = bp_addr;
+      bp_bytes = gdbarch_breakpoint_from_pc (gdbarch, &bp_addr_as_address,
+					     &bp_size);
+      if (bp_bytes != NULL)
+	write_memory (bp_addr_as_address, bp_bytes, bp_size);
+    }
+  else
+    {
+      /* Otherwise, put the call dummy at the entry point.  */
+      CORE_ADDR dummy_addr;
 
-	real_pc = funaddr;
-	dummy_addr = entry_point_address ();
+      real_pc = funaddr;
+      dummy_addr = entry_point_address ();
 
-	/* A call dummy always consists of just a single breakpoint, so
-	   its address is the same as the address of the dummy.
+      /* A call dummy always consists of just a single breakpoint, so
+	 its address is the same as the address of the dummy.
 
-	   The actual breakpoint is inserted separatly so there is no need to
-	   write that out.  */
-	bp_addr = dummy_addr;
-	break;
-      }
-    default:
-      internal_error (__FILE__, __LINE__, _("bad switch"));
+	 The actual breakpoint is inserted separatly so there is no need to
+	 write that out.  */
+      bp_addr = dummy_addr;
     }
 
   /* Coerce the arguments and handle pass-by-reference.
diff --git a/gdb/inferior.h b/gdb/inferior.h
index b8d5ff94fc5..16c179061fa 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -277,10 +277,6 @@ enum stop_kind
   };
 
 \f
-/* Possible values for gdbarch_call_dummy_location.  */
-#define ON_STACK 1
-#define AT_ENTRY_POINT 4
-
 /* Base class for target-specific inferior data.  */
 
 struct private_inferior
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 5419dd329c5..d30b6d7afe7 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -712,9 +712,6 @@ microblaze_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Map Dwarf2 registers to GDB registers.  */
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, microblaze_dwarf2_reg_to_regnum);
 
-  /* Call dummy code.  */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
-
   set_gdbarch_return_value (gdbarch, microblaze_return_value);
   set_gdbarch_stabs_argument_has_addr
     (gdbarch, microblaze_stabs_argument_has_addr);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 61545aed86c..66752ee83c2 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -8650,7 +8650,6 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* MIPS version of CALL_DUMMY.  */
 
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, mips_push_dummy_code);
   set_gdbarch_frame_align (gdbarch, mips_frame_align);
 
diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
index db6c41b32f2..09a0376ba7d 100644
--- a/gdb/or1k-tdep.c
+++ b/gdb/or1k-tdep.c
@@ -1173,7 +1173,6 @@ or1k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_unwind_sp (gdbarch, or1k_unwind_sp);
 
   /* Functions handling dummy frames */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, or1k_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, or1k_push_dummy_call);
 
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index da86ed12716..a159b79e789 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -3567,7 +3567,6 @@ riscv_gdbarch_init (struct gdbarch_info info,
   set_gdbarch_frame_align (gdbarch, riscv_frame_align);
 
   /* Functions handling dummy frames.  */
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, riscv_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, riscv_push_dummy_call);
 
diff --git a/gdb/score-tdep.c b/gdb/score-tdep.c
index 531b308749c..16d3eab2a34 100644
--- a/gdb/score-tdep.c
+++ b/gdb/score-tdep.c
@@ -1506,7 +1506,6 @@ score_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Dummy frame hooks.  */
   set_gdbarch_return_value (gdbarch, score_return_value);
-  set_gdbarch_call_dummy_location (gdbarch, AT_ENTRY_POINT);
   set_gdbarch_push_dummy_call (gdbarch, score_push_dummy_call);
 
   /* Normal frame hooks.  */
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 4f9c679b55c..f33b72c8d12 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1857,7 +1857,6 @@ sparc32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Call dummy code.  */
   set_gdbarch_frame_align (gdbarch, sparc32_frame_align);
-  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
   set_gdbarch_push_dummy_code (gdbarch, sparc32_push_dummy_code);
   set_gdbarch_push_dummy_call (gdbarch, sparc32_push_dummy_call);
 
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index c4d5ab21b2f..e8ec2ae1817 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -1833,7 +1833,6 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   /* Call dummy code.  */
   set_gdbarch_frame_align (gdbarch, sparc64_frame_align);
-  set_gdbarch_call_dummy_location (gdbarch, AT_ENTRY_POINT);
   set_gdbarch_push_dummy_code (gdbarch, NULL);
   set_gdbarch_push_dummy_call (gdbarch, sparc64_push_dummy_call);
 

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Andrew> LGTM.  I know you're thinking about rewriting this.
Tom> I looked at all the callers of set_gdbarch_push_dummy_code, and I
Tom> discovered that arc and cris set this, but don't use ON_STACK.

Tom> I guess we could delete these if I do the rewrite, to preserve existing
Tom> behavior.

Tom> Here's what the other approach looks like.  This found some latent
Tom> bugs -- basically the current code isn't "DRY"-ready so of course some
Tom> divergences crept in.

Tom> Let me know what you think.

Any comments on this one?  I wonder whether I missed some reason for
these two settings to be separate.

Tom

Re: [PATCH] Use enum for gdbarch_call_dummy_location

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-04-15 10:07:20 -0600]:

> >>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
> 
> Andrew> LGTM.  I know you're thinking about rewriting this.
> Tom> I looked at all the callers of set_gdbarch_push_dummy_code, and I
> Tom> discovered that arc and cris set this, but don't use ON_STACK.
> 
> Tom> I guess we could delete these if I do the rewrite, to preserve existing
> Tom> behavior.
> 
> Tom> Here's what the other approach looks like.  This found some latent
> Tom> bugs -- basically the current code isn't "DRY"-ready so of course some
> Tom> divergences crept in.
> 
> Tom> Let me know what you think.
> 
> Any comments on this one?  I wonder whether I missed some reason for
> these two settings to be separate.

I don't know of any reason why these are separate.  I'm happy with
your patch to merge them.

Thanks,
Andrew