[PATCH 1/3] [gdb/exp] Allow internal function to indicate return type

[PATCH 1/3] [gdb/exp] Allow internal function to indicate return type

[Tom de Vries]

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.
---
 gdb/ada-lang.c |  9 +-----
 gdb/eval.c     |  9 ++----
 gdb/value.c    | 75 ++++++++++++++++++++++++++++++++++++++++++++------
 gdb/value.h    | 35 +++++++++++++++++------
 4 files changed, 98 insertions(+), 30 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0b430428fb4..2da81290c91 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11200,16 +11200,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 6b752e70635..9c76ba61236 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -598,11 +598,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)
 	{
@@ -642,7 +638,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/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: 2c37ce86c1515243cb0ab135feb5f1bb11b768ce
-- 
2.35.3

[PATCH 2/3] [gdb/exp] Fix ptype $_creal/$_cimag

[Tom de Vries]

Consider test.c, compiled with -g:
...
__complex__ float cf = 1 + 2i;
int main (void) { return 0; }
...

The values of cf and its components are:
...
$ gdb -q a.out
Reading symbols from a.out...
(gdb) p cf
$1 = 1 + 2i
(gdb) p $_creal(cf)
$2 = 1
(gdb) p $_cimag(cf)
$3 = 2
...
and their respective types are:
...
(gdb) ptype $1
type = complex float
(gdb) ptype $2
type = float
(gdb) ptype $3
type = float
...

Now let's try that again, using ptype directly:
...
(gdb) ptype cf
type = complex float
(gdb) ptype $_creal(cf)
type = int
(gdb) ptype $_cimag(cf)
type = int
...

The last two types should have been float, not int.

Fix this by extending the internal function handlers creal_internal_fn and
cimag_internal_fn with the noside parameter, such that we get instead:
...
(gdb) ptype $_creal(cf)
type = float
(gdb) ptype $_cimag(cf)
type = float
...

Tested on x86_64-linux.

PR exp/31816
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31816
---
 gdb/testsuite/gdb.base/complex-parts.exp | 24 ++++++++++++++++++------
 gdb/value.c                              |  9 +++++++--
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
index e678cfcb899..fc758493dc8 100644
--- a/gdb/testsuite/gdb.base/complex-parts.exp
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -40,33 +40,45 @@ if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z3" " = complex long double"
 
 with_test_prefix "double imaginary" {
-    gdb_test "p \$_cimag (z1)" " = 4.5"
+    set expr {$_cimag (z1)}
+    gdb_test "p $expr" " = 4.5"
     gdb_test "ptype \$" " = double"
+    gdb_test "ptype $expr" " = double"
 }
 
 with_test_prefix "float imaginary" {
-    gdb_test "p \$_cimag (z2)" " = -5.5"
+    set expr {$_cimag (z2)}
+    gdb_test "p $expr" " = -5.5"
     gdb_test "ptype \$" " = float"
+    gdb_test "ptype $expr" " = float"
 }
 
 with_test_prefix "long double imaginary" {
-    gdb_test "p \$_cimag (z3)" " = 6.5"
+    set expr {$_cimag (z3)}
+    gdb_test "p $expr" " = 6.5"
     gdb_test "ptype \$" " = long double"
+    gdb_test "ptype $expr" " = long double"
 }
 
 with_test_prefix "double real" {
-    gdb_test "p \$_creal (z1)" " = 1.5"
+    set expr {$_creal (z1)}
+    gdb_test "p $expr" " = 1.5"
     gdb_test "ptype \$" " = double"
+    gdb_test "ptype $expr" " = double"
 }
 
 with_test_prefix "float real" {
-    gdb_test "p \$_creal (z2)" " = 2.5"
+    set expr {$_creal (z2)}
+    gdb_test "p $expr" " = 2.5"
     gdb_test "ptype \$" " = float"
+    gdb_test "ptype $expr" " = float"
 }
 
 with_test_prefix "long double real" {
-    gdb_test "p \$_creal (z3)" " = 3.5"
+    set expr {$_creal (z3)}
+    gdb_test "p $expr" " = 3.5"
     gdb_test "ptype \$" " = long double"
+    gdb_test "ptype $expr" " = long double"
 }
 
 gdb_test "p \$_cimag (d1)" "expected a complex number"
diff --git a/gdb/value.c b/gdb/value.c
index 9435900ba94..09fb19b9bf8 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -4295,7 +4295,8 @@ isvoid_internal_fn (struct gdbarch *gdbarch,
 static struct value *
 creal_internal_fn (struct gdbarch *gdbarch,
 		   const struct language_defn *language,
-		   void *cookie, int argc, struct value **argv)
+		   void *cookie, int argc, struct value **argv,
+		   enum noside noside)
 {
   if (argc != 1)
     error (_("You must provide one argument for $_creal."));
@@ -4304,6 +4305,8 @@ creal_internal_fn (struct gdbarch *gdbarch,
   type *ctype = check_typedef (cval->type ());
   if (ctype->code () != TYPE_CODE_COMPLEX)
     error (_("expected a complex number"));
+  if (noside == EVAL_AVOID_SIDE_EFFECTS)
+    return value::zero (ctype->target_type (), not_lval);
   return value_real_part (cval);
 }
 
@@ -4314,7 +4317,7 @@ static struct value *
 cimag_internal_fn (struct gdbarch *gdbarch,
 		   const struct language_defn *language,
 		   void *cookie, int argc,
-		   struct value **argv)
+		   struct value **argv, enum noside noside)
 {
   if (argc != 1)
     error (_("You must provide one argument for $_cimag."));
@@ -4323,6 +4326,8 @@ cimag_internal_fn (struct gdbarch *gdbarch,
   type *ctype = check_typedef (cval->type ());
   if (ctype->code () != TYPE_CODE_COMPLEX)
     error (_("expected a complex number"));
+  if (noside == EVAL_AVOID_SIDE_EFFECTS)
+    return value::zero (ctype->target_type (), not_lval);
   return value_imaginary_part (cval);
 }
 
-- 
2.35.3

[PATCH 3/3] [gdb/testsuite] Simplify gdb.base/complex-parts.exp

[Tom de Vries]

I noticed a lot of escaping in test-case gdb.base/complex-parts.exp.

Make the test-case more readable by using string_to_regexp, and {} instead
of "".

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.base/complex-parts.exp | 67 ++++++++++++------------
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
index fc758493dc8..24b16a6a5e4 100644
--- a/gdb/testsuite/gdb.base/complex-parts.exp
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -23,12 +23,13 @@ if {![runto_main]} {
     return 0
 }
 
-gdb_breakpoint [gdb_get_line_number "Break Here"]
-gdb_continue_to_breakpoint "breakpt" ".* Break Here\\. .*"
+set marker " Break Here. "
+gdb_breakpoint [gdb_get_line_number $marker]
+gdb_continue_to_breakpoint "breakpt" ".*[string_to_regexp $marker].*"
 
-gdb_test "p z1" " = 1.5 \\+ 4.5i"
-gdb_test "p z2" " = 2.5 \\+ -5.5i"
-gdb_test "p z3" " = 3.5 \\+ 6.5i"
+gdb_test "p z1" [string_to_regexp " = 1.5 + 4.5i"]
+gdb_test "p z2" [string_to_regexp " = 2.5 + -5.5i"]
+gdb_test "p z3" [string_to_regexp " = 3.5 + 6.5i"]
 
 # The following 3 tests are broken for Clang.
 # More info at https://github.com/llvm/llvm-project/issues/52996.
@@ -42,69 +43,69 @@ gdb_test "ptype z3" " = complex long double"
 with_test_prefix "double imaginary" {
     set expr {$_cimag (z1)}
     gdb_test "p $expr" " = 4.5"
-    gdb_test "ptype \$" " = double"
+    gdb_test {ptype $} " = double"
     gdb_test "ptype $expr" " = double"
 }
 
 with_test_prefix "float imaginary" {
     set expr {$_cimag (z2)}
     gdb_test "p $expr" " = -5.5"
-    gdb_test "ptype \$" " = float"
+    gdb_test {ptype $} " = float"
     gdb_test "ptype $expr" " = float"
 }
 
 with_test_prefix "long double imaginary" {
     set expr {$_cimag (z3)}
     gdb_test "p $expr" " = 6.5"
-    gdb_test "ptype \$" " = long double"
+    gdb_test {ptype $} " = long double"
     gdb_test "ptype $expr" " = long double"
 }
 
 with_test_prefix "double real" {
     set expr {$_creal (z1)}
     gdb_test "p $expr" " = 1.5"
-    gdb_test "ptype \$" " = double"
+    gdb_test {ptype $} " = double"
     gdb_test "ptype $expr" " = double"
 }
 
 with_test_prefix "float real" {
     set expr {$_creal (z2)}
     gdb_test "p $expr" " = 2.5"
-    gdb_test "ptype \$" " = float"
+    gdb_test {ptype $} " = float"
     gdb_test "ptype $expr" " = float"
 }
 
 with_test_prefix "long double real" {
     set expr {$_creal (z3)}
     gdb_test "p $expr" " = 3.5"
-    gdb_test "ptype \$" " = long double"
+    gdb_test {ptype $} " = long double"
     gdb_test "ptype $expr" " = long double"
 }
 
-gdb_test "p \$_cimag (d1)" "expected a complex number"
-gdb_test "p \$_cimag (f1)" "expected a complex number"
-gdb_test "p \$_cimag (i1)" "expected a complex number"
+gdb_test {p $_cimag (d1)} "expected a complex number"
+gdb_test {p $_cimag (f1)} "expected a complex number"
+gdb_test {p $_cimag (i1)} "expected a complex number"
 
-gdb_test "p \$_creal (d1)" "expected a complex number"
-gdb_test "p \$_creal (f1)" "expected a complex number"
-gdb_test "p \$_creal (i1)" "expected a complex number"
+gdb_test {p $_creal (d1)} "expected a complex number"
+gdb_test {p $_creal (f1)} "expected a complex number"
+gdb_test {p $_creal (i1)} "expected a complex number"
 
 #
 # General complex number tests.
 #
 
-gdb_test "print 23 + 7i" " = 23 \\+ 7i"
-gdb_test "print 23.125f + 7i" " = 23.125 \\+ 7i"
-gdb_test "print 23 + 7.25fi" " = 23 \\+ 7.25i"
-gdb_test "print (23 + 7i) + (17 + 10i)" " = 40 \\+ 17i"
-gdb_test "print 23 + -7i" " = 23 \\+ -7i"
-gdb_test "print 23 - 7i" " = 23 \\+ -7i"
+gdb_test "print 23 + 7i" [string_to_regexp " = 23 + 7i"]
+gdb_test "print 23.125f + 7i" [string_to_regexp " = 23.125 + 7i"]
+gdb_test "print 23 + 7.25fi" [string_to_regexp " = 23 + 7.25i"]
+gdb_test "print (23 + 7i) + (17 + 10i)" [string_to_regexp " = 40 + 17i"]
+gdb_test "print 23 + -7i" [string_to_regexp " = 23 + -7i"]
+gdb_test "print 23 - 7i" [string_to_regexp " = 23 + -7i"]
 
-gdb_test "print -(23 + 7i)" " = -23 \\+ -7i"
-gdb_test "print +(23 + 7i)" " = 23 \\+ 7i"
-gdb_test "print ~(23 + 7i)" " = 23 \\+ -7i"
+gdb_test "print -(23 + 7i)" [string_to_regexp " = -23 + -7i"]
+gdb_test "print +(23 + 7i)" [string_to_regexp " = 23 + 7i"]
+gdb_test "print ~(23 + 7i)" [string_to_regexp " = 23 + -7i"]
 
-gdb_test "print (5 + 5i) * (2 + 2i)" " = 0 \\+ 20i"
+gdb_test "print (5 + 5i) * (2 + 2i)" [string_to_regexp " = 0 + 20i"]
 
 gdb_test "print (5 + 7i) == (5 + 7i)" " = 1"
 gdb_test "print (5 + 7i) == (8 + 7i)" " = 0"
@@ -113,14 +114,14 @@ gdb_test "print (5 + 7i) != (5 + 7i)" " = 0"
 gdb_test "print (5 + 7i) != (8 + 7i)" " = 1"
 gdb_test "print (5 + 7i) != (5 + 92i)" " = 1"
 
-gdb_test "print (20 - 4i) / (3 + 2i)" " = 4 \\+ -4i"
+gdb_test "print (20 - 4i) / (3 + 2i)" [string_to_regexp " = 4 + -4i"]
 
-gdb_test "print (_Complex int) 4" " = 4 \\+ 0i"
-gdb_test "print (_Complex float) 4.5" " = 4.5 \\+ 0i"
+gdb_test "print (_Complex int) 4" [string_to_regexp " = 4 + 0i"]
+gdb_test "print (_Complex float) 4.5" [string_to_regexp " = 4.5 + 0i"]
 gdb_test "ptype __complex__ short" " = _Complex short"
-gdb_test "print (_Complex int) (23.75 + 8.88i)" " = 23 \\+ 8i"
+gdb_test "print (_Complex int) (23.75 + 8.88i)" [string_to_regexp " = 23 + 8i"]
 
-set re_reject_arg "Argument to complex arithmetic operation not supported\\."
+set re_reject_arg [string_to_regexp "Argument to complex arithmetic operation not supported."]
 gdb_test "print (void *)0 + 5i" $re_reject_arg
 gdb_test "print (_Decimal32)0 + 5i" $re_reject_arg
 
@@ -129,4 +130,4 @@ clean_restart
 gdb_test_no_output "set language c++"
 
 # C++ type tests.
-gdb_test "print (bool)1 + 1i" " = 1 \\+ 1i"
+gdb_test "print (bool)1 + 1i" [string_to_regexp "= 1 + 1i"]
-- 
2.35.3

Re: [PATCH 1/3] [gdb/exp] Allow internal function to indicate return type

[Keith Seitz]

On 5/30/24 3:35 AM, 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.

This is a nice addition which is long overdue. Thank you for tackling 
this. I just have a trivial nit or two. This series LGTM otherwise.

> No functional changes.
> 
> Tested on x86_64-linux.

Verified -- after fixing a python/py-value.c (see below) build error.

Feel free to add my Reviewed-by.

Keith

> diff --git a/gdb/eval.c b/gdb/eval.c
> index 6b752e70635..9c76ba61236 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> diff --git a/gdb/value.c b/gdb/value.c
> index e71f38b0ce4..9435900ba94 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
[snip]
> @@ -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);
> +}

Note the missing ' ' crept in here. [Git prehooks, where art thou?]

> +
>   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);

A recent commit to python/py-value.c requires a small update.

Re: [PATCH 2/3] [gdb/exp] Fix ptype $_creal/$_cimag

[Keith Seitz]

Hi,

This is a nice little cleanup. Thank you.

I just have a question about tail parentheses,
which predate your patch. [But since you're touching
some of those lines...]

On 5/30/24 3:35 AM, Tom de Vries wrote:
> diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
> index e678cfcb899..fc758493dc8 100644
> --- a/gdb/testsuite/gdb.base/complex-parts.exp
> +++ b/gdb/testsuite/gdb.base/complex-parts.exp
> @@ -40,33 +40,45 @@ if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
>   gdb_test "ptype z3" " = complex long double"
>   
>   with_test_prefix "double imaginary" {
> -    gdb_test "p \$_cimag (z1)" " = 4.5"
> +    set expr {$_cimag (z1)}
> +    gdb_test "p $expr" " = 4.5"
>       gdb_test "ptype \$" " = double"
> +    gdb_test "ptype $expr" " = double"
>   }

According to our wiki:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

the use of tail parentheses on test messages is discouraged. Some tests
here (and in the subsequent patch) appear to employ this idiom. For 
example, "p $_cimag (z1)".

Am I mistaken, or is this still a thin?g Does the wiki page (or
my memory :-) need updating?

Again, apologies for raising this -- you did not introduce this.

Keith
>   
>   with_test_prefix "float imaginary" {
> -    gdb_test "p \$_cimag (z2)" " = -5.5"
> +    set expr {$_cimag (z2)}
> +    gdb_test "p $expr" " = -5.5"
>       gdb_test "ptype \$" " = float"
> +    gdb_test "ptype $expr" " = float"
>   }
>   
>   with_test_prefix "long double imaginary" {
> -    gdb_test "p \$_cimag (z3)" " = 6.5"
> +    set expr {$_cimag (z3)}
> +    gdb_test "p $expr" " = 6.5"
>       gdb_test "ptype \$" " = long double"
> +    gdb_test "ptype $expr" " = long double"
>   }
>   
>   with_test_prefix "double real" {
> -    gdb_test "p \$_creal (z1)" " = 1.5"
> +    set expr {$_creal (z1)}
> +    gdb_test "p $expr" " = 1.5"
>       gdb_test "ptype \$" " = double"
> +    gdb_test "ptype $expr" " = double"
>   }
>   
>   with_test_prefix "float real" {
> -    gdb_test "p \$_creal (z2)" " = 2.5"
> +    set expr {$_creal (z2)}
> +    gdb_test "p $expr" " = 2.5"
>       gdb_test "ptype \$" " = float"
> +    gdb_test "ptype $expr" " = float"
>   }
>   
>   with_test_prefix "long double real" {
> -    gdb_test "p \$_creal (z3)" " = 3.5"
> +    set expr {$_creal (z3)}
> +    gdb_test "p $expr" " = 3.5"
>       gdb_test "ptype \$" " = long double"
> +    gdb_test "ptype $expr" " = long double"
>   }
>   
>   gdb_test "p \$_cimag (d1)" "expected a complex number"
> diff --git a/gdb/value.c b/gdb/value.c
> index 9435900ba94..09fb19b9bf8 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -4295,7 +4295,8 @@ isvoid_internal_fn (struct gdbarch *gdbarch,
>   static struct value *
>   creal_internal_fn (struct gdbarch *gdbarch,
>   		   const struct language_defn *language,
> -		   void *cookie, int argc, struct value **argv)
> +		   void *cookie, int argc, struct value **argv,
> +		   enum noside noside)
>   {
>     if (argc != 1)
>       error (_("You must provide one argument for $_creal."));
> @@ -4304,6 +4305,8 @@ creal_internal_fn (struct gdbarch *gdbarch,
>     type *ctype = check_typedef (cval->type ());
>     if (ctype->code () != TYPE_CODE_COMPLEX)
>       error (_("expected a complex number"));
> +  if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +    return value::zero (ctype->target_type (), not_lval);
>     return value_real_part (cval);
>   }
>   
> @@ -4314,7 +4317,7 @@ static struct value *
>   cimag_internal_fn (struct gdbarch *gdbarch,
>   		   const struct language_defn *language,
>   		   void *cookie, int argc,
> -		   struct value **argv)
> +		   struct value **argv, enum noside noside)
>   {
>     if (argc != 1)
>       error (_("You must provide one argument for $_cimag."));
> @@ -4323,6 +4326,8 @@ cimag_internal_fn (struct gdbarch *gdbarch,
>     type *ctype = check_typedef (cval->type ());
>     if (ctype->code () != TYPE_CODE_COMPLEX)
>       error (_("expected a complex number"));
> +  if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +    return value::zero (ctype->target_type (), not_lval);
>     return value_imaginary_part (cval);
>   }
>