[v2,1/2,gdb/exp] Allow internal function to indicate return type

Message ID 20240716152614.14512-1-tdevries@suse.de
State Committed
Headers
Series [v2,1/2,gdb/exp] Allow internal function to indicate return type |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries July 16, 2024, 3:26 p.m. UTC
  Currently an internal function handler has this prototype:
...
struct value *handler (struct gdbarch *gdbarch,
                       const struct language_defn *language,
                       void *cookie, int argc, struct value **argv);
...

Also allow an internal function with a handler with an additional
"enum noside noside" parameter:
...
struct value *handler (struct gdbarch *gdbarch,
                       const struct language_defn *language, void *cookie,
                       int argc, struct value **argv, enum noside noside);
...

In case such a handler is called with noside == EVAL_AVOID_SIDE_EFFECTS, it's
expected to return some value with the correct return type.

At least, provided it can do so without side effects, otherwise it should
throw an error.

No functional changes.

Tested on x86_64-linux and aarch64-linux.

Reviewed-By: Keith Seitz <keiths@redhat.com>
---
 gdb/ada-lang.c        |  9 +-----
 gdb/eval.c            |  9 ++----
 gdb/python/py-value.c |  3 +-
 gdb/value.c           | 75 ++++++++++++++++++++++++++++++++++++++-----
 gdb/value.h           | 35 +++++++++++++++-----
 5 files changed, 100 insertions(+), 31 deletions(-)


base-commit: b6a5604da00b0438a0fb9b93e8713b14f323b6e4
  

Comments

Tom de Vries July 24, 2024, 2:33 p.m. UTC | #1
On 7/16/24 17:26, Tom de Vries wrote:
> Currently an internal function handler has this prototype:
> ...
> struct value *handler (struct gdbarch *gdbarch,
>                         const struct language_defn *language,
>                         void *cookie, int argc, struct value **argv);
> ...
> 
> Also allow an internal function with a handler with an additional
> "enum noside noside" parameter:
> ...
> struct value *handler (struct gdbarch *gdbarch,
>                         const struct language_defn *language, void *cookie,
>                         int argc, struct value **argv, enum noside noside);
> ...
> 
> In case such a handler is called with noside == EVAL_AVOID_SIDE_EFFECTS, it's
> expected to return some value with the correct return type.
> 
> At least, provided it can do so without side effects, otherwise it should
> throw an error.
> 
> No functional changes.
> 

I've pushed this series.

Thanks,
- Tom

> Tested on x86_64-linux and aarch64-linux.
> 
> Reviewed-By: Keith Seitz <keiths@redhat.com>
> ---
>   gdb/ada-lang.c        |  9 +-----
>   gdb/eval.c            |  9 ++----
>   gdb/python/py-value.c |  3 +-
>   gdb/value.c           | 75 ++++++++++++++++++++++++++++++++++++++-----
>   gdb/value.h           | 35 +++++++++++++++-----
>   5 files changed, 100 insertions(+), 31 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index e2be7f97224..1cf6ff5db8b 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -11195,16 +11195,9 @@ ada_funcall_operation::evaluate (struct type *expect_type,
>   	}
>         return call_function_by_hand (callee, expect_type, argvec);
>       case TYPE_CODE_INTERNAL_FUNCTION:
> -      if (noside == EVAL_AVOID_SIDE_EFFECTS)
> -	/* We don't know anything about what the internal
> -	   function might return, but we have to return
> -	   something.  */
> -	return value::zero (builtin_type (exp->gdbarch)->builtin_int,
> -			   not_lval);
> -      else
>   	return call_internal_function (exp->gdbarch, exp->language_defn,
>   				       callee, nargs,
> -				       argvec.data ());
> +				       argvec.data (), noside);
>   
>       case TYPE_CODE_STRUCT:
>         {
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 40640ae97be..f6b81363125 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -622,11 +622,7 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
>   
>         if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
>   	{
> -	  /* We don't know anything about what the internal
> -	     function might return, but we have to return
> -	     something.  */
> -	  return value::zero (builtin_type (exp->gdbarch)->builtin_int,
> -			     not_lval);
> +	  /* The call to call_internal_function below handles noside.  */
>   	}
>         else if (ftype->code () == TYPE_CODE_XMETHOD)
>   	{
> @@ -666,7 +662,8 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
>       {
>       case TYPE_CODE_INTERNAL_FUNCTION:
>         return call_internal_function (exp->gdbarch, exp->language_defn,
> -				     callee, argvec.size (), argvec.data ());
> +				     callee, argvec.size (), argvec.data (),
> +				     noside);
>       case TYPE_CODE_XMETHOD:
>         return callee->call_xmethod (argvec);
>       default:
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index dada8bfb7e7..5c62ef681d4 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -1258,7 +1258,8 @@ valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
>         if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
>   	return_value = call_internal_function (gdbpy_enter::get_gdbarch (),
>   					       current_language,
> -					       function, args_count, vargs);
> +					       function, args_count, vargs,
> +					       EVAL_NORMAL);
>         else
>   	return_value
>   	  = call_function_by_hand (function, NULL,
> diff --git a/gdb/value.c b/gdb/value.c
> index e71f38b0ce4..9435900ba94 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -61,7 +61,7 @@ struct internal_function
>     char *name;
>   
>     /* The handler.  */
> -  internal_function_fn handler;
> +  internal_function_fn_noside handler;
>   
>     /* User data for the handler.  */
>     void *cookie;
> @@ -2336,9 +2336,9 @@ internalvar_name (const struct internalvar *var)
>   
>   static struct internal_function *
>   create_internal_function (const char *name,
> -			  internal_function_fn handler, void *cookie)
> +			  internal_function_fn_noside handler, void *cookie)
>   {
> -  struct internal_function *ifn = XNEW (struct internal_function);
> +  struct internal_function *ifn = new (struct internal_function);
>   
>     ifn->name = xstrdup (name);
>     ifn->handler = handler;
> @@ -2362,7 +2362,8 @@ value_internal_function_name (struct value *val)
>   struct value *
>   call_internal_function (struct gdbarch *gdbarch,
>   			const struct language_defn *language,
> -			struct value *func, int argc, struct value **argv)
> +			struct value *func, int argc, struct value **argv,
> +			enum noside noside)
>   {
>     struct internal_function *ifn;
>     int result;
> @@ -2371,7 +2372,7 @@ call_internal_function (struct gdbarch *gdbarch,
>     result = get_internalvar_function (VALUE_INTERNALVAR (func), &ifn);
>     gdb_assert (result);
>   
> -  return (*ifn->handler) (gdbarch, language, ifn->cookie, argc, argv);
> +  return ifn->handler (gdbarch, language, ifn->cookie, argc, argv, noside);
>   }
>   
>   /* The 'function' command.  This does nothing -- it is just a
> @@ -2388,7 +2389,7 @@ function_command (const char *command, int from_tty)
>   
>   static struct cmd_list_element *
>   do_add_internal_function (const char *name, const char *doc,
> -			  internal_function_fn handler, void *cookie)
> +			  internal_function_fn_noside handler, void *cookie)
>   {
>     struct internal_function *ifn;
>     struct internalvar *var = lookup_internalvar (name);
> @@ -2403,17 +2404,50 @@ do_add_internal_function (const char *name, const char *doc,
>   
>   void
>   add_internal_function (const char *name, const char *doc,
> -		       internal_function_fn handler, void *cookie)
> +		       internal_function_fn_noside handler, void *cookie)
>   {
>     do_add_internal_function (name, doc, handler, cookie);
>   }
>   
> +/* By default, internal functions are assumed to return int.  Return a value
> +   with that type to reflect this.  If this is not correct for a specific
> +   internal function, it should use an internal_function_fn_noside handler to
> +   bypass this default.  */
> +
> +static struct value *
> +internal_function_default_return_type (struct gdbarch *gdbarch)
> +{
> +  return value::zero (builtin_type (gdbarch)->builtin_int, not_lval);
> +}
> +
> +/* See value.h.  */
> +
> +void
> +add_internal_function (const char *name, const char *doc,
> +		       internal_function_fn handler, void *cookie)
> +{
> +  internal_function_fn_noside fn
> +    = [=] (struct gdbarch *gdbarch,
> +	   const struct language_defn *language,
> +	   void *_cookie,
> +	   int argc,
> +	   struct value **argv,
> +	   enum noside noside)
> +    {
> +      if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +	return internal_function_default_return_type (gdbarch);
> +      return handler (gdbarch, language, _cookie, argc, argv);
> +    };
> +
> +  do_add_internal_function (name, doc, fn, cookie);
> +}
> +
>   /* See value.h.  */
>   
>   void
>   add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
>   		       gdb::unique_xmalloc_ptr<char> &&doc,
> -		       internal_function_fn handler, void *cookie)
> +		       internal_function_fn_noside handler, void *cookie)
>   {
>     struct cmd_list_element *cmd
>       = do_add_internal_function (name.get (), doc.get (), handler, cookie);
> @@ -2426,6 +2460,31 @@ add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
>     cmd->name_allocated = 1;
>   }
>   
> +/* See value.h.  */
> +
> +void
> +add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
> +		       gdb::unique_xmalloc_ptr<char> &&doc,
> +		       internal_function_fn handler, void *cookie)
> +{
> +  internal_function_fn_noside fn
> +    = [=] (struct gdbarch *gdbarch,
> +	   const struct language_defn *language,
> +	   void *_cookie,
> +	   int argc,
> +	   struct value **argv,
> +	   enum noside noside)
> +    {
> +      if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +	return internal_function_default_return_type (gdbarch);
> +      return handler (gdbarch, language, _cookie, argc, argv);
> +    };
> +
> +  add_internal_function (std::forward<gdb::unique_xmalloc_ptr<char>>(name),
> +			 std::forward<gdb::unique_xmalloc_ptr<char>>(doc),
> +			 fn, cookie);
> +}
> +
>   void
>   value::preserve (struct objfile *objfile, htab_t copied_types)
>   {
> diff --git a/gdb/value.h b/gdb/value.h
> index 9d7e88d9433..13cfb007aa2 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -1598,13 +1598,24 @@ extern struct value *find_function_in_inferior (const char *,
>   
>   extern struct value *value_allocate_space_in_inferior (int);
>   
> -/* User function handler.  */
> -
> -typedef struct value *(*internal_function_fn) (struct gdbarch *gdbarch,
> -					       const struct language_defn *language,
> -					       void *cookie,
> -					       int argc,
> -					       struct value **argv);
> +/* User function handler.  The internal_function_fn variant assumes return
> +   type int.  The internal_function_fn_noside returns some value with the
> +   return type when passed noside == EVAL_AVOID_SIDE_EFFECTS.  */
> +
> +using internal_function_fn
> +  = std::function<struct value *(struct gdbarch *gdbarch,
> +				 const struct language_defn *language,
> +				 void *cookie,
> +				 int argc,
> +				 struct value **argv)>;
> +
> +using internal_function_fn_noside
> +  = std::function<struct value *(struct gdbarch *gdbarch,
> +				 const struct language_defn *language,
> +				 void *cookie,
> +				 int argc,
> +				 struct value **argv,
> +				 enum noside noside)>;
>   
>   /* Add a new internal function.  NAME is the name of the function; DOC
>      is a documentation string describing the function.  HANDLER is
> @@ -1615,6 +1626,9 @@ typedef struct value *(*internal_function_fn) (struct gdbarch *gdbarch,
>   extern void add_internal_function (const char *name, const char *doc,
>   				   internal_function_fn handler,
>   				   void *cookie);
> +extern void add_internal_function (const char *name, const char *doc,
> +				   internal_function_fn_noside handler,
> +				   void *cookie);
>   
>   /* This overload takes an allocated documentation string.  */
>   
> @@ -1622,11 +1636,16 @@ extern void add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
>   				   gdb::unique_xmalloc_ptr<char> &&doc,
>   				   internal_function_fn handler,
>   				   void *cookie);
> +extern void add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
> +				   gdb::unique_xmalloc_ptr<char> &&doc,
> +				   internal_function_fn_noside handler,
> +				   void *cookie);
>   
>   struct value *call_internal_function (struct gdbarch *gdbarch,
>   				      const struct language_defn *language,
>   				      struct value *function,
> -				      int argc, struct value **argv);
> +				      int argc, struct value **argv,
> +				      enum noside noside);
>   
>   const char *value_internal_function_name (struct value *);
>   
> 
> base-commit: b6a5604da00b0438a0fb9b93e8713b14f323b6e4
  

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index e2be7f97224..1cf6ff5db8b 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11195,16 +11195,9 @@  ada_funcall_operation::evaluate (struct type *expect_type,
 	}
       return call_function_by_hand (callee, expect_type, argvec);
     case TYPE_CODE_INTERNAL_FUNCTION:
-      if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	/* We don't know anything about what the internal
-	   function might return, but we have to return
-	   something.  */
-	return value::zero (builtin_type (exp->gdbarch)->builtin_int,
-			   not_lval);
-      else
 	return call_internal_function (exp->gdbarch, exp->language_defn,
 				       callee, nargs,
-				       argvec.data ());
+				       argvec.data (), noside);
 
     case TYPE_CODE_STRUCT:
       {
diff --git a/gdb/eval.c b/gdb/eval.c
index 40640ae97be..f6b81363125 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -622,11 +622,7 @@  evaluate_subexp_do_call (expression *exp, enum noside noside,
 
       if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
 	{
-	  /* We don't know anything about what the internal
-	     function might return, but we have to return
-	     something.  */
-	  return value::zero (builtin_type (exp->gdbarch)->builtin_int,
-			     not_lval);
+	  /* The call to call_internal_function below handles noside.  */
 	}
       else if (ftype->code () == TYPE_CODE_XMETHOD)
 	{
@@ -666,7 +662,8 @@  evaluate_subexp_do_call (expression *exp, enum noside noside,
     {
     case TYPE_CODE_INTERNAL_FUNCTION:
       return call_internal_function (exp->gdbarch, exp->language_defn,
-				     callee, argvec.size (), argvec.data ());
+				     callee, argvec.size (), argvec.data (),
+				     noside);
     case TYPE_CODE_XMETHOD:
       return callee->call_xmethod (argvec);
     default:
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index dada8bfb7e7..5c62ef681d4 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1258,7 +1258,8 @@  valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
       if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
 	return_value = call_internal_function (gdbpy_enter::get_gdbarch (),
 					       current_language,
-					       function, args_count, vargs);
+					       function, args_count, vargs,
+					       EVAL_NORMAL);
       else
 	return_value
 	  = call_function_by_hand (function, NULL,
diff --git a/gdb/value.c b/gdb/value.c
index e71f38b0ce4..9435900ba94 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -61,7 +61,7 @@  struct internal_function
   char *name;
 
   /* The handler.  */
-  internal_function_fn handler;
+  internal_function_fn_noside handler;
 
   /* User data for the handler.  */
   void *cookie;
@@ -2336,9 +2336,9 @@  internalvar_name (const struct internalvar *var)
 
 static struct internal_function *
 create_internal_function (const char *name,
-			  internal_function_fn handler, void *cookie)
+			  internal_function_fn_noside handler, void *cookie)
 {
-  struct internal_function *ifn = XNEW (struct internal_function);
+  struct internal_function *ifn = new (struct internal_function);
 
   ifn->name = xstrdup (name);
   ifn->handler = handler;
@@ -2362,7 +2362,8 @@  value_internal_function_name (struct value *val)
 struct value *
 call_internal_function (struct gdbarch *gdbarch,
 			const struct language_defn *language,
-			struct value *func, int argc, struct value **argv)
+			struct value *func, int argc, struct value **argv,
+			enum noside noside)
 {
   struct internal_function *ifn;
   int result;
@@ -2371,7 +2372,7 @@  call_internal_function (struct gdbarch *gdbarch,
   result = get_internalvar_function (VALUE_INTERNALVAR (func), &ifn);
   gdb_assert (result);
 
-  return (*ifn->handler) (gdbarch, language, ifn->cookie, argc, argv);
+  return ifn->handler (gdbarch, language, ifn->cookie, argc, argv, noside);
 }
 
 /* The 'function' command.  This does nothing -- it is just a
@@ -2388,7 +2389,7 @@  function_command (const char *command, int from_tty)
 
 static struct cmd_list_element *
 do_add_internal_function (const char *name, const char *doc,
-			  internal_function_fn handler, void *cookie)
+			  internal_function_fn_noside handler, void *cookie)
 {
   struct internal_function *ifn;
   struct internalvar *var = lookup_internalvar (name);
@@ -2403,17 +2404,50 @@  do_add_internal_function (const char *name, const char *doc,
 
 void
 add_internal_function (const char *name, const char *doc,
-		       internal_function_fn handler, void *cookie)
+		       internal_function_fn_noside handler, void *cookie)
 {
   do_add_internal_function (name, doc, handler, cookie);
 }
 
+/* By default, internal functions are assumed to return int.  Return a value
+   with that type to reflect this.  If this is not correct for a specific
+   internal function, it should use an internal_function_fn_noside handler to
+   bypass this default.  */
+
+static struct value *
+internal_function_default_return_type (struct gdbarch *gdbarch)
+{
+  return value::zero (builtin_type (gdbarch)->builtin_int, not_lval);
+}
+
+/* See value.h.  */
+
+void
+add_internal_function (const char *name, const char *doc,
+		       internal_function_fn handler, void *cookie)
+{
+  internal_function_fn_noside fn
+    = [=] (struct gdbarch *gdbarch,
+	   const struct language_defn *language,
+	   void *_cookie,
+	   int argc,
+	   struct value **argv,
+	   enum noside noside)
+    {
+      if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	return internal_function_default_return_type (gdbarch);
+      return handler (gdbarch, language, _cookie, argc, argv);
+    };
+
+  do_add_internal_function (name, doc, fn, cookie);
+}
+
 /* See value.h.  */
 
 void
 add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
 		       gdb::unique_xmalloc_ptr<char> &&doc,
-		       internal_function_fn handler, void *cookie)
+		       internal_function_fn_noside handler, void *cookie)
 {
   struct cmd_list_element *cmd
     = do_add_internal_function (name.get (), doc.get (), handler, cookie);
@@ -2426,6 +2460,31 @@  add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
   cmd->name_allocated = 1;
 }
 
+/* See value.h.  */
+
+void
+add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
+		       gdb::unique_xmalloc_ptr<char> &&doc,
+		       internal_function_fn handler, void *cookie)
+{
+  internal_function_fn_noside fn
+    = [=] (struct gdbarch *gdbarch,
+	   const struct language_defn *language,
+	   void *_cookie,
+	   int argc,
+	   struct value **argv,
+	   enum noside noside)
+    {
+      if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	return internal_function_default_return_type (gdbarch);
+      return handler (gdbarch, language, _cookie, argc, argv);
+    };
+
+  add_internal_function (std::forward<gdb::unique_xmalloc_ptr<char>>(name),
+			 std::forward<gdb::unique_xmalloc_ptr<char>>(doc),
+			 fn, cookie);
+}
+
 void
 value::preserve (struct objfile *objfile, htab_t copied_types)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 9d7e88d9433..13cfb007aa2 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1598,13 +1598,24 @@  extern struct value *find_function_in_inferior (const char *,
 
 extern struct value *value_allocate_space_in_inferior (int);
 
-/* User function handler.  */
-
-typedef struct value *(*internal_function_fn) (struct gdbarch *gdbarch,
-					       const struct language_defn *language,
-					       void *cookie,
-					       int argc,
-					       struct value **argv);
+/* User function handler.  The internal_function_fn variant assumes return
+   type int.  The internal_function_fn_noside returns some value with the
+   return type when passed noside == EVAL_AVOID_SIDE_EFFECTS.  */
+
+using internal_function_fn
+  = std::function<struct value *(struct gdbarch *gdbarch,
+				 const struct language_defn *language,
+				 void *cookie,
+				 int argc,
+				 struct value **argv)>;
+
+using internal_function_fn_noside
+  = std::function<struct value *(struct gdbarch *gdbarch,
+				 const struct language_defn *language,
+				 void *cookie,
+				 int argc,
+				 struct value **argv,
+				 enum noside noside)>;
 
 /* Add a new internal function.  NAME is the name of the function; DOC
    is a documentation string describing the function.  HANDLER is
@@ -1615,6 +1626,9 @@  typedef struct value *(*internal_function_fn) (struct gdbarch *gdbarch,
 extern void add_internal_function (const char *name, const char *doc,
 				   internal_function_fn handler,
 				   void *cookie);
+extern void add_internal_function (const char *name, const char *doc,
+				   internal_function_fn_noside handler,
+				   void *cookie);
 
 /* This overload takes an allocated documentation string.  */
 
@@ -1622,11 +1636,16 @@  extern void add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
 				   gdb::unique_xmalloc_ptr<char> &&doc,
 				   internal_function_fn handler,
 				   void *cookie);
+extern void add_internal_function (gdb::unique_xmalloc_ptr<char> &&name,
+				   gdb::unique_xmalloc_ptr<char> &&doc,
+				   internal_function_fn_noside handler,
+				   void *cookie);
 
 struct value *call_internal_function (struct gdbarch *gdbarch,
 				      const struct language_defn *language,
 				      struct value *function,
-				      int argc, struct value **argv);
+				      int argc, struct value **argv,
+				      enum noside noside);
 
 const char *value_internal_function_name (struct value *);