[PATCH 0/6] Don't throw quit while handling inferior events

[PATCH 0/6] Don't throw quit while handling inferior events

[Pedro Alves]

This series implements what I suggested here:

  https://inbox.sourceware.org/gdb-patches/ab97c553-f406-b094-cdf3-ba031fdea925@palves.net/

... and then some more.

To expose the problem with a testcase, I added a new $_shell
convenience function, and I'm using that to send a SIGINT signal to
GDB from a breakpoint condition.

While writing tests for the new $_shell convenience function, I ran
into a number of bugs, also fixed in the series.

Pedro Alves (6):
  Fix "ptype INTERNAL_FUNC" (PR gdb/30105)
  Make "ptype INTERNAL_FUNCTION" in Ada print like other languages
  Add new "$_shell(CMD)" internal function
  Don't throw quit while handling inferior events
  GC get_active_ext_lang
  Don't throw quit while handling inferior events, part II

 gdb/NEWS                                      | 10 ++
 gdb/ada-typeprint.c                           |  7 ++
 gdb/c-typeprint.c                             | 51 ----------
 gdb/cli/cli-cmds.c                            | 89 ++++++++++++++++-
 gdb/doc/gdb.texinfo                           | 47 +++++++++
 gdb/extension-priv.h                          |  2 -
 gdb/extension.c                               | 70 +++++++++++--
 gdb/extension.h                               | 16 +++
 gdb/infrun.c                                  | 54 ++++++++++
 gdb/p-typeprint.c                             | 46 ---------
 .../gdb.base/bg-exec-sigint-bp-cond.c         | 35 +++++++
 .../gdb.base/bg-exec-sigint-bp-cond.exp       | 98 +++++++++++++++++++
 gdb/testsuite/gdb.base/default.exp            |  1 +
 .../gdb.base/internal-functions-ptype.exp     | 40 ++++++++
 gdb/testsuite/gdb.base/shell.exp              | 36 +++++++
 gdb/testsuite/gdb.python/py-xmethods.exp      |  8 ++
 16 files changed, 498 insertions(+), 112 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
 create mode 100644 gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
 create mode 100644 gdb/testsuite/gdb.base/internal-functions-ptype.exp


base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
-- 
2.36.0

[PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105)

[Pedro Alves]

Currently, looking at the type of an internal function, like below,
hits an odd error:

 (gdb) ptype $_isvoid
 type = <internal function>type not handled in c_type_print_varspec_prefix()

That is an error thrown from
c-typeprint.c:c_type_print_varspec_prefix, where it reads:

    ...
    case TYPE_CODE_DECFLOAT:
    case TYPE_CODE_FIXED_POINT:
      /* These types need no prefix.  They are listed here so that
	 gcc -Wall will reveal any types that haven't been handled.  */
      break;
    default:
      error (_("type not handled in c_type_print_varspec_prefix()"));
      break;

Internal function types have type code TYPE_CODE_INTERNAL_FUNCTION,
which is not explicitly handled by that switch.

That comment quoted above says that gcc -Wall will reveal any types
that haven't been handled, but that's not actually true, at least with
modern GCCs.  You would need to enable -Wswitch-enum for that, which
we don't.  If I do enable that warning, then I see that we're missing
handling for the following type codes:

   TYPE_CODE_INTERNAL_FUNCTION,
   TYPE_CODE_MODULE,
   TYPE_CODE_NAMELIST,
   TYPE_CODE_XMETHOD

TYPE_CODE_MODULE and TYPE_CODE_NAMELIST and Fortran-specific, so it'd
be a little weird to handle them here.

I tried to reach this code with TYPE_CODE_XMETHOD, but couldn't figure
out how to.  ptype on an xmethod isn't treated specially, it just
complains that the method doesn't exist.  I've extended the
gdb.python/py-xmethods.exp testcase to make sure of that.

My thinking is that whatever type code we add next, the most likely
scenario is that it won't need any special handling, so we'd just be
adding another case to that "do nothing" list.  If we do need special
casing for whatever type code, I think that tests added at the same
time as the feature would uncover it anyhow.  If we do miss adding the
special casing, then it still looks better to me to print the type
somewhat incompletely than to error out and make it harder for users
to debug whatever they need.  So I think that the best thing to do
here is to just remove all those explicit "do nothing" cases, along
with the error default case.

After doing that, I decided to write a testcase that iterates over all
supported languages doing "ptype INTERNAL_FUNC".  That revealed that
Pascal has a similar problem, except the default case hits a
gdb_assert instead of an error:

 (gdb) with language pascal -- ptype $_isvoid
 type =
 ../../src/gdb/p-typeprint.c:268: internal-error: type_print_varspec_prefix: unexpected type
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.

That is fixed by this patch in the same way.

You'll notice that the new testcase special-cases the Ada expected
output:

	} elseif {$lang == "ada"} {
	    gdb_test "ptype \$_isvoid" "<<internal function>>"
	} else {
	    gdb_test "ptype \$_isvoid" "<internal function>"
	}

That will be subject of the following patch.

Change-Id: I81aec03523cceb338b5180a0b4c2e4ad26b4c4db
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30105
---
 gdb/c-typeprint.c                             | 51 -------------------
 gdb/p-typeprint.c                             | 46 -----------------
 .../gdb.base/internal-functions-ptype.exp     | 42 +++++++++++++++
 gdb/testsuite/gdb.python/py-xmethods.exp      |  8 +++
 4 files changed, 50 insertions(+), 97 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/internal-functions-ptype.exp

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index dca96231117..7e9d941a435 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -441,31 +441,6 @@ c_type_print_varspec_prefix (struct type *type,
 				   stream, show, passed_a_ptr, 0,
 				   language, flags, podata);
       break;
-
-    case TYPE_CODE_UNDEF:
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_FLAGS:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_VOID:
-    case TYPE_CODE_ERROR:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_STRING:
-    case TYPE_CODE_COMPLEX:
-    case TYPE_CODE_NAMESPACE:
-    case TYPE_CODE_DECFLOAT:
-    case TYPE_CODE_FIXED_POINT:
-      /* These types need no prefix.  They are listed here so that
-	 gcc -Wall will reveal any types that haven't been handled.  */
-      break;
-    default:
-      error (_("type not handled in c_type_print_varspec_prefix()"));
-      break;
     }
 }
 
@@ -821,32 +796,6 @@ c_type_print_varspec_suffix (struct type *type,
       c_type_print_varspec_suffix (type->target_type (), stream,
 				   show, passed_a_ptr, 0, language, flags);
       break;
-
-    case TYPE_CODE_UNDEF:
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_FLAGS:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_VOID:
-    case TYPE_CODE_ERROR:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_STRING:
-    case TYPE_CODE_COMPLEX:
-    case TYPE_CODE_NAMESPACE:
-    case TYPE_CODE_DECFLOAT:
-    case TYPE_CODE_FIXED_POINT:
-      /* These types do not need a suffix.  They are listed so that
-	 gcc -Wall will report types that may not have been
-	 considered.  */
-      break;
-    default:
-      error (_("type not handled in c_type_print_varspec_suffix()"));
-      break;
     }
 }
 
diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
index e8542d6845a..7458aa6c095 100644
--- a/gdb/p-typeprint.c
+++ b/gdb/p-typeprint.c
@@ -244,29 +244,6 @@ pascal_language::type_print_varspec_prefix (struct type *type,
 		    plongest (type->bounds ()->high.const_val ()));
       gdb_printf (stream, "of ");
       break;
-
-    case TYPE_CODE_UNDEF:
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_VOID:
-    case TYPE_CODE_ERROR:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_STRING:
-    case TYPE_CODE_COMPLEX:
-    case TYPE_CODE_TYPEDEF:
-    case TYPE_CODE_FIXED_POINT:
-      /* These types need no prefix.  They are listed here so that
-	 gcc -Wall will reveal any types that haven't been handled.  */
-      break;
-    default:
-      gdb_assert_not_reached ("unexpected type");
-      break;
     }
 }
 
@@ -377,29 +354,6 @@ pascal_language::type_print_varspec_suffix (struct type *type,
       type_print_func_varspec_suffix (type, stream, show,
 					     passed_a_ptr, 0, flags);
       break;
-
-    case TYPE_CODE_UNDEF:
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_VOID:
-    case TYPE_CODE_ERROR:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_STRING:
-    case TYPE_CODE_COMPLEX:
-    case TYPE_CODE_TYPEDEF:
-    case TYPE_CODE_FIXED_POINT:
-      /* These types do not need a suffix.  They are listed so that
-	 gcc -Wall will report types that may not have been considered.  */
-      break;
-    default:
-      gdb_assert_not_reached ("unexpected type");
-      break;
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/internal-functions-ptype.exp b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
new file mode 100644
index 00000000000..42caae05aad
--- /dev/null
+++ b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
@@ -0,0 +1,42 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test "ptype INTERNAL_FUNCTION" in all languages.
+
+proc test_ptype_internal_function {} {
+    set all_languages [get_set_option_choices "set language"]
+
+    foreach_with_prefix lang $all_languages {
+	if { $lang == "auto" || $lang == "local" } {
+	    # Avoid duplicate testing.
+	    continue
+	}
+
+	gdb_test_no_output "set language $lang"
+
+	if {$lang == "unknown"} {
+	    gdb_test "ptype \$_isvoid" \
+		"expression parsing not implemented for language \"Unknown\""
+	} elseif {$lang == "ada"} {
+	    gdb_test "ptype \$_isvoid" "<<internal function>>"
+	} else {
+	    gdb_test "ptype \$_isvoid" "<internal function>"
+	}
+    }
+}
+
+clean_restart
+
+test_ptype_internal_function
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index 97d560476fc..2cf7bbb68b0 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -52,6 +52,9 @@ gdb_test "p a_geta" ".* = 1" "before: a_geta 1"
 gdb_test "p ++a1" "No symbol.*" "before: ++a1"
 gdb_test "p a1.getarrayind(5)" "Couldn't find method.*" \
   "before: a1.getarrayind(5)"
+gdb_test "ptype a1.getarrayind" \
+    "There is no member or method named getarrayind\\." \
+    "before: ptype a1.getarrayind"
 
 gdb_test "p a_ptr->geta()" ".* = 60" "before: a_ptr->geta()"
 gdb_test "p b_geta" ".* = 1" "before: b_geta 1"
@@ -94,9 +97,14 @@ gdb_test "p b1 - a1" ".* = 25" "after: b1 - a1"
 gdb_test "p a_minus_a" ".* = 4" "after: a_minus_a 4"
 
 gdb_test "p a1.geta()" "From Python <A_geta>.*5" "after: a1.geta()"
+
 gdb_test "p ++a1" "From Python <plus_plus_A>.*6" "after: ++a1"
 gdb_test "p a1.getarrayind(5)" "From Python <A_getarrayind>.*5" \
   "after: a1.getarrayind(5)"
+gdb_test "ptype a1.getarrayind" \
+  "There is no member or method named getarrayind\\." \
+  "after: ptype a1.getarrayind"
+
 gdb_test "p a1\[6\]" ".*int &.*6" "after a1\[\]"
 gdb_test "p b1\[7\]" ".*const int &.*7" "after b1\[\]"
 # Note the following test.  Xmethods on dynamc types are not looked up
-- 
2.36.0

Re: [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105)

[Andrew Burgess]

Pedro Alves <pedro@palves.net> writes:

> Currently, looking at the type of an internal function, like below,
> hits an odd error:
>
>  (gdb) ptype $_isvoid
>  type = <internal function>type not handled in c_type_print_varspec_prefix()
>
> That is an error thrown from
> c-typeprint.c:c_type_print_varspec_prefix, where it reads:
>
>     ...
>     case TYPE_CODE_DECFLOAT:
>     case TYPE_CODE_FIXED_POINT:
>       /* These types need no prefix.  They are listed here so that
> 	 gcc -Wall will reveal any types that haven't been handled.  */
>       break;
>     default:
>       error (_("type not handled in c_type_print_varspec_prefix()"));
>       break;
>
> Internal function types have type code TYPE_CODE_INTERNAL_FUNCTION,
> which is not explicitly handled by that switch.
>
> That comment quoted above says that gcc -Wall will reveal any types
> that haven't been handled, but that's not actually true, at least with
> modern GCCs.  You would need to enable -Wswitch-enum for that, which
> we don't.  If I do enable that warning, then I see that we're missing
> handling for the following type codes:
>
>    TYPE_CODE_INTERNAL_FUNCTION,
>    TYPE_CODE_MODULE,
>    TYPE_CODE_NAMELIST,
>    TYPE_CODE_XMETHOD
>
> TYPE_CODE_MODULE and TYPE_CODE_NAMELIST and Fortran-specific, so it'd
> be a little weird to handle them here.
>
> I tried to reach this code with TYPE_CODE_XMETHOD, but couldn't figure
> out how to.  ptype on an xmethod isn't treated specially, it just
> complains that the method doesn't exist.  I've extended the
> gdb.python/py-xmethods.exp testcase to make sure of that.
>
> My thinking is that whatever type code we add next, the most likely
> scenario is that it won't need any special handling, so we'd just be
> adding another case to that "do nothing" list.  If we do need special
> casing for whatever type code, I think that tests added at the same
> time as the feature would uncover it anyhow.  If we do miss adding the
> special casing, then it still looks better to me to print the type
> somewhat incompletely than to error out and make it harder for users
> to debug whatever they need.  So I think that the best thing to do
> here is to just remove all those explicit "do nothing" cases, along
> with the error default case.
>
> After doing that, I decided to write a testcase that iterates over all
> supported languages doing "ptype INTERNAL_FUNC".  That revealed that
> Pascal has a similar problem, except the default case hits a
> gdb_assert instead of an error:
>
>  (gdb) with language pascal -- ptype $_isvoid
>  type =
>  ../../src/gdb/p-typeprint.c:268: internal-error: type_print_varspec_prefix: unexpected type
>  A problem internal to GDB has been detected,
>  further debugging may prove unreliable.
>
> That is fixed by this patch in the same way.
>
> You'll notice that the new testcase special-cases the Ada expected
> output:
>
> 	} elseif {$lang == "ada"} {
> 	    gdb_test "ptype \$_isvoid" "<<internal function>>"
> 	} else {
> 	    gdb_test "ptype \$_isvoid" "<internal function>"
> 	}
>
> That will be subject of the following patch.
>
> Change-Id: I81aec03523cceb338b5180a0b4c2e4ad26b4c4db
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30105

LGTM.

Thanks,
Andrew

> ---
>  gdb/c-typeprint.c                             | 51 -------------------
>  gdb/p-typeprint.c                             | 46 -----------------
>  .../gdb.base/internal-functions-ptype.exp     | 42 +++++++++++++++
>  gdb/testsuite/gdb.python/py-xmethods.exp      |  8 +++
>  4 files changed, 50 insertions(+), 97 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/internal-functions-ptype.exp
>
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index dca96231117..7e9d941a435 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -441,31 +441,6 @@ c_type_print_varspec_prefix (struct type *type,
>  				   stream, show, passed_a_ptr, 0,
>  				   language, flags, podata);
>        break;
> -
> -    case TYPE_CODE_UNDEF:
> -    case TYPE_CODE_STRUCT:
> -    case TYPE_CODE_UNION:
> -    case TYPE_CODE_ENUM:
> -    case TYPE_CODE_FLAGS:
> -    case TYPE_CODE_INT:
> -    case TYPE_CODE_FLT:
> -    case TYPE_CODE_VOID:
> -    case TYPE_CODE_ERROR:
> -    case TYPE_CODE_CHAR:
> -    case TYPE_CODE_BOOL:
> -    case TYPE_CODE_SET:
> -    case TYPE_CODE_RANGE:
> -    case TYPE_CODE_STRING:
> -    case TYPE_CODE_COMPLEX:
> -    case TYPE_CODE_NAMESPACE:
> -    case TYPE_CODE_DECFLOAT:
> -    case TYPE_CODE_FIXED_POINT:
> -      /* These types need no prefix.  They are listed here so that
> -	 gcc -Wall will reveal any types that haven't been handled.  */
> -      break;
> -    default:
> -      error (_("type not handled in c_type_print_varspec_prefix()"));
> -      break;
>      }
>  }
>  
> @@ -821,32 +796,6 @@ c_type_print_varspec_suffix (struct type *type,
>        c_type_print_varspec_suffix (type->target_type (), stream,
>  				   show, passed_a_ptr, 0, language, flags);
>        break;
> -
> -    case TYPE_CODE_UNDEF:
> -    case TYPE_CODE_STRUCT:
> -    case TYPE_CODE_UNION:
> -    case TYPE_CODE_FLAGS:
> -    case TYPE_CODE_ENUM:
> -    case TYPE_CODE_INT:
> -    case TYPE_CODE_FLT:
> -    case TYPE_CODE_VOID:
> -    case TYPE_CODE_ERROR:
> -    case TYPE_CODE_CHAR:
> -    case TYPE_CODE_BOOL:
> -    case TYPE_CODE_SET:
> -    case TYPE_CODE_RANGE:
> -    case TYPE_CODE_STRING:
> -    case TYPE_CODE_COMPLEX:
> -    case TYPE_CODE_NAMESPACE:
> -    case TYPE_CODE_DECFLOAT:
> -    case TYPE_CODE_FIXED_POINT:
> -      /* These types do not need a suffix.  They are listed so that
> -	 gcc -Wall will report types that may not have been
> -	 considered.  */
> -      break;
> -    default:
> -      error (_("type not handled in c_type_print_varspec_suffix()"));
> -      break;
>      }
>  }
>  
> diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
> index e8542d6845a..7458aa6c095 100644
> --- a/gdb/p-typeprint.c
> +++ b/gdb/p-typeprint.c
> @@ -244,29 +244,6 @@ pascal_language::type_print_varspec_prefix (struct type *type,
>  		    plongest (type->bounds ()->high.const_val ()));
>        gdb_printf (stream, "of ");
>        break;
> -
> -    case TYPE_CODE_UNDEF:
> -    case TYPE_CODE_STRUCT:
> -    case TYPE_CODE_UNION:
> -    case TYPE_CODE_ENUM:
> -    case TYPE_CODE_INT:
> -    case TYPE_CODE_FLT:
> -    case TYPE_CODE_VOID:
> -    case TYPE_CODE_ERROR:
> -    case TYPE_CODE_CHAR:
> -    case TYPE_CODE_BOOL:
> -    case TYPE_CODE_SET:
> -    case TYPE_CODE_RANGE:
> -    case TYPE_CODE_STRING:
> -    case TYPE_CODE_COMPLEX:
> -    case TYPE_CODE_TYPEDEF:
> -    case TYPE_CODE_FIXED_POINT:
> -      /* These types need no prefix.  They are listed here so that
> -	 gcc -Wall will reveal any types that haven't been handled.  */
> -      break;
> -    default:
> -      gdb_assert_not_reached ("unexpected type");
> -      break;
>      }
>  }
>  
> @@ -377,29 +354,6 @@ pascal_language::type_print_varspec_suffix (struct type *type,
>        type_print_func_varspec_suffix (type, stream, show,
>  					     passed_a_ptr, 0, flags);
>        break;
> -
> -    case TYPE_CODE_UNDEF:
> -    case TYPE_CODE_STRUCT:
> -    case TYPE_CODE_UNION:
> -    case TYPE_CODE_ENUM:
> -    case TYPE_CODE_INT:
> -    case TYPE_CODE_FLT:
> -    case TYPE_CODE_VOID:
> -    case TYPE_CODE_ERROR:
> -    case TYPE_CODE_CHAR:
> -    case TYPE_CODE_BOOL:
> -    case TYPE_CODE_SET:
> -    case TYPE_CODE_RANGE:
> -    case TYPE_CODE_STRING:
> -    case TYPE_CODE_COMPLEX:
> -    case TYPE_CODE_TYPEDEF:
> -    case TYPE_CODE_FIXED_POINT:
> -      /* These types do not need a suffix.  They are listed so that
> -	 gcc -Wall will report types that may not have been considered.  */
> -      break;
> -    default:
> -      gdb_assert_not_reached ("unexpected type");
> -      break;
>      }
>  }
>  
> diff --git a/gdb/testsuite/gdb.base/internal-functions-ptype.exp b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
> new file mode 100644
> index 00000000000..42caae05aad
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
> @@ -0,0 +1,42 @@
> +# Copyright 2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test "ptype INTERNAL_FUNCTION" in all languages.
> +
> +proc test_ptype_internal_function {} {
> +    set all_languages [get_set_option_choices "set language"]
> +
> +    foreach_with_prefix lang $all_languages {
> +	if { $lang == "auto" || $lang == "local" } {
> +	    # Avoid duplicate testing.
> +	    continue
> +	}
> +
> +	gdb_test_no_output "set language $lang"
> +
> +	if {$lang == "unknown"} {
> +	    gdb_test "ptype \$_isvoid" \
> +		"expression parsing not implemented for language \"Unknown\""
> +	} elseif {$lang == "ada"} {
> +	    gdb_test "ptype \$_isvoid" "<<internal function>>"
> +	} else {
> +	    gdb_test "ptype \$_isvoid" "<internal function>"
> +	}
> +    }
> +}
> +
> +clean_restart
> +
> +test_ptype_internal_function
> diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
> index 97d560476fc..2cf7bbb68b0 100644
> --- a/gdb/testsuite/gdb.python/py-xmethods.exp
> +++ b/gdb/testsuite/gdb.python/py-xmethods.exp
> @@ -52,6 +52,9 @@ gdb_test "p a_geta" ".* = 1" "before: a_geta 1"
>  gdb_test "p ++a1" "No symbol.*" "before: ++a1"
>  gdb_test "p a1.getarrayind(5)" "Couldn't find method.*" \
>    "before: a1.getarrayind(5)"
> +gdb_test "ptype a1.getarrayind" \
> +    "There is no member or method named getarrayind\\." \
> +    "before: ptype a1.getarrayind"
>  
>  gdb_test "p a_ptr->geta()" ".* = 60" "before: a_ptr->geta()"
>  gdb_test "p b_geta" ".* = 1" "before: b_geta 1"
> @@ -94,9 +97,14 @@ gdb_test "p b1 - a1" ".* = 25" "after: b1 - a1"
>  gdb_test "p a_minus_a" ".* = 4" "after: a_minus_a 4"
>  
>  gdb_test "p a1.geta()" "From Python <A_geta>.*5" "after: a1.geta()"
> +
>  gdb_test "p ++a1" "From Python <plus_plus_A>.*6" "after: ++a1"
>  gdb_test "p a1.getarrayind(5)" "From Python <A_getarrayind>.*5" \
>    "after: a1.getarrayind(5)"
> +gdb_test "ptype a1.getarrayind" \
> +  "There is no member or method named getarrayind\\." \
> +  "after: ptype a1.getarrayind"
> +
>  gdb_test "p a1\[6\]" ".*int &.*6" "after a1\[\]"
>  gdb_test "p b1\[7\]" ".*const int &.*7" "after b1\[\]"
>  # Note the following test.  Xmethods on dynamc types are not looked up
> -- 
> 2.36.0

Re: [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105)

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> That comment quoted above says that gcc -Wall will reveal any types
Pedro> that haven't been handled, but that's not actually true, at least with
Pedro> modern GCCs.  You would need to enable -Wswitch-enum for that, which
Pedro> we don't.

We could do it for selected switches using a pragma, if we wanted.
(What would be cool is an attribute to apply to the switch so we didn't
have to wrap the whole thing.)

I tried enabling it globally but it has a lot of noise, and adding
"default" all over defeats the purpose of the flag.

Tom

Re: [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105)

[Pedro Alves]

On 2023-02-14 3:26 p.m., Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> That comment quoted above says that gcc -Wall will reveal any types
> Pedro> that haven't been handled, but that's not actually true, at least with
> Pedro> modern GCCs.  You would need to enable -Wswitch-enum for that, which
> Pedro> we don't.
> 
> We could do it for selected switches using a pragma, if we wanted.

Yeah, that is actually what I had done locally, to check which enums we
weren't handling, with the patch below.

> (What would be cool is an attribute to apply to the switch so we didn't
> have to wrap the whole thing.)

Yeah!  I have the vague impression of once seeing that clang has something
like that, but I may be wrong.

> 
> I tried enabling it globally but it has a lot of noise, and adding
> "default" all over defeats the purpose of the flag.

*nod*

From 715b0e7d83f02fe2e6356ea297d81dd6e3105858 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Thu, 9 Feb 2023 21:26:50 +0000
Subject: [PATCH] -Wswitch-enum

Change-Id: I09d8bab56abe241e21564812e910452a68752b1e
---
 gdb/c-typeprint.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index dca96231117..3966961f149 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -372,6 +372,9 @@ c_type_print_varspec_prefix (struct type *type,
 
   QUIT;
 
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_ERROR ("-Wswitch-enum")
+
   switch (type->code ())
     {
     case TYPE_CODE_PTR:
@@ -467,6 +470,8 @@ c_type_print_varspec_prefix (struct type *type,
       error (_("type not handled in c_type_print_varspec_prefix()"));
       break;
     }
+
+DIAGNOSTIC_POP
 }
 
 /* Print out "const" and "volatile" attributes,
@@ -763,6 +768,9 @@ c_type_print_varspec_suffix (struct type *type,
 
   QUIT;
 
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_ERROR ("-Wswitch-enum")
+
   switch (type->code ())
     {
     case TYPE_CODE_ARRAY:
@@ -848,6 +856,9 @@ c_type_print_varspec_suffix (struct type *type,
       error (_("type not handled in c_type_print_varspec_suffix()"));
       break;
     }
+
+DIAGNOSTIC_POP
+
 }
 
 /* A helper for c_type_print_base that displays template

base-commit: b885aea1bb987435929b4298982ac6fc27f69403
prerequisite-patch-id: 8137bd248a83bbcb2cacfb0250eef1994eafaeac
prerequisite-patch-id: 78b3c4f199e7af4170c5fc713bd88fd4ac62dc36
-- 
2.36.0

Re: [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105)

[Tom Tromey]

>> (What would be cool is an attribute to apply to the switch so we didn't
>> have to wrap the whole thing.)

Pedro> Yeah!  I have the vague impression of once seeing that clang has something
Pedro> like that, but I may be wrong.

I filed a GCC bug about it.

Tom

[PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Pedro Alves]

Currently, printing the type of an internal function in Ada shows
double <>s, like:

 (gdb) with language ada -- ptype $_isvoid
 type = <<internal function>>

while all other languages print it with a single <>, like:

 (gdb) with language c -- ptype $_isvoid
 type = <internal function>

I don't think there's a reason that Ada needs to be different.  We
currently print the double <>s because we take this path in
ada_print_type:

    switch (type->code ())
      {
      default:
	gdb_printf (stream, "<");
	c_print_type (type, "", stream, show, level, language_ada, flags);
	gdb_printf (stream, ">");
	break;

... and the type's name already has the <>s.

Fix this by simply adding an early check for
TYPE_CODE_INTERNAL_FUNCTION.

Change-Id: Ic2b6527b9240a367471431023f6e27e6daed5501
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30105
---
 gdb/ada-typeprint.c                                 | 7 +++++++
 gdb/testsuite/gdb.base/internal-functions-ptype.exp | 2 --
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index e95034c9285..3866b2d35eb 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -941,6 +941,13 @@ ada_print_type (struct type *type0, const char *varstring,
 		struct ui_file *stream, int show, int level,
 		const struct type_print_options *flags)
 {
+  if (type0->code () == TYPE_CODE_INTERNAL_FUNCTION)
+    {
+      c_print_type (type0, "", stream, show, level,
+		    language_ada, flags);
+      return;
+    }
+
   struct type *type = ada_check_typedef (ada_get_base_type (type0));
   /* If we can decode the original type name, use it.  However, there
      are cases where the original type is an internally-generated type
diff --git a/gdb/testsuite/gdb.base/internal-functions-ptype.exp b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
index 42caae05aad..748f33a87cd 100644
--- a/gdb/testsuite/gdb.base/internal-functions-ptype.exp
+++ b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
@@ -29,8 +29,6 @@ proc test_ptype_internal_function {} {
 	if {$lang == "unknown"} {
 	    gdb_test "ptype \$_isvoid" \
 		"expression parsing not implemented for language \"Unknown\""
-	} elseif {$lang == "ada"} {
-	    gdb_test "ptype \$_isvoid" "<<internal function>>"
 	} else {
 	    gdb_test "ptype \$_isvoid" "<internal function>"
 	}
-- 
2.36.0

Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Andrew Burgess]

Pedro Alves <pedro@palves.net> writes:

> Currently, printing the type of an internal function in Ada shows
> double <>s, like:
>
>  (gdb) with language ada -- ptype $_isvoid
>  type = <<internal function>>
>
> while all other languages print it with a single <>, like:
>
>  (gdb) with language c -- ptype $_isvoid
>  type = <internal function>
>
> I don't think there's a reason that Ada needs to be different.  We
> currently print the double <>s because we take this path in
> ada_print_type:
>
>     switch (type->code ())
>       {
>       default:
> 	gdb_printf (stream, "<");
> 	c_print_type (type, "", stream, show, level, language_ada, flags);
> 	gdb_printf (stream, ">");
> 	break;
>
> ... and the type's name already has the <>s.
>
> Fix this by simply adding an early check for
> TYPE_CODE_INTERNAL_FUNCTION.

I confess, this is not the solution I though you'd go with.  I was
expecting you to handle TYPE_CODE_INTERNAL_FUNCTION in the switch, just
to leave things consistent.

I guess it doesn't hurt though, so LGTM.

Thanks,
Andrew

>
> Change-Id: Ic2b6527b9240a367471431023f6e27e6daed5501
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30105
> ---
>  gdb/ada-typeprint.c                                 | 7 +++++++
>  gdb/testsuite/gdb.base/internal-functions-ptype.exp | 2 --
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
> index e95034c9285..3866b2d35eb 100644
> --- a/gdb/ada-typeprint.c
> +++ b/gdb/ada-typeprint.c
> @@ -941,6 +941,13 @@ ada_print_type (struct type *type0, const char *varstring,
>  		struct ui_file *stream, int show, int level,
>  		const struct type_print_options *flags)
>  {
> +  if (type0->code () == TYPE_CODE_INTERNAL_FUNCTION)
> +    {
> +      c_print_type (type0, "", stream, show, level,
> +		    language_ada, flags);
> +      return;
> +    }
> +
>    struct type *type = ada_check_typedef (ada_get_base_type (type0));
>    /* If we can decode the original type name, use it.  However, there
>       are cases where the original type is an internally-generated type
> diff --git a/gdb/testsuite/gdb.base/internal-functions-ptype.exp b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
> index 42caae05aad..748f33a87cd 100644
> --- a/gdb/testsuite/gdb.base/internal-functions-ptype.exp
> +++ b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
> @@ -29,8 +29,6 @@ proc test_ptype_internal_function {} {
>  	if {$lang == "unknown"} {
>  	    gdb_test "ptype \$_isvoid" \
>  		"expression parsing not implemented for language \"Unknown\""
> -	} elseif {$lang == "ada"} {
> -	    gdb_test "ptype \$_isvoid" "<<internal function>>"
>  	} else {
>  	    gdb_test "ptype \$_isvoid" "<internal function>"
>  	}
> -- 
> 2.36.0

Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Tom Tromey]

>> I don't think there's a reason that Ada needs to be different.

Agreed.

>> Fix this by simply adding an early check for
>> TYPE_CODE_INTERNAL_FUNCTION.

Andrew> I confess, this is not the solution I though you'd go with.  I was
Andrew> expecting you to handle TYPE_CODE_INTERNAL_FUNCTION in the switch, just
Andrew> to leave things consistent.

I think this would be better.

Tom

Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Pedro Alves]

On 2023-02-14 3:30 p.m., Tom Tromey wrote:

>>> Fix this by simply adding an early check for
>>> TYPE_CODE_INTERNAL_FUNCTION.
> 
> Andrew> I confess, this is not the solution I though you'd go with.  I was
> Andrew> expecting you to handle TYPE_CODE_INTERNAL_FUNCTION in the switch, just
> Andrew> to leave things consistent.
> 
> I think this would be better.

My point with adding this check early is that these functions' type never
has anything to do with Ada, so all that code at the beginning of ada_print_type,
like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
etc. is always a nop for internal functions, so might as well skip it all.

I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
to common code (say, rename the virtual language_defn::print_type to do_print_type,
and add a new non-virtual wrapper language_defn::print_type method and print
TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
myself that a different language might want to print it differently (say, some other
characters instead of "<>"), and I guess that's why I ended up with putting it at the start
of the function, as that is the closest to putting it at the caller instead.

Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Pedro Alves]

On 2023-02-15 1:38 p.m., Pedro Alves wrote:
> On 2023-02-14 3:30 p.m., Tom Tromey wrote:
> 
>>>> Fix this by simply adding an early check for
>>>> TYPE_CODE_INTERNAL_FUNCTION.
>>
>> Andrew> I confess, this is not the solution I though you'd go with.  I was
>> Andrew> expecting you to handle TYPE_CODE_INTERNAL_FUNCTION in the switch, just
>> Andrew> to leave things consistent.
>>
>> I think this would be better.
> 
> My point with adding this check early is that these functions' type never
> has anything to do with Ada, so all that code at the beginning of ada_print_type,
> like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
> etc. is always a nop for internal functions, so might as well skip it all.
> 
> I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
> to common code (say, rename the virtual language_defn::print_type to do_print_type,
> and add a new non-virtual wrapper language_defn::print_type method and print
> TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
> myself that a different language might want to print it differently (say, some other
> characters instead of "<>"), and I guess that's why I ended up with putting it at the start
> of the function, as that is the closest to putting it at the caller instead.
> 

Here's the alternative patch handling TYPE_CODE_INTERNAL_FUNCTION in the switch.

WDYT?

From: Pedro Alves <pedro@palves.net>
Subject: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other
 languages

Currently, printing the type of an internal function in Ada shows
double <>s, like:

 (gdb) with language ada -- ptype $_isvoid
 type = <<internal function>>

while all other languages print it with a single <>, like:

 (gdb) with language c -- ptype $_isvoid
 type = <internal function>

I don't think there's a reason that Ada needs to be different.  We
currently print the double <>s because we take this path in
ada_print_type:

    switch (type->code ())
      {
      default:
	gdb_printf (stream, "<");
	c_print_type (type, "", stream, show, level, language_ada, flags);
	gdb_printf (stream, ">");
	break;

... and the type's name already has the <>s.

Fix this by making ada_print_type handle TYPE_CODE_INTERNAL_FUNCTION.

Change-Id: Ic2b6527b9240a367471431023f6e27e6daed5501
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30105
---
 gdb/ada-typeprint.c                                 | 3 +++
 gdb/testsuite/gdb.base/internal-functions-ptype.exp | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index e95034c9285..e094bc4c9f4 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -991,6 +991,9 @@ ada_print_type (struct type *type0, const char *varstring,
 	c_print_type (type, "", stream, show, level, language_ada, flags);
 	gdb_printf (stream, ">");
 	break;
+      case TYPE_CODE_INTERNAL_FUNCTION:
+	c_print_type (type, "", stream, show, level, language_ada, flags);
+	break;
       case TYPE_CODE_PTR:
       case TYPE_CODE_TYPEDEF:
 	/* An __XVL field is not truly a pointer, so don't print
diff --git a/gdb/testsuite/gdb.base/internal-functions-ptype.exp b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
index 42caae05aad..748f33a87cd 100644
--- a/gdb/testsuite/gdb.base/internal-functions-ptype.exp
+++ b/gdb/testsuite/gdb.base/internal-functions-ptype.exp
@@ -29,8 +29,6 @@ proc test_ptype_internal_function {} {
 	if {$lang == "unknown"} {
 	    gdb_test "ptype \$_isvoid" \
 		"expression parsing not implemented for language \"Unknown\""
-	} elseif {$lang == "ada"} {
-	    gdb_test "ptype \$_isvoid" "<<internal function>>"
 	} else {
 	    gdb_test "ptype \$_isvoid" "<internal function>"
 	}

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
-- 
2.36.0

Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages

[Tom Tromey]

>> I think this would be better.

Pedro> My point with adding this check early is that these functions' type never
Pedro> has anything to do with Ada, so all that code at the beginning of ada_print_type,
Pedro> like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
Pedro> etc. is always a nop for internal functions, so might as well skip it all.

I see.  That makes sense to me, thanks.

Sorry about the noise, as far as I'm concerned you can land either
version of the patch.

Pedro> I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
Pedro> to common code (say, rename the virtual language_defn::print_type to do_print_type,
Pedro> and add a new non-virtual wrapper language_defn::print_type method and print
Pedro> TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
Pedro> myself that a different language might want to print it differently (say, some other
Pedro> characters instead of "<>")

I think this is something we can simply force on languages.  The type of
these things isn't a language feature and probably isn't worth
customizing anyway.

More generally I think it's better for gdb to try to do more in the
common code and leave less to the languages.

thanks,
Tom

[PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code (was: Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages)

[Pedro Alves]

On 2023-02-15 4:56 p.m., Tom Tromey wrote:
>>> I think this would be better.
> 
> Pedro> My point with adding this check early is that these functions' type never
> Pedro> has anything to do with Ada, so all that code at the beginning of ada_print_type,
> Pedro> like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
> Pedro> etc. is always a nop for internal functions, so might as well skip it all.
> 
> I see.  That makes sense to me, thanks.
> 
> Sorry about the noise, as far as I'm concerned you can land either
> version of the patch.

No problem.  I've merged the original version, along with the whole series.

> 
> Pedro> I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
> Pedro> to common code (say, rename the virtual language_defn::print_type to do_print_type,
> Pedro> and add a new non-virtual wrapper language_defn::print_type method and print
> Pedro> TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
> Pedro> myself that a different language might want to print it differently (say, some other
> Pedro> characters instead of "<>")
> 
> I think this is something we can simply force on languages.  The type of
> these things isn't a language feature and probably isn't worth
> customizing anyway.
> 
> More generally I think it's better for gdb to try to do more in the
> common code and leave less to the languages.

I implemented the idea I mentioned above.

WDYT?

From 73958ac0ee47fd1a887515a47436cb3835514724 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 15 Feb 2023 20:19:10 +0000
Subject: [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code

Internal functions should print the same for all languages.  It is
currently left to each language to make sure that is true.  This patch
centralizes the logic.

Rename language_defn::print_type to language_defn::do_print_type and
making it protected, and then add a new public non-virtual
language_defn::print_type method that implements
TYPE_CODE_INTERNAL_FUNCTION type printing before dispatching to the
virtual language_defn::do_print_type.

Change-Id: Idd5fbb437cc91990b051e1a9a027c3909c09dd31
---
 gdb/ada-lang.c      |  6 +++---
 gdb/ada-typeprint.c |  7 -------
 gdb/c-lang.c        | 24 ++++++++++++------------
 gdb/d-lang.c        |  6 +++---
 gdb/f-lang.h        |  6 +++---
 gdb/f-typeprint.c   |  6 +++---
 gdb/go-lang.h       |  6 +++---
 gdb/go-typeprint.c  |  6 +++---
 gdb/language.c      | 25 ++++++++++++++++++++++---
 gdb/language.h      | 21 +++++++++++++++++----
 gdb/m2-lang.h       |  6 +++---
 gdb/objc-lang.c     |  6 +++---
 gdb/opencl-lang.c   |  6 +++---
 gdb/p-lang.h        |  6 +++---
 gdb/p-typeprint.c   |  6 +++---
 gdb/rust-lang.c     |  6 +++---
 gdb/rust-lang.h     |  6 +++---
 17 files changed, 90 insertions(+), 65 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index ec85729042f..e3bbfdeae73 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13625,9 +13625,9 @@ class ada_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     ada_print_type (type, varstring, stream, show, level, flags);
   }
diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 3866b2d35eb..e95034c9285 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -941,13 +941,6 @@ ada_print_type (struct type *type0, const char *varstring,
 		struct ui_file *stream, int show, int level,
 		const struct type_print_options *flags)
 {
-  if (type0->code () == TYPE_CODE_INTERNAL_FUNCTION)
-    {
-      c_print_type (type0, "", stream, show, level,
-		    language_ada, flags);
-      return;
-    }
-
   struct type *type = ada_check_typedef (ada_get_base_type (type0));
   /* If we can decode the original type name, use it.  However, there
      are cases where the original type is an internally-generated type
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 6535ab93498..9832d676445 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -836,9 +836,9 @@ class c_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
@@ -993,9 +993,9 @@ class cplus_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
@@ -1100,9 +1100,9 @@ class asm_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
@@ -1159,9 +1159,9 @@ class minimal_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index 8286c5be646..a374fcfeb8f 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -151,9 +151,9 @@ class d_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
diff --git a/gdb/f-lang.h b/gdb/f-lang.h
index 673e273d31a..7c44a46b981 100644
--- a/gdb/f-lang.h
+++ b/gdb/f-lang.h
@@ -87,9 +87,9 @@ class f_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override;
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override;
 
   /* See language.h.  This just returns default set of word break
      characters but with the modules separator `::' removed.  */
diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
index e4aed6e5904..b3c675e2c2a 100644
--- a/gdb/f-typeprint.c
+++ b/gdb/f-typeprint.c
@@ -46,9 +46,9 @@ f_language::print_typedef (struct type *type, struct symbol *new_symbol,
 /* See f-lang.h.  */
 
 void
-f_language::print_type (struct type *type, const char *varstring,
-			struct ui_file *stream, int show, int level,
-			const struct type_print_options *flags) const
+f_language::do_print_type (struct type *type, const char *varstring,
+			   struct ui_file *stream, int show, int level,
+			   const struct type_print_options *flags) const
 {
   enum type_code code;
 
diff --git a/gdb/go-lang.h b/gdb/go-lang.h
index 1820b4c9658..df607d053f9 100644
--- a/gdb/go-lang.h
+++ b/gdb/go-lang.h
@@ -110,9 +110,9 @@ class go_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override;
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override;
 
   /* See language.h.  */
 
diff --git a/gdb/go-typeprint.c b/gdb/go-typeprint.c
index 0c4e9a26563..c664aaa8284 100644
--- a/gdb/go-typeprint.c
+++ b/gdb/go-typeprint.c
@@ -42,9 +42,9 @@
    LEVEL indicates level of recursion (for nested definitions).  */
 
 void
-go_language::print_type (struct type *type, const char *varstring,
-			 struct ui_file *stream, int show, int level,
-			 const struct type_print_options *flags) const
+go_language::do_print_type (struct type *type, const char *varstring,
+			    struct ui_file *stream, int show, int level,
+			    const struct type_print_options *flags) const
 {
   /* Borrowed from c-typeprint.c.  */
   if (show > 0)
diff --git a/gdb/language.c b/gdb/language.c
index 50a53c647f5..23601c42ad6 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -734,9 +734,9 @@ class auto_or_unknown_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     error (_("type printing not implemented for language \"%s\""),
 	   natural_name ());
@@ -1094,6 +1094,25 @@ language_lookup_primitive_type_as_symbol (const struct language_defn *la,
   return sym;
 }
 
+/* See language.h.  */
+
+void
+language_defn::print_type (struct type *type, const char *varstring,
+			   struct ui_file *stream, int show, int level,
+			   const struct type_print_options *flags) const
+{
+  /* Print things here which should print the same for every language,
+     before dispatching to the virtual method.  */
+  if (type->code () == TYPE_CODE_INTERNAL_FUNCTION)
+    {
+      c_print_type (type, varstring, stream, show, level,
+		    la_language, flags);
+      return;
+    }
+
+  do_print_type (type, varstring, stream, show, level, flags);
+}
+
 /* Initialize the language routines.  */
 
 void _initialize_language ();
diff --git a/gdb/language.h b/gdb/language.h
index a51ddf97381..ebe3ffff00e 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -461,11 +461,24 @@ struct language_defn
   /* Print TYPE to STREAM using syntax appropriate for this language.
      LEVEL is the depth to indent lines by.  VARSTRING, if not NULL or the
      empty string, is the name of a variable and TYPE should be printed in
-     the form of a declaration of a variable named VARSTRING.  */
+     the form of a declaration of a variable named VARSTRING.
 
-  virtual void print_type (struct type *type, const char *varstring,
-			   struct ui_file *stream, int show, int level,
-			   const struct type_print_options *flags) const = 0;
+     This is the public-facing method, which contains code that is
+     common to all languages, and then dispatches to the virtual
+     do_print_type method.  */
+  void print_type (struct type *type, const char *varstring,
+		   struct ui_file *stream, int show, int level,
+		   const struct type_print_options *flags) const;
+
+protected:
+
+  /* Implements actual language-specific parts of print_type.
+     Arguments and return are like the print_type method.  */
+  virtual void do_print_type (struct type *type, const char *varstring,
+			      struct ui_file *stream, int show, int level,
+			      const struct type_print_options *flags) const = 0;
+
+public:
 
   /* PC is possibly an unknown languages trampoline.
      If that PC falls in a trampoline belonging to this language, return
diff --git a/gdb/m2-lang.h b/gdb/m2-lang.h
index cda6e241c8c..7d7556fabbd 100644
--- a/gdb/m2-lang.h
+++ b/gdb/m2-lang.h
@@ -73,9 +73,9 @@ class m2_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     m2_print_type (type, varstring, stream, show, level, flags);
   }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index f43d158a770..7964e6da8c5 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -273,9 +273,9 @@ class objc_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     c_print_type (type, varstring, stream, show, level, la_language, flags);
   }
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 3e4a9c360b2..14df2756360 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -959,9 +959,9 @@ class opencl_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override
   {
     /* We nearly always defer to C type printing, except that vector types
        are considered primitive in OpenCL, and should always be printed
diff --git a/gdb/p-lang.h b/gdb/p-lang.h
index 662079114ed..c16a4cd528e 100644
--- a/gdb/p-lang.h
+++ b/gdb/p-lang.h
@@ -88,9 +88,9 @@ class pascal_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override;
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override;
 
   /* See language.h.  */
 
diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
index 7458aa6c095..568ff61b607 100644
--- a/gdb/p-typeprint.c
+++ b/gdb/p-typeprint.c
@@ -37,9 +37,9 @@
 /* See language.h.  */
 
 void
-pascal_language::print_type (struct type *type, const char *varstring,
-			     struct ui_file *stream, int show, int level,
-			     const struct type_print_options *flags) const
+pascal_language::do_print_type (struct type *type, const char *varstring,
+				struct ui_file *stream, int show, int level,
+				const struct type_print_options *flags) const
 {
   enum type_code code;
   int demangled_args;
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index fb9db9fe31b..8424736e045 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1626,9 +1626,9 @@ rust_language::language_arch_info (struct gdbarch *gdbarch,
 /* See language.h.  */
 
 void
-rust_language::print_type (struct type *type, const char *varstring,
-			   struct ui_file *stream, int show, int level,
-			   const struct type_print_options *flags) const
+rust_language::do_print_type (struct type *type, const char *varstring,
+			      struct ui_file *stream, int show, int level,
+			      const struct type_print_options *flags) const
 {
   print_offset_data podata (flags);
   rust_internal_print_type (type, varstring, stream, show, level,
diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
index 89e03550fb7..ef95fbd3f3d 100644
--- a/gdb/rust-lang.h
+++ b/gdb/rust-lang.h
@@ -114,9 +114,9 @@ class rust_language : public language_defn
 
   /* See language.h.  */
 
-  void print_type (struct type *type, const char *varstring,
-		   struct ui_file *stream, int show, int level,
-		   const struct type_print_options *flags) const override;
+  void do_print_type (struct type *type, const char *varstring,
+		      struct ui_file *stream, int show, int level,
+		      const struct type_print_options *flags) const override;
 
   /* See language.h.  */
 

base-commit: 141cd158423a5ee248245fb2075fd2e5a580cff2
-- 
2.36.0

Re: [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code (was: Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages)

[Andrew Burgess]

Pedro Alves <pedro@palves.net> writes:

> On 2023-02-15 4:56 p.m., Tom Tromey wrote:
>>>> I think this would be better.
>> 
>> Pedro> My point with adding this check early is that these functions' type never
>> Pedro> has anything to do with Ada, so all that code at the beginning of ada_print_type,
>> Pedro> like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
>> Pedro> etc. is always a nop for internal functions, so might as well skip it all.
>> 
>> I see.  That makes sense to me, thanks.
>> 
>> Sorry about the noise, as far as I'm concerned you can land either
>> version of the patch.
>
> No problem.  I've merged the original version, along with the whole series.
>
>> 
>> Pedro> I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
>> Pedro> to common code (say, rename the virtual language_defn::print_type to do_print_type,
>> Pedro> and add a new non-virtual wrapper language_defn::print_type method and print
>> Pedro> TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
>> Pedro> myself that a different language might want to print it differently (say, some other
>> Pedro> characters instead of "<>")
>> 
>> I think this is something we can simply force on languages.  The type of
>> these things isn't a language feature and probably isn't worth
>> customizing anyway.
>> 
>> More generally I think it's better for gdb to try to do more in the
>> common code and leave less to the languages.
>
> I implemented the idea I mentioned above.
>
> WDYT?
>
> From 73958ac0ee47fd1a887515a47436cb3835514724 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Wed, 15 Feb 2023 20:19:10 +0000
> Subject: [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code
>
> Internal functions should print the same for all languages.  It is
> currently left to each language to make sure that is true.  This patch
> centralizes the logic.
>
> Rename language_defn::print_type to language_defn::do_print_type and
> making it protected, and then add a new public non-virtual
> language_defn::print_type method that implements
> TYPE_CODE_INTERNAL_FUNCTION type printing before dispatching to the
> virtual language_defn::do_print_type.

Looks like a good change to me.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Change-Id: Idd5fbb437cc91990b051e1a9a027c3909c09dd31
> ---
>  gdb/ada-lang.c      |  6 +++---
>  gdb/ada-typeprint.c |  7 -------
>  gdb/c-lang.c        | 24 ++++++++++++------------
>  gdb/d-lang.c        |  6 +++---
>  gdb/f-lang.h        |  6 +++---
>  gdb/f-typeprint.c   |  6 +++---
>  gdb/go-lang.h       |  6 +++---
>  gdb/go-typeprint.c  |  6 +++---
>  gdb/language.c      | 25 ++++++++++++++++++++++---
>  gdb/language.h      | 21 +++++++++++++++++----
>  gdb/m2-lang.h       |  6 +++---
>  gdb/objc-lang.c     |  6 +++---
>  gdb/opencl-lang.c   |  6 +++---
>  gdb/p-lang.h        |  6 +++---
>  gdb/p-typeprint.c   |  6 +++---
>  gdb/rust-lang.c     |  6 +++---
>  gdb/rust-lang.h     |  6 +++---
>  17 files changed, 90 insertions(+), 65 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index ec85729042f..e3bbfdeae73 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -13625,9 +13625,9 @@ class ada_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      ada_print_type (type, varstring, stream, show, level, flags);
>    }
> diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
> index 3866b2d35eb..e95034c9285 100644
> --- a/gdb/ada-typeprint.c
> +++ b/gdb/ada-typeprint.c
> @@ -941,13 +941,6 @@ ada_print_type (struct type *type0, const char *varstring,
>  		struct ui_file *stream, int show, int level,
>  		const struct type_print_options *flags)
>  {
> -  if (type0->code () == TYPE_CODE_INTERNAL_FUNCTION)
> -    {
> -      c_print_type (type0, "", stream, show, level,
> -		    language_ada, flags);
> -      return;
> -    }
> -
>    struct type *type = ada_check_typedef (ada_get_base_type (type0));
>    /* If we can decode the original type name, use it.  However, there
>       are cases where the original type is an internally-generated type
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index 6535ab93498..9832d676445 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -836,9 +836,9 @@ class c_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> @@ -993,9 +993,9 @@ class cplus_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> @@ -1100,9 +1100,9 @@ class asm_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> @@ -1159,9 +1159,9 @@ class minimal_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> diff --git a/gdb/d-lang.c b/gdb/d-lang.c
> index 8286c5be646..a374fcfeb8f 100644
> --- a/gdb/d-lang.c
> +++ b/gdb/d-lang.c
> @@ -151,9 +151,9 @@ class d_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> diff --git a/gdb/f-lang.h b/gdb/f-lang.h
> index 673e273d31a..7c44a46b981 100644
> --- a/gdb/f-lang.h
> +++ b/gdb/f-lang.h
> @@ -87,9 +87,9 @@ class f_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override;
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override;
>  
>    /* See language.h.  This just returns default set of word break
>       characters but with the modules separator `::' removed.  */
> diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
> index e4aed6e5904..b3c675e2c2a 100644
> --- a/gdb/f-typeprint.c
> +++ b/gdb/f-typeprint.c
> @@ -46,9 +46,9 @@ f_language::print_typedef (struct type *type, struct symbol *new_symbol,
>  /* See f-lang.h.  */
>  
>  void
> -f_language::print_type (struct type *type, const char *varstring,
> -			struct ui_file *stream, int show, int level,
> -			const struct type_print_options *flags) const
> +f_language::do_print_type (struct type *type, const char *varstring,
> +			   struct ui_file *stream, int show, int level,
> +			   const struct type_print_options *flags) const
>  {
>    enum type_code code;
>  
> diff --git a/gdb/go-lang.h b/gdb/go-lang.h
> index 1820b4c9658..df607d053f9 100644
> --- a/gdb/go-lang.h
> +++ b/gdb/go-lang.h
> @@ -110,9 +110,9 @@ class go_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override;
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override;
>  
>    /* See language.h.  */
>  
> diff --git a/gdb/go-typeprint.c b/gdb/go-typeprint.c
> index 0c4e9a26563..c664aaa8284 100644
> --- a/gdb/go-typeprint.c
> +++ b/gdb/go-typeprint.c
> @@ -42,9 +42,9 @@
>     LEVEL indicates level of recursion (for nested definitions).  */
>  
>  void
> -go_language::print_type (struct type *type, const char *varstring,
> -			 struct ui_file *stream, int show, int level,
> -			 const struct type_print_options *flags) const
> +go_language::do_print_type (struct type *type, const char *varstring,
> +			    struct ui_file *stream, int show, int level,
> +			    const struct type_print_options *flags) const
>  {
>    /* Borrowed from c-typeprint.c.  */
>    if (show > 0)
> diff --git a/gdb/language.c b/gdb/language.c
> index 50a53c647f5..23601c42ad6 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -734,9 +734,9 @@ class auto_or_unknown_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      error (_("type printing not implemented for language \"%s\""),
>  	   natural_name ());
> @@ -1094,6 +1094,25 @@ language_lookup_primitive_type_as_symbol (const struct language_defn *la,
>    return sym;
>  }
>  
> +/* See language.h.  */
> +
> +void
> +language_defn::print_type (struct type *type, const char *varstring,
> +			   struct ui_file *stream, int show, int level,
> +			   const struct type_print_options *flags) const
> +{
> +  /* Print things here which should print the same for every language,
> +     before dispatching to the virtual method.  */
> +  if (type->code () == TYPE_CODE_INTERNAL_FUNCTION)
> +    {
> +      c_print_type (type, varstring, stream, show, level,
> +		    la_language, flags);
> +      return;
> +    }
> +
> +  do_print_type (type, varstring, stream, show, level, flags);
> +}
> +
>  /* Initialize the language routines.  */
>  
>  void _initialize_language ();
> diff --git a/gdb/language.h b/gdb/language.h
> index a51ddf97381..ebe3ffff00e 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -461,11 +461,24 @@ struct language_defn
>    /* Print TYPE to STREAM using syntax appropriate for this language.
>       LEVEL is the depth to indent lines by.  VARSTRING, if not NULL or the
>       empty string, is the name of a variable and TYPE should be printed in
> -     the form of a declaration of a variable named VARSTRING.  */
> +     the form of a declaration of a variable named VARSTRING.
>  
> -  virtual void print_type (struct type *type, const char *varstring,
> -			   struct ui_file *stream, int show, int level,
> -			   const struct type_print_options *flags) const = 0;
> +     This is the public-facing method, which contains code that is
> +     common to all languages, and then dispatches to the virtual
> +     do_print_type method.  */
> +  void print_type (struct type *type, const char *varstring,
> +		   struct ui_file *stream, int show, int level,
> +		   const struct type_print_options *flags) const;
> +
> +protected:
> +
> +  /* Implements actual language-specific parts of print_type.
> +     Arguments and return are like the print_type method.  */
> +  virtual void do_print_type (struct type *type, const char *varstring,
> +			      struct ui_file *stream, int show, int level,
> +			      const struct type_print_options *flags) const = 0;
> +
> +public:
>  
>    /* PC is possibly an unknown languages trampoline.
>       If that PC falls in a trampoline belonging to this language, return
> diff --git a/gdb/m2-lang.h b/gdb/m2-lang.h
> index cda6e241c8c..7d7556fabbd 100644
> --- a/gdb/m2-lang.h
> +++ b/gdb/m2-lang.h
> @@ -73,9 +73,9 @@ class m2_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      m2_print_type (type, varstring, stream, show, level, flags);
>    }
> diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
> index f43d158a770..7964e6da8c5 100644
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -273,9 +273,9 @@ class objc_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      c_print_type (type, varstring, stream, show, level, la_language, flags);
>    }
> diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
> index 3e4a9c360b2..14df2756360 100644
> --- a/gdb/opencl-lang.c
> +++ b/gdb/opencl-lang.c
> @@ -959,9 +959,9 @@ class opencl_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override
>    {
>      /* We nearly always defer to C type printing, except that vector types
>         are considered primitive in OpenCL, and should always be printed
> diff --git a/gdb/p-lang.h b/gdb/p-lang.h
> index 662079114ed..c16a4cd528e 100644
> --- a/gdb/p-lang.h
> +++ b/gdb/p-lang.h
> @@ -88,9 +88,9 @@ class pascal_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override;
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override;
>  
>    /* See language.h.  */
>  
> diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
> index 7458aa6c095..568ff61b607 100644
> --- a/gdb/p-typeprint.c
> +++ b/gdb/p-typeprint.c
> @@ -37,9 +37,9 @@
>  /* See language.h.  */
>  
>  void
> -pascal_language::print_type (struct type *type, const char *varstring,
> -			     struct ui_file *stream, int show, int level,
> -			     const struct type_print_options *flags) const
> +pascal_language::do_print_type (struct type *type, const char *varstring,
> +				struct ui_file *stream, int show, int level,
> +				const struct type_print_options *flags) const
>  {
>    enum type_code code;
>    int demangled_args;
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index fb9db9fe31b..8424736e045 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -1626,9 +1626,9 @@ rust_language::language_arch_info (struct gdbarch *gdbarch,
>  /* See language.h.  */
>  
>  void
> -rust_language::print_type (struct type *type, const char *varstring,
> -			   struct ui_file *stream, int show, int level,
> -			   const struct type_print_options *flags) const
> +rust_language::do_print_type (struct type *type, const char *varstring,
> +			      struct ui_file *stream, int show, int level,
> +			      const struct type_print_options *flags) const
>  {
>    print_offset_data podata (flags);
>    rust_internal_print_type (type, varstring, stream, show, level,
> diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
> index 89e03550fb7..ef95fbd3f3d 100644
> --- a/gdb/rust-lang.h
> +++ b/gdb/rust-lang.h
> @@ -114,9 +114,9 @@ class rust_language : public language_defn
>  
>    /* See language.h.  */
>  
> -  void print_type (struct type *type, const char *varstring,
> -		   struct ui_file *stream, int show, int level,
> -		   const struct type_print_options *flags) const override;
> +  void do_print_type (struct type *type, const char *varstring,
> +		      struct ui_file *stream, int show, int level,
> +		      const struct type_print_options *flags) const override;
>  
>    /* See language.h.  */
>  
>
> base-commit: 141cd158423a5ee248245fb2075fd2e5a580cff2
> -- 
> 2.36.0

[PATCH 3/6] Add new "$_shell(CMD)" internal function

[Pedro Alves]

For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
 5 files changed, 179 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b85923cf80d..0d3445438b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -56,6 +56,16 @@ maintenance info frame-unwinders
   List the frame unwinders currently in effect, starting with the highest
   priority.
 
+* New convenience function "$_shell", to execute a shell command and
+  return the result.  This lets you run shell commands in expressions.
+  Some examples:
+
+   (gdb) p $_shell("true")
+   $1 = 0
+   (gdb) p $_shell("false")
+   $2 = 1
+   (gdb) break func if $_shell("some command") == 0
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 6c0d780face..27fbfb035b3 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/filestuff.h"
 #include "location.h"
 #include "block.h"
+#include "valprint.h"
 
 #include "ui-out.h"
 #include "interps.h"
@@ -873,6 +874,9 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
+
+  /* Keep the logic here in sync with shell_internal_fn.  */
+
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 #ifdef __MINGW32__
@@ -893,8 +897,11 @@ exit_status_set_internal_vars (int exit_status)
     warning (_("unexpected shell command exit status %d"), exit_status);
 }
 
-static void
-shell_escape (const char *arg, int from_tty)
+/* Run ARG under the shell, and return the exit status.  If ARG is
+   NULL, run an interactive shell.  */
+
+static int
+run_under_shell (const char *arg, int from_tty)
 {
 #if defined(CANT_FORK) || \
       (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
@@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
      the shell command we just ran changed it.  */
   chdir (current_directory);
 #endif
-  exit_status_set_internal_vars (rc);
+  return rc;
 #else /* Can fork.  */
   int status, pid;
 
@@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
-  exit_status_set_internal_vars (status);
+  return status;
 #endif /* Can fork.  */
 }
 
+/* Escape out to the shell to run ARG.  If ARG is NULL, launch and
+   interactive shell.  Sets $_shell_exitcode and $_shell_exitsignal
+   convenience variables based on the exits status.  */
+
+static void
+shell_escape (const char *arg, int from_tty)
+{
+  int status = run_under_shell (arg, from_tty);
+  exit_status_set_internal_vars (status);
+}
+
 /* Implementation of the "shell" command.  */
 
 static void
@@ -2417,6 +2435,63 @@ gdb_maint_setting_str_internal_fn (struct gdbarch *gdbarch,
   return str_value_from_setting (*show_cmd->var, gdbarch);
 }
 
+/* Implementation of the convenience function $_shell.  */
+
+static struct value *
+shell_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_shell."));
+
+  value *val = argv[0];
+  struct type *type = check_typedef (value_type (val));
+
+  if (!language->is_string_type_p (type))
+    error (_("Argument must be a string."));
+
+  value_print_options opts;
+  get_no_prettyformat_print_options (&opts);
+
+  string_file stream;
+  value_print (val, &stream, &opts);
+
+  /* We should always have two quote chars, which we'll strip.  */
+  gdb_assert (stream.size () >= 2);
+
+  /* Now strip them.  We don't need the original string, so it's
+     cheaper to do it in place, avoiding a string allocation.  */
+  std::string str = stream.release ();
+  str[str.size () - 1] = 0;
+  const char *command = str.c_str () + 1;
+
+  int exit_status = run_under_shell (command, 0);
+
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+
+  /* Keep the logic here in sync with
+     exit_status_set_internal_vars.  */
+
+  if (WIFEXITED (exit_status))
+    return value_from_longest (int_type, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* See exit_status_set_internal_vars.  */
+      return value_from_longest (int_type, exit_status);
+    }
+#endif
+  else if (WIFSIGNALED (exit_status))
+    {
+      /* (0x80 | SIGNO) is what most (all?) POSIX-like shells set as
+	 exit code on fatal signal termination.  */
+      return value_from_longest (int_type, 0x80 | WTERMSIG (exit_status));
+    }
+  else
+    return allocate_optimized_out_value (int_type);
+}
+
 void _initialize_cli_cmds ();
 void
 _initialize_cli_cmds ()
@@ -2606,6 +2681,12 @@ Some integer settings accept an unlimited value, returned\n\
 as 0 or -1 depending on the setting."),
 			 gdb_maint_setting_internal_fn, NULL);
 
+  add_internal_function ("_shell", _("\
+$_shell - execute a shell command and return the result.\n\
+Usage: $_shell (command)\n\
+Returns the command's exit code: zero on success, non-zero otherwise."),
+			 shell_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7b128053b5a..b2552173093 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1629,6 +1629,10 @@ the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
+You may also invoke shell commands from expressions, using the
+@code{$_shell} convenience function.  @xref{$_shell convenience
+function}.
+
 The utility @code{make} is often needed in development environments.
 You do not have to use the @code{shell} command for this purpose in
 @value{GDBN}:
@@ -12969,6 +12973,49 @@ Like the @code{$_gdb_setting_str} function, but works with
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
+@anchor{$_shell convenience function}
+@item $_shell (@var{command-string})
+@findex $_shell@r{, convenience function}
+
+Invoke a standard shell to execute @var{command-string}.  Returns the
+command's exit status.  On Unix systems, a command which exits with a
+zero exit status has succeeded, and non-zero exit status indicates
+failure.  When a command terminates on a fatal signal whose number is
+N, @value{GDBN} uses the value 128+N as the exit status, as is
+standard in Unix shells.  The shell to run is determined in the same
+way as for the @code{shell} command.  @xref{Shell Commands, ,Shell
+Commands}.  The shell runs on the host machine, the machine
+@value{GDBN} is running on.
+
+@smallexample
+(@value{GDBP}) print $_shell("true")
+$1 = 0
+(@value{GDBP}) print $_shell("false")
+$2 = 1
+(@value{GDBP}) p $_shell("echo hello")
+hello
+$3 = 0
+(@value{GDBP}) p $_shell("foobar")
+bash: line 1: foobar: command not found
+$4 = 127
+@end smallexample
+
+This may also be useful in breakpoint conditions.  For example:
+
+@smallexample
+(@value{GDBP}) break function if $_shell("some command") == 0
+@end smallexample
+
+In this scenario, you'll want to make sure that the shell command you
+run in the breakpoint condition takes the least amount of time
+possible.  This is important to minimize the time it takes to evaluate
+the condition and re-resume the program if the condition turns out to
+be false.
+
+Note: unlike the @code{shell} command, the @code{$_shell} convenience
+function does not affect the @code{$_shell_exitcode} and
+@code{$_shell_exitsignal} convenience variables.
+
 @end table
 
 The following functions require @value{GDBN} to be configured with
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index d0789a64401..7e73db0576a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,7 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_shell = <internal function _shell>} \
 	{$_gdb_maint_setting_str = <internal function _gdb_maint_setting_str>} \
 	{$_gdb_maint_setting = <internal function _gdb_maint_setting>} \
 	{$_gdb_setting_str = <internal function _gdb_setting_str>} \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 31cdcb41af5..ba1691ea2b0 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -41,6 +41,42 @@ if { ! [ishost *-*-mingw*] } {
     gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
 }
 
+# Test the $_shell convenience function.
+
+with_test_prefix "\$_shell convenience function" {
+    # Simple commands, check the result code.
+    gdb_test "p \$_shell(\"true\")" " = 0"
+    gdb_test "p \$_shell(\"false\")" " = 1"
+
+    # Test command with arguments.
+    gdb_test "p \$_shell(\"echo foo\")" "foo\r\n\\$${decimal} = 0"
+
+    # Check the type of the result.
+    gdb_test "ptype \$_shell(\"true\")" "type = int"
+
+    # Test passing a non-literal string as command name.
+    gdb_test "p \$cmd = \"echo bar\"" " = \"echo bar\""
+    gdb_test "p \$_shell(\$cmd)" "bar\r\n\\$${decimal} = 0"
+
+    # Test executing a non-existing command.  The result is
+    # shell-dependent, but most (all?) POSIX-like shells return 127 in
+    # this case.
+    gdb_test "p \$_shell(\"non-existing-command-foo-bar-qux\")" " = 127"
+
+    gdb_test "p \$_shell" \
+	" = <internal function _shell>"
+    gdb_test "ptype \$_shell" \
+	"type = <internal function>"
+
+    # Test error scenarios.
+    gdb_test "p \$_shell()" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(\"a\", \"b\")" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(1)" \
+	"Argument must be a string\\\."
+}
+
 # Define the user command "foo", used to test "pipe" command.
 gdb_test_multiple "define foo" "define foo" {
     -re "End with"  {
-- 
2.36.0

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Eli Zaretskii]

> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 10 Feb 2023 23:36:01 +0000
> 
>  gdb/NEWS                           | 10 ++++
>  gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
>  gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
>  gdb/testsuite/gdb.base/default.exp |  1 +
>  gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
>  5 files changed, 179 insertions(+), 4 deletions(-)

Thanks.

> +* New convenience function "$_shell", to execute a shell command and
> +  return the result.  This lets you run shell commands in expressions.
> +  Some examples:
> +
> +   (gdb) p $_shell("true")
> +   $1 = 0
> +   (gdb) p $_shell("false")
> +   $2 = 1
> +   (gdb) break func if $_shell("some command") == 0
> +
>  * MI changes

This part is OK.

> -static void
> -shell_escape (const char *arg, int from_tty)
> +/* Run ARG under the shell, and return the exit status.  If ARG is
> +   NULL, run an interactive shell.  */
> +
> +static int
> +run_under_shell (const char *arg, int from_tty)
>  {
>  #if defined(CANT_FORK) || \
>        (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
> @@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
>       the shell command we just ran changed it.  */
>    chdir (current_directory);
>  #endif
> -  exit_status_set_internal_vars (rc);
> +  return rc;
>  #else /* Can fork.  */
>    int status, pid;
>  
> @@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
>      waitpid (pid, &status, 0);
>    else
>      error (_("Fork failed"));
> -  exit_status_set_internal_vars (status);
> +  return status;
>  #endif /* Can fork.  */
>  }

This part seems to leave in place the error message in case invoking
the shell command failed (i.e. 'system' or 'execl' failed).  I wonder
whether we should still show that error message if this was called via
the new built-in function.  Perhaps it would be better to instead
return a distinct return value?  Having these messages emitted during
breakpoint commands, for example, disrupts the "normal" display of
breakpoint data and output from breakpoint commands, so perhaps it is
better to leave the handling of this situation to the caller?

And another comment/question: on Posix hosts this calls fork/exec, so
if GDB was configured to follow forks, does it mean it might start
debugging the program invoked via the shell function? if so, what does
that mean for using that function in breakpoint commands?  If there is
some subtleties to be aware of here, I think at least the manual
should mention them.

> +  add_internal_function ("_shell", _("\
> +$_shell - execute a shell command and return the result.\n\
> +Usage: $_shell (command)\n\
> +Returns the command's exit code: zero on success, non-zero otherwise."),
   ^^^^^^^
I think our usual style is to say "Return", not "Returns".

> +@anchor{$_shell convenience function}
> +@item $_shell (@var{command-string})
> +@findex $_shell@r{, convenience function}

@fined should be _before_ @item, so that the index records the
position of @item, and the Info reader places you before the @item
when you use index-search.

> +Invoke a standard shell to execute @var{command-string}.  Returns the
          ^^^^^^^^^^^^^^^^
"the standard system shell" is better, I think.

> +command's exit status.  On Unix systems, a command which exits with a
> +zero exit status has succeeded, and non-zero exit status indicates
> +failure.  When a command terminates on a fatal signal whose number is
> +N, @value{GDBN} uses the value 128+N as the exit status, as is
> +standard in Unix shells.

"N" should be "@var{n}" here.

Also, this text could benefit from a cross-reference to where we
describe the commands that convert from signal numbers to their
mnemonics, since that is system-dependent.  Maybe "info signal N" is
the right tool here?  If so, a cross-reference to "Signals" is what we
should have here.  (Is there a better way of asking GDB about signal
number N?)

> +In this scenario, you'll want to make sure that the shell command you
> +run in the breakpoint condition takes the least amount of time
> +possible.  This is important to minimize the time it takes to evaluate
> +the condition and re-resume the program if the condition turns out to
> +be false.

I understand what this says, but not what it means in practice.
Perhaps one or two additional sentences to help the reader understand
how to "make sure the shell command in a breakpoint condition takes
the least amount of time" would be in order?

Reviewed-by: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Pedro Alves]

Hi,

On 2023-02-11 8:02 a.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>> Date: Fri, 10 Feb 2023 23:36:01 +0000
>>
>>  gdb/NEWS                           | 10 ++++
>>  gdb/cli/cli-cmds.c                 | 89 ++++++++++++++++++++++++++++--
>>  gdb/doc/gdb.texinfo                | 47 ++++++++++++++++
>>  gdb/testsuite/gdb.base/default.exp |  1 +
>>  gdb/testsuite/gdb.base/shell.exp   | 36 ++++++++++++
>>  5 files changed, 179 insertions(+), 4 deletions(-)
> 
> Thanks.
> 
>> +* New convenience function "$_shell", to execute a shell command and
>> +  return the result.  This lets you run shell commands in expressions.
>> +  Some examples:
>> +
>> +   (gdb) p $_shell("true")
>> +   $1 = 0
>> +   (gdb) p $_shell("false")
>> +   $2 = 1
>> +   (gdb) break func if $_shell("some command") == 0
>> +
>>  * MI changes
> 
> This part is OK.
> 
>> -static void
>> -shell_escape (const char *arg, int from_tty)
>> +/* Run ARG under the shell, and return the exit status.  If ARG is
>> +   NULL, run an interactive shell.  */
>> +
>> +static int
>> +run_under_shell (const char *arg, int from_tty)
>>  {
>>  #if defined(CANT_FORK) || \
>>        (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
>> @@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
>>       the shell command we just ran changed it.  */
>>    chdir (current_directory);
>>  #endif
>> -  exit_status_set_internal_vars (rc);
>> +  return rc;
>>  #else /* Can fork.  */
>>    int status, pid;
>>  
>> @@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
>>      waitpid (pid, &status, 0);
>>    else
>>      error (_("Fork failed"));
>> -  exit_status_set_internal_vars (status);
>> +  return status;
>>  #endif /* Can fork.  */
>>  }
> 
> This part seems to leave in place the error message in case invoking
> the shell command failed (i.e. 'system' or 'execl' failed).  

Actually, if 'system' failed, we return "rc", not throw an error.
Similarly, if 'execl' fails, the child calls _exit(0177), which is reported to
the parent (gdb) via waitpid, and so the function ends up returning the status.

The only scenario we throw an error is if fork fails, which is really a
catastrophic failure indicating GDB is out of resources, and not a problem
with the command the user wanted to run.

For example:

 (gdb) p $_shell ("doesnt exit")
 bash: line 1: doesnt: command not found
 $1 = 127

Note we still printed the $1 result, not aborted with an error.  The printout
comes from bash.

Here's what the patched function looks like:

static int
run_under_shell (const char *arg, int from_tty)
{
#if defined(CANT_FORK) || \
      (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
  /* If ARG is NULL, they want an inferior shell, but `system' just
     reports if the shell is available when passed a NULL arg.  */
  int rc = system (arg ? arg : "");

  if (!arg)
    arg = "inferior shell";

  if (rc == -1)
    gdb_printf (gdb_stderr, "Cannot execute %s: %s\n", arg,
		safe_strerror (errno));
  else if (rc)
    gdb_printf (gdb_stderr, "%s exited with status %d\n", arg, rc);
#ifdef GLOBAL_CURDIR
  /* Make sure to return to the directory GDB thinks it is, in case
     the shell command we just ran changed it.  */
  chdir (current_directory);
#endif
  return rc;
#else /* Can fork.  */
  int status, pid;

  if ((pid = vfork ()) == 0)
    {
      const char *p, *user_shell = get_shell ();

      close_most_fds ();

      /* Get the name of the shell for arg0.  */
      p = lbasename (user_shell);

      if (!arg)
	execl (user_shell, p, (char *) 0);
      else
	execl (user_shell, p, "-c", arg, (char *) 0);

      gdb_printf (gdb_stderr, "Cannot execute %s: %s\n", user_shell,
		  safe_strerror (errno));
      _exit (0177);
    }

  if (pid != -1)
    waitpid (pid, &status, 0);
  else
    error (_("Fork failed"));
  return status;
#endif /* Can fork.  */
}

Note there's only on error call in the function.


> I wonder
> whether we should still show that error message if this was called via
> the new built-in function.  Perhaps it would be better to instead
> return a distinct return value?  Having these messages emitted during
> breakpoint commands, for example, disrupts the "normal" display of
> breakpoint data and output from breakpoint commands, so perhaps it is
> better to leave the handling of this situation to the caller?
> 
> And another comment/question: on Posix hosts this calls fork/exec, so
> if GDB was configured to follow forks, does it mean it might start
> debugging the program invoked via the shell function? if so, what does
> that mean for using that function in breakpoint commands?  If there is
> some subtleties to be aware of here, I think at least the manual
> should mention them.

It does not mean that (follow fork), because like with the "shell" command, it is
GDB that invokes the shell, not the inferior.  We follow forks when the
inferior itself forks, as we decide whether to continue debugging the parent,
or switch to debugging the child, or debugging both.  The inferior could even
be a remote inferior running under gdbserver or some other stub, and the
shell would run on the gdb side.

> 
>> +  add_internal_function ("_shell", _("\
>> +$_shell - execute a shell command and return the result.\n\
>> +Usage: $_shell (command)\n\
>> +Returns the command's exit code: zero on success, non-zero otherwise."),
>    ^^^^^^^
> I think our usual style is to say "Return", not "Returns".


Hmmm.  If I do "apropos returns", I get:

(gdb) apropos returns
finish, fin -- Execute until selected stack frame returns.
function _any_caller_is -- Check all calling function's names.
function _any_caller_matches -- Compare all calling function's names with a regexp.
function _as_string -- Return the string representation of a value.
function _caller_is -- Check the calling function's name.
function _caller_matches -- Compare the calling function's name with a regexp.
function _gdb_maint_setting -- $_gdb_maint_setting - returns the value of a GDB maintenance setting.
function _gdb_maint_setting_str -- $_gdb_maint_setting_str - returns the value of a GDB maintenance setting as a string.
function _gdb_setting -- $_gdb_setting - returns the value of a GDB setting.
function _gdb_setting_str -- $_gdb_setting_str - returns the value of a GDB setting as a string.
function _memeq -- $_memeq - compare bytes of memory.
function _regex -- $_regex - check if a string matches a regular expression.
function _shell -- $_shell - execute a shell command and return the result.
function _streq -- $_streq - check string equality.
function _strlen -- $_strlen - compute string length.

and looking at several of those builtin function's help, we see this style:

(gdb) help function _any_caller_is 
Check all calling function's names.

    Usage: $_any_caller_is (NAME [, NUMBER-OF-FRAMES])

    Arguments:

      NAME: The name of the function to search for.

      NUMBER-OF-FRAMES: How many stack frames to traverse back from the currently
        selected frame to compare with.  If the value is greater than the depth of
        the stack from that point then the result is False.
        The default is 1.

    Returns:
      True if any function's name is equal to NAME.


I guess I should instead use that style here too?  I've done that now, and here's
what we get:

(gdb) help function _shell
$_shell - execute a shell command and return the result.

    Usage: $_shell (COMMAND)

    Arguments:

      COMMAND: The command to execute.  Must be a string.

    Returns:
      The command's exit code: zero on success, non-zero otherwise.


WDYT?


> 
>> +@anchor{$_shell convenience function}
>> +@item $_shell (@var{command-string})
>> +@findex $_shell@r{, convenience function}
> 
> @fined should be _before_ @item, so that the index records the
> position of @item, and the Info reader places you before the @item
> when you use index-search.

But the whole "Convenience Funs" node has has item/findex sorted like that,
I just copied the preceding example.  Here's what we have:

 @item $_isvoid (@var{expr})
 @findex $_isvoid@r{, convenience function}
 ...
 @item $_gdb_setting_str (@var{setting})
 @findex $_gdb_setting_str@r{, convenience function}
 ...
 @item $_gdb_setting (@var{setting})
 @findex $_gdb_setting@r{, convenience function}
 ...
 @item $_gdb_maint_setting_str (@var{setting})
 @findex $_gdb_maint_setting_str@r{, convenience function}
 ...
 @item $_gdb_maint_setting (@var{setting})
 @findex $_gdb_maint_setting@r{, convenience function}

Are all these wrong?  Sounds like they should all be fixed in one go.
I don't usually use the info reader, so I'm not really sure what to look for.

> 
>> +Invoke a standard shell to execute @var{command-string}.  Returns the
>           ^^^^^^^^^^^^^^^^
> "the standard system shell" is better, I think.

Note this is the same wording used when describing the "shell" command:

 @table @code
 @kindex shell
 @kindex !
 @cindex shell escape
 @item shell @var{command-string}
 @itemx !@var{command-string}
 Invoke a standard shell to execute @var{command-string}.
 Note that no space is needed between @code{!} and @var{command-string}.
 On GNU and Unix systems, the environment variable @env{SHELL}, if it
 exists, determines which shell to run.  Otherwise @value{GDBN} uses
 the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table


Maybe the intention was to avoid "system" as @env{SHELL} may point
to some shell the user installed, which doesn't come with or from
the "system"?  So I'd prefer to use the same terminology in $_shell too.

> 
>> +command's exit status.  On Unix systems, a command which exits with a
>> +zero exit status has succeeded, and non-zero exit status indicates
>> +failure.  When a command terminates on a fatal signal whose number is
>> +N, @value{GDBN} uses the value 128+N as the exit status, as is
>> +standard in Unix shells.
> 
> "N" should be "@var{n}" here.

Thanks, done.

BTW, I forgot to mention, but I had borrowed that wording from the
Bash documentation:

  https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html

> Also, this text could benefit from a cross-reference to where we
> describe the commands that convert from signal numbers to their
> mnemonics, since that is system-dependent.  Maybe "info signal N" is
> the right tool here?  If so, a cross-reference to "Signals" is what we
> should have here.  (Is there a better way of asking GDB about signal
> number N?)

I don't think it's the right tool here, because "info signal" and
"handle", etc., are about target signals, signals that trigger in the
inferior, which GDB is able to intercept.  OTOH, the $_shell function
and the "shell" command run the shell on the host, so any fatal signal code
is a host signal number, and doesn't go via any translation at all.

I'll note that where we document $_shell_exitsignal, the convenience
variable that the "shell" command sets, we don't describe the host signals
either.

I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
running on." sentence earlier, in case that helps set expectations, and
added a couple sentences about host vs target signal numbers.  

I had also forgotten to document that the argument must be a string.

Here is the result:

"Invoke a standard shell to execute @var{command-string}.
@var{command-string} must be a string.  The shell runs on the host
machine, the machine @value{GDBN} is running on.  Returns the
command's exit status.  On Unix systems, a command which exits with a
zero exit status has succeeded, and non-zero exit status indicates
failure.  When a command terminates on a fatal signal whose number is
@var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
as is standard in Unix shells.  Note that @var{N} is a host signal
number, not a target signal number.  If you're cross debugging, the
host vs target signal numbers may be completely unrelated.  The shell
to run is determined in the same way as for the @code{shell} command.
@xref{Shell Commands, ,Shell Commands}."

WDYT?

> 
>> +In this scenario, you'll want to make sure that the shell command you
>> +run in the breakpoint condition takes the least amount of time
>> +possible.  This is important to minimize the time it takes to evaluate
>> +the condition and re-resume the program if the condition turns out to
>> +be false.
> 
> I understand what this says, but not what it means in practice.
> Perhaps one or two additional sentences to help the reader understand
> how to "make sure the shell command in a breakpoint condition takes
> the least amount of time" would be in order?

Maybe something like this?

"In this scenario, you'll want to make sure that the shell command you
run in the breakpoint condition takes the least amount of time
possible.  For example, avoid running a command that may block
indefinitely, or that sleeps for a while before exiting.  Prefer a
command or script which analyzes some state and exits immediately.
This is important because the debugged program stops for the
breakpoint every time, and then @value{GDBN} evaluates the breakpoint
condition.  If the condition is false, the program is re-resumed
transparently, without informing you of the stop.  A quick shell
command thus avoids significantly slowing down the debugged program
unnecessarily."

> 
> Reviewed-by: Eli Zaretskii <eliz@gnu.org>
> 

Thanks.  Here's the updated patch with the changes addressing the comments
above.  Let me know what you think.

From 1fc4d7e1fd145df7bca31c0630cb44fd8c85bbd4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 8 Feb 2023 16:06:23 +0000
Subject: [PATCH] Add new "$_shell(CMD)" internal function

For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 96 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 56 +++++++++++++++++
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 +++++++++++
 5 files changed, 195 insertions(+), 4 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b85923cf80d..0d3445438b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -56,6 +56,16 @@ maintenance info frame-unwinders
   List the frame unwinders currently in effect, starting with the highest
   priority.
 
+* New convenience function "$_shell", to execute a shell command and
+  return the result.  This lets you run shell commands in expressions.
+  Some examples:
+
+   (gdb) p $_shell("true")
+   $1 = 0
+   (gdb) p $_shell("false")
+   $2 = 1
+   (gdb) break func if $_shell("some command") == 0
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 6c0d780face..7607fe59b05 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/filestuff.h"
 #include "location.h"
 #include "block.h"
+#include "valprint.h"
 
 #include "ui-out.h"
 #include "interps.h"
@@ -873,6 +874,9 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
+
+  /* Keep the logic here in sync with shell_internal_fn.  */
+
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 #ifdef __MINGW32__
@@ -893,8 +897,11 @@ exit_status_set_internal_vars (int exit_status)
     warning (_("unexpected shell command exit status %d"), exit_status);
 }
 
-static void
-shell_escape (const char *arg, int from_tty)
+/* Run ARG under the shell, and return the exit status.  If ARG is
+   NULL, run an interactive shell.  */
+
+static int
+run_under_shell (const char *arg, int from_tty)
 {
 #if defined(CANT_FORK) || \
       (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
@@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
      the shell command we just ran changed it.  */
   chdir (current_directory);
 #endif
-  exit_status_set_internal_vars (rc);
+  return rc;
 #else /* Can fork.  */
   int status, pid;
 
@@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
-  exit_status_set_internal_vars (status);
+  return status;
 #endif /* Can fork.  */
 }
 
+/* Escape out to the shell to run ARG.  If ARG is NULL, launch and
+   interactive shell.  Sets $_shell_exitcode and $_shell_exitsignal
+   convenience variables based on the exits status.  */
+
+static void
+shell_escape (const char *arg, int from_tty)
+{
+  int status = run_under_shell (arg, from_tty);
+  exit_status_set_internal_vars (status);
+}
+
 /* Implementation of the "shell" command.  */
 
 static void
@@ -2417,6 +2435,63 @@ gdb_maint_setting_str_internal_fn (struct gdbarch *gdbarch,
   return str_value_from_setting (*show_cmd->var, gdbarch);
 }
 
+/* Implementation of the convenience function $_shell.  */
+
+static struct value *
+shell_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_shell."));
+
+  value *val = argv[0];
+  struct type *type = check_typedef (value_type (val));
+
+  if (!language->is_string_type_p (type))
+    error (_("Argument must be a string."));
+
+  value_print_options opts;
+  get_no_prettyformat_print_options (&opts);
+
+  string_file stream;
+  value_print (val, &stream, &opts);
+
+  /* We should always have two quote chars, which we'll strip.  */
+  gdb_assert (stream.size () >= 2);
+
+  /* Now strip them.  We don't need the original string, so it's
+     cheaper to do it in place, avoiding a string allocation.  */
+  std::string str = stream.release ();
+  str[str.size () - 1] = 0;
+  const char *command = str.c_str () + 1;
+
+  int exit_status = run_under_shell (command, 0);
+
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+
+  /* Keep the logic here in sync with
+     exit_status_set_internal_vars.  */
+
+  if (WIFEXITED (exit_status))
+    return value_from_longest (int_type, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* See exit_status_set_internal_vars.  */
+      return value_from_longest (int_type, exit_status);
+    }
+#endif
+  else if (WIFSIGNALED (exit_status))
+    {
+      /* (0x80 | SIGNO) is what most (all?) POSIX-like shells set as
+	 exit code on fatal signal termination.  */
+      return value_from_longest (int_type, 0x80 | WTERMSIG (exit_status));
+    }
+  else
+    return allocate_optimized_out_value (int_type);
+}
+
 void _initialize_cli_cmds ();
 void
 _initialize_cli_cmds ()
@@ -2606,6 +2681,19 @@ Some integer settings accept an unlimited value, returned\n\
 as 0 or -1 depending on the setting."),
 			 gdb_maint_setting_internal_fn, NULL);
 
+  add_internal_function ("_shell", _("\
+$_shell - execute a shell command and return the result.\n\
+\n\
+    Usage: $_shell (COMMAND)\n\
+\n\
+    Arguments:\n\
+\n\
+      COMMAND: The command to execute.  Must be a string.\n\
+\n\
+    Returns:\n\
+      The command's exit code: zero on success, non-zero otherwise."),
+			 shell_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7b128053b5a..310f857f09a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1629,6 +1629,10 @@ the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
+You may also invoke shell commands from expressions, using the
+@code{$_shell} convenience function.  @xref{$_shell convenience
+function}.
+
 The utility @code{make} is often needed in development environments.
 You do not have to use the @code{shell} command for this purpose in
 @value{GDBN}:
@@ -12969,6 +12973,58 @@ Like the @code{$_gdb_setting_str} function, but works with
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
+@anchor{$_shell convenience function}
+@item $_shell (@var{command-string})
+@findex $_shell@r{, convenience function}
+
+Invoke a standard shell to execute @var{command-string}.
+@var{command-string} must be a string.  The shell runs on the host
+machine, the machine @value{GDBN} is running on.  Returns the
+command's exit status.  On Unix systems, a command which exits with a
+zero exit status has succeeded, and non-zero exit status indicates
+failure.  When a command terminates on a fatal signal whose number is
+@var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
+as is standard in Unix shells.  Note that @var{N} is a host signal
+number, not a target signal number.  If you're cross debugging, the
+host vs target signal numbers may be completely unrelated.  The shell
+to run is determined in the same way as for the @code{shell} command.
+@xref{Shell Commands, ,Shell Commands}.
+
+@smallexample
+(@value{GDBP}) print $_shell("true")
+$1 = 0
+(@value{GDBP}) print $_shell("false")
+$2 = 1
+(@value{GDBP}) p $_shell("echo hello")
+hello
+$3 = 0
+(@value{GDBP}) p $_shell("foobar")
+bash: line 1: foobar: command not found
+$4 = 127
+@end smallexample
+
+This may also be useful in breakpoint conditions.  For example:
+
+@smallexample
+(@value{GDBP}) break function if $_shell("some command") == 0
+@end smallexample
+
+In this scenario, you'll want to make sure that the shell command you
+run in the breakpoint condition takes the least amount of time
+possible.  For example, avoid running a command that may block
+indefinitely, or that sleeps for a while before exiting.  Prefer a
+command or script which analyzes some state and exits immediately.
+This is important because the debugged program stops for the
+breakpoint every time, and then @value{GDBN} evaluates the breakpoint
+condition.  If the condition is false, the program is re-resumed
+transparently, without informing you of the stop.  A quick shell
+command thus avoids significantly slowing down the debugged program
+unnecessarily.
+
+Note: unlike the @code{shell} command, the @code{$_shell} convenience
+function does not affect the @code{$_shell_exitcode} and
+@code{$_shell_exitsignal} convenience variables.
+
 @end table
 
 The following functions require @value{GDBN} to be configured with
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index d0789a64401..7e73db0576a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,7 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_shell = <internal function _shell>} \
 	{$_gdb_maint_setting_str = <internal function _gdb_maint_setting_str>} \
 	{$_gdb_maint_setting = <internal function _gdb_maint_setting>} \
 	{$_gdb_setting_str = <internal function _gdb_setting_str>} \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 31cdcb41af5..ba1691ea2b0 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -41,6 +41,42 @@ if { ! [ishost *-*-mingw*] } {
     gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
 }
 
+# Test the $_shell convenience function.
+
+with_test_prefix "\$_shell convenience function" {
+    # Simple commands, check the result code.
+    gdb_test "p \$_shell(\"true\")" " = 0"
+    gdb_test "p \$_shell(\"false\")" " = 1"
+
+    # Test command with arguments.
+    gdb_test "p \$_shell(\"echo foo\")" "foo\r\n\\$${decimal} = 0"
+
+    # Check the type of the result.
+    gdb_test "ptype \$_shell(\"true\")" "type = int"
+
+    # Test passing a non-literal string as command name.
+    gdb_test "p \$cmd = \"echo bar\"" " = \"echo bar\""
+    gdb_test "p \$_shell(\$cmd)" "bar\r\n\\$${decimal} = 0"
+
+    # Test executing a non-existing command.  The result is
+    # shell-dependent, but most (all?) POSIX-like shells return 127 in
+    # this case.
+    gdb_test "p \$_shell(\"non-existing-command-foo-bar-qux\")" " = 127"
+
+    gdb_test "p \$_shell" \
+	" = <internal function _shell>"
+    gdb_test "ptype \$_shell" \
+	"type = <internal function>"
+
+    # Test error scenarios.
+    gdb_test "p \$_shell()" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(\"a\", \"b\")" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(1)" \
+	"Argument must be a string\\\."
+}
+
 # Define the user command "foo", used to test "pipe" command.
 gdb_test_multiple "define foo" "define foo" {
     -re "End with"  {

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
prerequisite-patch-id: 36aeba89f6bb6550d7bdf756dbd3c265f2d95d58
prerequisite-patch-id: cc6142da7c38349c0dfe95325b5b501998ba7e67
-- 
2.36.0

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 13 Feb 2023 15:11:45 +0000
> 
> I guess I should instead use that style here too?  I've done that now, and here's
> what we get:
> 
> (gdb) help function _shell
> $_shell - execute a shell command and return the result.
> 
>     Usage: $_shell (COMMAND)
> 
>     Arguments:
> 
>       COMMAND: The command to execute.  Must be a string.
> 
>     Returns:
>       The command's exit code: zero on success, non-zero otherwise.
> 
> 
> WDYT?

OK.

> >> +@anchor{$_shell convenience function}
> >> +@item $_shell (@var{command-string})
> >> +@findex $_shell@r{, convenience function}
> > 
> > @fined should be _before_ @item, so that the index records the
> > position of @item, and the Info reader places you before the @item
> > when you use index-search.
> 
> But the whole "Convenience Funs" node has has item/findex sorted like that,
> I just copied the preceding example.  Here's what we have:
> 
>  @item $_isvoid (@var{expr})
>  @findex $_isvoid@r{, convenience function}
>  ...
>  @item $_gdb_setting_str (@var{setting})
>  @findex $_gdb_setting_str@r{, convenience function}
>  ...
>  @item $_gdb_setting (@var{setting})
>  @findex $_gdb_setting@r{, convenience function}
>  ...
>  @item $_gdb_maint_setting_str (@var{setting})
>  @findex $_gdb_maint_setting_str@r{, convenience function}
>  ...
>  @item $_gdb_maint_setting (@var{setting})
>  @findex $_gdb_maint_setting@r{, convenience function}
> 
> Are all these wrong?  Sounds like they should all be fixed in one go.

Yes, they are all wrong.  The result is not a catastrophe, it's just
sub-optimal.

> I don't usually use the info reader, so I'm not really sure what to look for.

In an Info reader, type "i NAME" where NAME is the name of a function,
and see where it lands you.

> >> +Invoke a standard shell to execute @var{command-string}.  Returns the
> >           ^^^^^^^^^^^^^^^^
> > "the standard system shell" is better, I think.
> 
> Note this is the same wording used when describing the "shell" command:
> 
>  @table @code
>  @kindex shell
>  @kindex !
>  @cindex shell escape
>  @item shell @var{command-string}
>  @itemx !@var{command-string}
>  Invoke a standard shell to execute @var{command-string}.
>  Note that no space is needed between @code{!} and @var{command-string}.
>  On GNU and Unix systems, the environment variable @env{SHELL}, if it
>  exists, determines which shell to run.  Otherwise @value{GDBN} uses
>  the default shell (@file{/bin/sh} on GNU and Unix systems,
>  @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
>  @end table
> 
> 
> Maybe the intention was to avoid "system" as @env{SHELL} may point
> to some shell the user installed, which doesn't come with or from
> the "system"?  So I'd prefer to use the same terminology in $_shell too.

I thought 'system' is not affected by $SHELL, but if it is, then
something like "Invoke a shell to execute @var{command-string}", I
think.  The explanation of how $SHELL affects the shell describes the
details.

> > Also, this text could benefit from a cross-reference to where we
> > describe the commands that convert from signal numbers to their
> > mnemonics, since that is system-dependent.  Maybe "info signal N" is
> > the right tool here?  If so, a cross-reference to "Signals" is what we
> > should have here.  (Is there a better way of asking GDB about signal
> > number N?)
> 
> I don't think it's the right tool here, because "info signal" and
> "handle", etc., are about target signals, signals that trigger in the
> inferior, which GDB is able to intercept.  OTOH, the $_shell function
> and the "shell" command run the shell on the host, so any fatal signal code
> is a host signal number, and doesn't go via any translation at all.

For native debugging, they are the same, of course.  So mentioning
that with the caveat of native debugging will help in some cases.

> I'll note that where we document $_shell_exitsignal, the convenience
> variable that the "shell" command sets, we don't describe the host signals
> either.

Then maybe we should have some text to tell the user how to map
numbers to signal mnemonics in the non-native case.

> I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
> running on." sentence earlier, in case that helps set expectations, and
> added a couple sentences about host vs target signal numbers.  
> 
> I had also forgotten to document that the argument must be a string.
> 
> Here is the result:
> 
> "Invoke a standard shell to execute @var{command-string}.
> @var{command-string} must be a string.  The shell runs on the host
> machine, the machine @value{GDBN} is running on.  Returns the
> command's exit status.  On Unix systems, a command which exits with a
> zero exit status has succeeded, and non-zero exit status indicates
> failure.  When a command terminates on a fatal signal whose number is
> @var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
> as is standard in Unix shells.  Note that @var{N} is a host signal
> number, not a target signal number.  If you're cross debugging, the
> host vs target signal numbers may be completely unrelated.  The shell
> to run is determined in the same way as for the @code{shell} command.
> @xref{Shell Commands, ,Shell Commands}."
> 
> WDYT?

OK, with the above nits.

> >> +In this scenario, you'll want to make sure that the shell command you
> >> +run in the breakpoint condition takes the least amount of time
> >> +possible.  This is important to minimize the time it takes to evaluate
> >> +the condition and re-resume the program if the condition turns out to
> >> +be false.
> > 
> > I understand what this says, but not what it means in practice.
> > Perhaps one or two additional sentences to help the reader understand
> > how to "make sure the shell command in a breakpoint condition takes
> > the least amount of time" would be in order?
> 
> Maybe something like this?
> 
> "In this scenario, you'll want to make sure that the shell command you
> run in the breakpoint condition takes the least amount of time
> possible.  For example, avoid running a command that may block
> indefinitely, or that sleeps for a while before exiting.  Prefer a
> command or script which analyzes some state and exits immediately.
> This is important because the debugged program stops for the
> breakpoint every time, and then @value{GDBN} evaluates the breakpoint
> condition.  If the condition is false, the program is re-resumed
> transparently, without informing you of the stop.  A quick shell
> command thus avoids significantly slowing down the debugged program
> unnecessarily."

Thanks, this is much more helpful.

[PATCH] gdb/manual: Move @findex entries (was: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function)

[Pedro Alves]

On 2023-02-13 3:36 p.m., Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>
>>>> +@anchor{$_shell convenience function}
>>>> +@item $_shell (@var{command-string})
>>>> +@findex $_shell@r{, convenience function}
>>>
>>> @fined should be _before_ @item, so that the index records the
>>> position of @item, and the Info reader places you before the @item
>>> when you use index-search.
>>
>> But the whole "Convenience Funs" node has has item/findex sorted like that,
>> I just copied the preceding example.  Here's what we have:
>>
>>  @item $_isvoid (@var{expr})
>>  @findex $_isvoid@r{, convenience function}
>>  ...
>>  @item $_gdb_setting_str (@var{setting})
>>  @findex $_gdb_setting_str@r{, convenience function}
>>  ...
>>  @item $_gdb_setting (@var{setting})
>>  @findex $_gdb_setting@r{, convenience function}
>>  ...
>>  @item $_gdb_maint_setting_str (@var{setting})
>>  @findex $_gdb_maint_setting_str@r{, convenience function}
>>  ...
>>  @item $_gdb_maint_setting (@var{setting})
>>  @findex $_gdb_maint_setting@r{, convenience function}
>>
>> Are all these wrong?  Sounds like they should all be fixed in one go.
> 
> Yes, they are all wrong.  The result is not a catastrophe, it's just
> sub-optimal.
> 
>> I don't usually use the info reader, so I'm not really sure what to look for.
> 
> In an Info reader, type "i NAME" where NAME is the name of a function,
> and see where it lands you.

Thanks, I see now.

Here's a patch fixing that throughout the manual.  Turns out that most if not
all @findex calls in gdb.texinfo needed to be fixed.

WDYT?

From: Pedro Alves <pedro@palves.net>
Subject: [PATCH] gdb/manual: Move @findex entries

The manual currently has many cases like these:

 @item $_gdb_setting_str (@var{setting})
 @findex $_gdb_setting_str@r{, convenience function}

As suggested by Eli, move the @findex entries before @item so that the
index records the position of @item, and the Info reader places you
there when you use index-search.

I went over all @findex calls in the manual, and most are like the
above.  Most either appear before @item, or before @subheading, like:

 @subheading The @code{-break-after} Command
 @findex -break-after

I fixed all of them.

There are findex entries in annotate.texinfo,python.texi, and
stabs.texinfo as well, though those all look right to me already.

Tested by typing "i _isvoid" (@item case) and "i -complete"
(@subheading case) in an Info reader, and checking where those took
me.

Change-Id: Idb6903b0bb39ff03f93524628dcef86b5585c97e
Suggested-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/doc/gdb.texinfo | 330 ++++++++++++++++++++++----------------------
 1 file changed, 165 insertions(+), 165 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7b128053b5a..79cb06e496e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -12824,8 +12824,8 @@ These functions do not require @value{GDBN} to be configured with
 
 @table @code
 
-@item $_isvoid (@var{expr})
 @findex $_isvoid@r{, convenience function}
+@item $_isvoid (@var{expr})
 Return one if the expression @var{expr} is @code{void}.  Otherwise it
 returns zero.
 
@@ -12880,8 +12880,8 @@ $3 = void
 $4 = 1
 @end smallexample
 
-@item $_gdb_setting_str (@var{setting})
 @findex $_gdb_setting_str@r{, convenience function}
+@item $_gdb_setting_str (@var{setting})
 Return the value of the @value{GDBN} @var{setting} as a string.
 @var{setting} is any setting that can be used in a @code{set} or
 @code{show} command (@pxref{Controlling GDB}).
@@ -12896,8 +12896,8 @@ $2 = "30"
 (@value{GDBP})
 @end smallexample
 
-@item $_gdb_setting (@var{setting})
 @findex $_gdb_setting@r{, convenience function}
+@item $_gdb_setting (@var{setting})
 Return the value of the @value{GDBN} @var{setting}.
 The type of the returned value depends on the setting.
 
@@ -12959,13 +12959,13 @@ Other setting types (enum, filename, optional filename, string, string noescape)
 are returned as string values.
 
 
-@item $_gdb_maint_setting_str (@var{setting})
 @findex $_gdb_maint_setting_str@r{, convenience function}
+@item $_gdb_maint_setting_str (@var{setting})
 Like the @code{$_gdb_setting_str} function, but works with
 @code{maintenance set} variables.
 
-@item $_gdb_maint_setting (@var{setting})
 @findex $_gdb_maint_setting@r{, convenience function}
+@item $_gdb_maint_setting (@var{setting})
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
@@ -12976,30 +12976,30 @@ The following functions require @value{GDBN} to be configured with
 
 @table @code
 
-@item $_memeq(@var{buf1}, @var{buf2}, @var{length})
 @findex $_memeq@r{, convenience function}
+@item $_memeq(@var{buf1}, @var{buf2}, @var{length})
 Returns one if the @var{length} bytes at the addresses given by
 @var{buf1} and @var{buf2} are equal.
 Otherwise it returns zero.
 
-@item $_regex(@var{str}, @var{regex})
 @findex $_regex@r{, convenience function}
+@item $_regex(@var{str}, @var{regex})
 Returns one if the string @var{str} matches the regular expression
 @var{regex}.  Otherwise it returns zero.
 The syntax of the regular expression is that specified by @code{Python}'s
 regular expression support.
 
-@item $_streq(@var{str1}, @var{str2})
 @findex $_streq@r{, convenience function}
+@item $_streq(@var{str1}, @var{str2})
 Returns one if the strings @var{str1} and @var{str2} are equal.
 Otherwise it returns zero.
 
-@item $_strlen(@var{str})
 @findex $_strlen@r{, convenience function}
+@item $_strlen(@var{str})
 Returns the length of string @var{str}.
 
-@item $_caller_is(@var{name}@r{[}, @var{number_of_frames}@r{]})
 @findex $_caller_is@r{, convenience function}
+@item $_caller_is(@var{name}@r{[}, @var{number_of_frames}@r{]})
 Returns one if the calling function's name is equal to @var{name}.
 Otherwise it returns zero.
 
@@ -13025,8 +13025,8 @@ $1 = 1
 $1 = 1
 @end smallexample
 
-@item $_caller_matches(@var{regexp}@r{[}, @var{number_of_frames}@r{]})
 @findex $_caller_matches@r{, convenience function}
+@item $_caller_matches(@var{regexp}@r{[}, @var{number_of_frames}@r{]})
 Returns one if the calling function's name matches the regular expression
 @var{regexp}.  Otherwise it returns zero.
 
@@ -13034,8 +13034,8 @@ If the optional argument @var{number_of_frames} is provided,
 it is the number of frames up in the stack to look.
 The default is 1.
 
-@item $_any_caller_is(@var{name}@r{[}, @var{number_of_frames}@r{]})
 @findex $_any_caller_is@r{, convenience function}
+@item $_any_caller_is(@var{name}@r{[}, @var{number_of_frames}@r{]})
 Returns one if any calling function's name is equal to @var{name}.
 Otherwise it returns zero.
 
@@ -13048,8 +13048,8 @@ checks all stack frames from the immediate caller to the frame specified
 by @var{number_of_frames}, whereas @code{$_caller_is} only checks the
 frame specified by @var{number_of_frames}.
 
-@item $_any_caller_matches(@var{regexp}@r{[}, @var{number_of_frames}@r{]})
 @findex $_any_caller_matches@r{, convenience function}
+@item $_any_caller_matches(@var{regexp}@r{[}, @var{number_of_frames}@r{]})
 Returns one if any calling function's name matches the regular expression
 @var{regexp}.  Otherwise it returns zero.
 
@@ -13062,8 +13062,8 @@ checks all stack frames from the immediate caller to the frame specified
 by @var{number_of_frames}, whereas @code{$_caller_matches} only checks the
 frame specified by @var{number_of_frames}.
 
-@item $_as_string(@var{value})
 @findex $_as_string@r{, convenience function}
+@item $_as_string(@var{value})
 Return the string representation of @var{value}.
 
 This function is useful to obtain the textual label (enumerator) of an
@@ -13075,10 +13075,10 @@ an enumerated type:
 Visiting node of type NODE_INTEGER
 @end smallexample
 
-@item $_cimag(@var{value})
-@itemx $_creal(@var{value})
 @findex $_cimag@r{, convenience function}
 @findex $_creal@r{, convenience function}
+@item $_cimag(@var{value})
+@itemx $_creal(@var{value})
 Return the imaginary (@code{$_cimag}) or real (@code{$_creal}) part of
 the complex number @var{value}.
 
@@ -24260,15 +24260,15 @@ The debugging stub for your architecture supplies these three
 subroutines:
 
 @table @code
-@item set_debug_traps
 @findex set_debug_traps
+@item set_debug_traps
 @cindex remote serial stub, initialization
 This routine arranges for @code{handle_exception} to run when your
 program stops.  You must call this subroutine explicitly in your
 program's startup code.
 
-@item handle_exception
 @findex handle_exception
+@item handle_exception
 @cindex remote serial stub, main routine
 This is the central workhorse, but your program never calls it
 explicitly---the setup code arranges for @code{handle_exception} to
@@ -24315,14 +24315,14 @@ First of all you need to tell the stub how to communicate with the
 serial port.
 
 @table @code
-@item int getDebugChar()
 @findex getDebugChar
+@item int getDebugChar()
 Write this subroutine to read a single character from the serial port.
 It may be identical to @code{getchar} for your target system; a
 different name is used to allow you to distinguish the two if you wish.
 
-@item void putDebugChar(int)
 @findex putDebugChar
+@item void putDebugChar(int)
 Write this subroutine to write a single character to the serial port.
 It may be identical to @code{putchar} for your target system; a
 different name is used to allow you to distinguish the two if you wish.
@@ -24344,8 +24344,8 @@ is to just execute a breakpoint instruction (the ``dirty'' part is that
 Other routines you need to supply are:
 
 @table @code
-@item void exceptionHandler (int @var{exception_number}, void *@var{exception_address})
 @findex exceptionHandler
+@item void exceptionHandler (int @var{exception_number}, void *@var{exception_address})
 Write this function to install @var{exception_address} in the exception
 handling tables.  You need to do this because the stub does not have any
 way of knowing what the exception handling tables on your target system
@@ -24366,8 +24366,8 @@ should be at privilege level 0 (the most privileged level).  The
 @sc{sparc} and 68k stubs are able to mask interrupts themselves without
 help from @code{exceptionHandler}.
 
-@item void flush_i_cache()
 @findex flush_i_cache
+@item void flush_i_cache()
 On @sc{sparc} and @sc{sparclite} only, write this subroutine to flush the
 instruction cache, if any, on your target machine.  If there is no
 instruction cache, this subroutine may be a no-op.
@@ -24380,8 +24380,8 @@ function to make certain that the state of your program is stable.
 You must also make sure this library routine is available:
 
 @table @code
-@item void *memset(void *, int, int)
 @findex memset
+@item void *memset(void *, int, int)
 This is the standard library function @code{memset} that sets an area of
 memory to a known value.  If you have one of the free versions of
 @code{libc.a}, @code{memset} can be found there; otherwise, you must
@@ -31068,8 +31068,8 @@ In addition to a number of out-of-band notifications, the response to a
 The synchronous operation was successful, @code{@var{results}} are the return
 values.
 
-@item "^running"
 @findex ^running
+@item "^running"
 This result record is equivalent to @samp{^done}.  Historically, it
 was output instead of @samp{^done} if the command has resumed the
 target.  This behaviour is maintained for backward compatibility, but
@@ -31077,12 +31077,12 @@ all frontends should treat @samp{^done} and @samp{^running}
 identically and rely on the @samp{*running} output record to determine
 which threads are resumed.
 
-@item "^connected"
 @findex ^connected
+@item "^connected"
 @value{GDBN} has connected to a remote target.
 
-@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 @findex ^error
+@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 The operation failed.  The @code{msg=@var{c-string}} variable contains
 the corresponding error message.
 
@@ -31095,8 +31095,8 @@ error condition.  At present, only one error code is defined:
 Indicates that the command causing the error does not exist.
 @end table
 
-@item "^exit"
 @findex ^exit
+@item "^exit"
 @value{GDBN} has terminated.
 
 @end table
@@ -31752,8 +31752,8 @@ not been implemented yet and these are labeled N.A.@: (not available).
 This section documents @sc{gdb/mi} commands for manipulating
 breakpoints.
 
-@subheading The @code{-break-after} Command
 @findex -break-after
+@subheading The @code{-break-after} Command
 
 @subsubheading Synopsis
 
@@ -31799,12 +31799,12 @@ line="5",thread-groups=["i1"],times="0",ignore="3"@}]@}
 @end smallexample
 
 @ignore
-@subheading The @code{-break-catch} Command
 @findex -break-catch
+@subheading The @code{-break-catch} Command
 @end ignore
 
-@subheading The @code{-break-commands} Command
 @findex -break-commands
+@subheading The @code{-break-commands} Command
 
 @subsubheading Synopsis
 
@@ -31838,8 +31838,8 @@ times="0"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-condition} Command
 @findex -break-condition
+@subheading The @code{-break-condition} Command
 
 @subsubheading Synopsis
 
@@ -31880,8 +31880,8 @@ line="5",cond="1",thread-groups=["i1"],times="0",ignore="3"@}]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-delete} Command
 @findex -break-delete
+@subheading The @code{-break-delete} Command
 
 @subsubheading Synopsis
 
@@ -31915,8 +31915,8 @@ body=[]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-disable} Command
 @findex -break-disable
+@subheading The @code{-break-disable} Command
 
 @subsubheading Synopsis
 
@@ -31952,8 +31952,8 @@ line="5",thread-groups=["i1"],times="0"@}]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-enable} Command
 @findex -break-enable
+@subheading The @code{-break-enable} Command
 
 @subsubheading Synopsis
 
@@ -31988,8 +31988,8 @@ line="5",thread-groups=["i1"],times="0"@}]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-info} Command
 @findex -break-info
+@subheading The @code{-break-info} Command
 
 @subsubheading Synopsis
 
@@ -32011,9 +32011,9 @@ The corresponding @value{GDBN} command is @samp{info break @var{breakpoint}}.
 @subsubheading Example
 N.A.
 
-@subheading The @code{-break-insert} Command
 @findex -break-insert
 @anchor{-break-insert}
+@subheading The @code{-break-insert} Command
 
 @subsubheading Synopsis
 
@@ -32134,8 +32134,8 @@ times="0"@}]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-dprintf-insert} Command
 @findex -dprintf-insert
+@subheading The @code{-dprintf-insert} Command
 
 @subsubheading Synopsis
 
@@ -32208,8 +32208,8 @@ original-location="mi-dprintf.c:26"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-list} Command
 @findex -break-list
+@subheading The @code{-break-list} Command
 
 @subsubheading Synopsis
 
@@ -32284,8 +32284,8 @@ body=[]@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-break-passcount} Command
 @findex -break-passcount
+@subheading The @code{-break-passcount} Command
 
 @subsubheading Synopsis
 
@@ -32298,8 +32298,8 @@ Set the passcount for tracepoint @var{tracepoint-number} to
 is not a tracepoint, error is emitted.  This corresponds to CLI
 command @samp{passcount}.
 
-@subheading The @code{-break-watch} Command
 @findex -break-watch
+@subheading The @code{-break-watch} Command
 
 @subsubheading Synopsis
 
@@ -32467,8 +32467,8 @@ catchpoints.
 @node Shared Library GDB/MI Catchpoint Commands
 @subsection Shared Library @sc{gdb/mi} Catchpoints
 
-@subheading The @code{-catch-load} Command
 @findex -catch-load
+@subheading The @code{-catch-load} Command
 
 @subsubheading Synopsis
 
@@ -32497,8 +32497,8 @@ what="load of library matching foo.so",catch-type="load",times="0"@}
 @end smallexample
 
 
-@subheading The @code{-catch-unload} Command
 @findex -catch-unload
+@subheading The @code{-catch-unload} Command
 
 @subsubheading Synopsis
 
@@ -32531,8 +32531,8 @@ what="load of library matching bar.so",catch-type="unload",times="0"@}
 The following @sc{gdb/mi} commands can be used to create catchpoints
 that stop the execution when Ada exceptions are being raised.
 
-@subheading The @code{-catch-assert} Command
 @findex -catch-assert
+@subheading The @code{-catch-assert} Command
 
 @subsubheading Synopsis
 
@@ -32568,8 +32568,8 @@ original-location="__gnat_debug_raise_assert_failure"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-catch-exception} Command
 @findex -catch-exception
+@subheading The @code{-catch-exception} Command
 
 @subsubheading Synopsis
 
@@ -32617,8 +32617,8 @@ times="0",original-location="__gnat_debug_raise_exception"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-catch-handlers} Command
 @findex -catch-handlers
+@subheading The @code{-catch-handlers} Command
 
 @subsubheading Synopsis
 
@@ -32668,8 +32668,8 @@ The following @sc{gdb/mi} commands can be used to create catchpoints
 that stop the execution when C@t{++} exceptions are being throw, rethrown,
 or caught.
 
-@subheading The @code{-catch-throw} Command
 @findex -catch-throw
+@subheading The @code{-catch-throw} Command
 
 @subsubheading Synopsis
 
@@ -32712,8 +32712,8 @@ and @samp{tcatch throw} (@pxref{Set Catchpoints}).
 (gdb)
 @end smallexample
 
-@subheading The @code{-catch-rethrow} Command
 @findex -catch-rethrow
+@subheading The @code{-catch-rethrow} Command
 
 @subsubheading Synopsis
 
@@ -32756,8 +32756,8 @@ and @samp{tcatch rethrow} (@pxref{Set Catchpoints}).
 (gdb)
 @end smallexample
 
-@subheading The @code{-catch-catch} Command
 @findex -catch-catch
+@subheading The @code{-catch-catch} Command
 
 @subsubheading Synopsis
 
@@ -32804,8 +32804,8 @@ and @samp{tcatch catch} (@pxref{Set Catchpoints}).
 @node GDB/MI Program Context
 @section @sc{gdb/mi}  Program Context
 
-@subheading The @code{-exec-arguments} Command
 @findex -exec-arguments
+@subheading The @code{-exec-arguments} Command
 
 
 @subsubheading Synopsis
@@ -32832,8 +32832,8 @@ The corresponding @value{GDBN} command is @samp{set args}.
 
 
 @ignore
-@subheading The @code{-exec-show-arguments} Command
 @findex -exec-show-arguments
+@subheading The @code{-exec-show-arguments} Command
 
 @subsubheading Synopsis
 
@@ -32852,8 +32852,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-environment-cd} Command
 @findex -environment-cd
+@subheading The @code{-environment-cd} Command
 
 @subsubheading Synopsis
 
@@ -32877,8 +32877,8 @@ The corresponding @value{GDBN} command is @samp{cd}.
 @end smallexample
 
 
-@subheading The @code{-environment-directory} Command
 @findex -environment-directory
+@subheading The @code{-environment-directory} Command
 
 @subsubheading Synopsis
 
@@ -32926,8 +32926,8 @@ The corresponding @value{GDBN} command is @samp{dir}.
 @end smallexample
 
 
-@subheading The @code{-environment-path} Command
 @findex -environment-path
+@subheading The @code{-environment-path} Command
 
 @subsubheading Synopsis
 
@@ -32974,8 +32974,8 @@ The corresponding @value{GDBN} command is @samp{path}.
 @end smallexample
 
 
-@subheading The @code{-environment-pwd} Command
 @findex -environment-pwd
+@subheading The @code{-environment-pwd} Command
 
 @subsubheading Synopsis
 
@@ -33003,8 +33003,8 @@ The corresponding @value{GDBN} command is @samp{pwd}.
 @section @sc{gdb/mi} Thread Commands
 
 
-@subheading The @code{-thread-info} Command
 @findex -thread-info
+@subheading The @code{-thread-info} Command
 
 @subsubheading Synopsis
 
@@ -33057,8 +33057,8 @@ current-thread-id="1"
 (gdb)
 @end smallexample
 
-@subheading The @code{-thread-list-ids} Command
 @findex -thread-list-ids
+@subheading The @code{-thread-list-ids} Command
 
 @subsubheading Synopsis
 
@@ -33088,8 +33088,8 @@ current-thread-id="1",number-of-threads="3"
 @end smallexample
 
 
-@subheading The @code{-thread-select} Command
 @findex -thread-select
+@subheading The @code{-thread-select} Command
 
 @subsubheading Synopsis
 
@@ -33135,8 +33135,8 @@ args=[@{name="format",value="0x8048e9c \"%*s%c %d %c\\n\""@},
 @node GDB/MI Ada Tasking Commands
 @section @sc{gdb/mi} Ada Tasking Commands
 
-@subheading The @code{-ada-task-info} Command
 @findex -ada-task-info
+@subheading The @code{-ada-task-info} Command
 
 @subsubheading Synopsis
 
@@ -33218,8 +33218,8 @@ record @samp{*stopped}.  Currently @value{GDBN} only really executes
 asynchronously with remote targets and this interaction is mimicked in
 other cases.
 
-@subheading The @code{-exec-continue} Command
 @findex -exec-continue
+@subheading The @code{-exec-continue} Command
 
 @subsubheading Synopsis
 
@@ -33280,8 +33280,8 @@ line="13",arch="i386:x86_64"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-exec-finish} Command
 @findex -exec-finish
+@subheading The @code{-exec-finish} Command
 
 @subsubheading Synopsis
 
@@ -33330,8 +33330,8 @@ gdb-result-var="$1",return-value="0"
 @end smallexample
 
 
-@subheading The @code{-exec-interrupt} Command
 @findex -exec-interrupt
+@subheading The @code{-exec-interrupt} Command
 
 @subsubheading Synopsis
 
@@ -33381,8 +33381,8 @@ fullname="/home/foo/bar/try.c",line="13",arch="i386:x86_64"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-exec-jump} Command
 @findex -exec-jump
+@subheading The @code{-exec-jump} Command
 
 @subsubheading Synopsis
 
@@ -33407,8 +33407,8 @@ The corresponding @value{GDBN} command is @samp{jump}.
 @end smallexample
 
 
-@subheading The @code{-exec-next} Command
 @findex -exec-next
+@subheading The @code{-exec-next} Command
 
 @subsubheading Synopsis
 
@@ -33441,8 +33441,8 @@ The corresponding @value{GDBN} command is @samp{next}.
 @end smallexample
 
 
-@subheading The @code{-exec-next-instruction} Command
 @findex -exec-next-instruction
+@subheading The @code{-exec-next-instruction} Command
 
 @subsubheading Synopsis
 
@@ -33479,8 +33479,8 @@ addr="0x000100d4",line="5",file="hello.c"
 @end smallexample
 
 
-@subheading The @code{-exec-return} Command
 @findex -exec-return
+@subheading The @code{-exec-return} Command
 
 @subsubheading Synopsis
 
@@ -33526,8 +33526,8 @@ arch="i386:x86_64"@}
 @end smallexample
 
 
-@subheading The @code{-exec-run} Command
 @findex -exec-run
+@subheading The @code{-exec-run} Command
 
 @subsubheading Synopsis
 
@@ -33610,8 +33610,8 @@ signal-meaning="Interrupt"
 @c @subheading -exec-signal
 
 
-@subheading The @code{-exec-step} Command
 @findex -exec-step
+@subheading The @code{-exec-step} Command
 
 @subsubheading Synopsis
 
@@ -33656,8 +33656,8 @@ Regular stepping:
 @end smallexample
 
 
-@subheading The @code{-exec-step-instruction} Command
 @findex -exec-step-instruction
+@subheading The @code{-exec-step-instruction} Command
 
 @subsubheading Synopsis
 
@@ -33700,8 +33700,8 @@ fullname="/home/foo/bar/try.c",line="10",arch="i386:x86_64"@}
 @end smallexample
 
 
-@subheading The @code{-exec-until} Command
 @findex -exec-until
+@subheading The @code{-exec-until} Command
 
 @subsubheading Synopsis
 
@@ -33741,8 +33741,8 @@ Is this going away????
 @node GDB/MI Stack Manipulation
 @section @sc{gdb/mi} Stack Manipulation Commands
 
-@subheading The @code{-enable-frame-filters} Command
 @findex -enable-frame-filters
+@subheading The @code{-enable-frame-filters} Command
 
 @smallexample
 -enable-frame-filters
@@ -33758,8 +33758,8 @@ Once enabled, this feature cannot be disabled.
 Note that if Python support has not been compiled into @value{GDBN},
 this command will still succeed (and do nothing).
 
-@subheading The @code{-stack-info-frame} Command
 @findex -stack-info-frame
+@subheading The @code{-stack-info-frame} Command
 
 @subsubheading Synopsis
 
@@ -33786,8 +33786,8 @@ arch="i386:x86_64"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-stack-info-depth} Command
 @findex -stack-info-depth
+@subheading The @code{-stack-info-depth} Command
 
 @subsubheading Synopsis
 
@@ -33826,8 +33826,8 @@ For a stack with frame levels 0 through 11:
 @end smallexample
 
 @anchor{-stack-list-arguments}
-@subheading The @code{-stack-list-arguments} Command
 @findex -stack-list-arguments
+@subheading The @code{-stack-list-arguments} Command
 
 @subsubheading Synopsis
 
@@ -33931,8 +33931,8 @@ args=[@{name="intarg",value="2"@},
 
 
 @anchor{-stack-list-frames}
-@subheading The @code{-stack-list-frames} Command
 @findex -stack-list-frames
+@subheading The @code{-stack-list-frames} Command
 
 @subsubheading Synopsis
 
@@ -34055,9 +34055,9 @@ Show a single frame:
 @end smallexample
 
 
-@subheading The @code{-stack-list-locals} Command
 @findex -stack-list-locals
 @anchor{-stack-list-locals}
+@subheading The @code{-stack-list-locals} Command
 
 @subsubheading Synopsis
 
@@ -34104,8 +34104,8 @@ This command is deprecated in favor of the
 @end smallexample
 
 @anchor{-stack-list-variables}
-@subheading The @code{-stack-list-variables} Command
 @findex -stack-list-variables
+@subheading The @code{-stack-list-variables} Command
 
 @subsubheading Synopsis
 
@@ -34135,8 +34135,8 @@ available arguments and local variables are still displayed, however.
 @end smallexample
 
 
-@subheading The @code{-stack-select-frame} Command
 @findex -stack-select-frame
+@subheading The @code{-stack-select-frame} Command
 
 @subsubheading Synopsis
 
@@ -34332,8 +34332,8 @@ how it can be used.
 
 @subheading Description And Use of Operations on Variable Objects
 
-@subheading The @code{-enable-pretty-printing} Command
 @findex -enable-pretty-printing
+@subheading The @code{-enable-pretty-printing} Command
 
 @smallexample
 -enable-pretty-printing
@@ -34349,8 +34349,8 @@ Once enabled, this feature cannot be disabled.
 Note that if Python support has not been compiled into @value{GDBN},
 this command will still succeed (and do nothing).
 
-@subheading The @code{-var-create} Command
 @findex -var-create
+@subheading The @code{-var-create} Command
 
 @subsubheading Synopsis
 
@@ -34449,8 +34449,8 @@ Typical output will look like this:
 @end smallexample
 
 
-@subheading The @code{-var-delete} Command
 @findex -var-delete
+@subheading The @code{-var-delete} Command
 
 @subsubheading Synopsis
 
@@ -34464,8 +34464,8 @@ With the @samp{-c} option, just deletes the children.
 Returns an error if the object @var{name} is not found.
 
 
-@subheading The @code{-var-set-format} Command
 @findex -var-set-format
+@subheading The @code{-var-set-format} Command
 
 @subsubheading Synopsis
 
@@ -34496,8 +34496,8 @@ zero-hexadecimal format.
 For a variable with children, the format is set only on the 
 variable itself, and the children are not affected.  
 
-@subheading The @code{-var-show-format} Command
 @findex -var-show-format
+@subheading The @code{-var-show-format} Command
 
 @subsubheading Synopsis
 
@@ -34513,8 +34513,8 @@ Returns the format used to display the value of the object @var{name}.
 @end smallexample
 
 
-@subheading The @code{-var-info-num-children} Command
 @findex -var-info-num-children
+@subheading The @code{-var-info-num-children} Command
 
 @subsubheading Synopsis
 
@@ -34533,8 +34533,8 @@ It will return the current number of children, but more children may
 be available.
 
 
-@subheading The @code{-var-list-children} Command
 @findex -var-list-children
+@subheading The @code{-var-list-children} Command
 
 @subsubheading Synopsis
 
@@ -34650,8 +34650,8 @@ remaining after the end of the selected range.
 @end smallexample
 
 
-@subheading The @code{-var-info-type} Command
 @findex -var-info-type
+@subheading The @code{-var-info-type} Command
 
 @subsubheading Synopsis
 
@@ -34668,8 +34668,8 @@ returned as a string in the same format as it is output by the
 @end smallexample
 
 
-@subheading The @code{-var-info-expression} Command
 @findex -var-info-expression
+@subheading The @code{-var-info-expression} Command
 
 @subsubheading Synopsis
 
@@ -34697,8 +34697,8 @@ Note that the output of the @code{-var-list-children} command also
 includes those expressions, so the @code{-var-info-expression} command
 is of limited use.
 
-@subheading The @code{-var-info-path-expression} Command
 @findex -var-info-path-expression
+@subheading The @code{-var-info-path-expression} Command
 
 @subsubheading Synopsis
 
@@ -34726,8 +34726,8 @@ For example, suppose @code{C} is a C@t{++} class, derived from class
 ^done,path_expr=((Base)c).m_size)
 @end smallexample
 
-@subheading The @code{-var-show-attributes} Command
 @findex -var-show-attributes
+@subheading The @code{-var-show-attributes} Command
 
 @subsubheading Synopsis
 
@@ -34744,8 +34744,8 @@ List attributes of the specified variable object @var{name}:
 @noindent
 where @var{attr} is @code{@{ @{ editable | noneditable @} | TBD @}}.
 
-@subheading The @code{-var-evaluate-expression} Command
 @findex -var-evaluate-expression
+@subheading The @code{-var-evaluate-expression} Command
 
 @subsubheading Synopsis
 
@@ -34768,8 +34768,8 @@ can be changed using the @code{-var-set-format} command.
 Note that one must invoke @code{-var-list-children} for a variable
 before the value of a child variable can be evaluated.
 
-@subheading The @code{-var-assign} Command
 @findex -var-assign
+@subheading The @code{-var-assign} Command
 
 @subsubheading Synopsis
 
@@ -34794,8 +34794,8 @@ subsequent @code{-var-update} list.
 (gdb)
 @end smallexample
 
-@subheading The @code{-var-update} Command
 @findex -var-update
+@subheading The @code{-var-update} Command
 
 @subsubheading Synopsis
 
@@ -34922,9 +34922,9 @@ type_changed="false"@}]
 (gdb)
 @end smallexample
 
-@subheading The @code{-var-set-frozen} Command
 @findex -var-set-frozen
 @anchor{-var-set-frozen}
+@subheading The @code{-var-set-frozen} Command
 
 @subsubheading Synopsis
 
@@ -34953,9 +34953,9 @@ Unfreezing a variable does not update it, only subsequent
 (gdb)
 @end smallexample
 
-@subheading The @code{-var-set-update-range} command
 @findex -var-set-update-range
 @anchor{-var-set-update-range}
+@subheading The @code{-var-set-update-range} command
 
 @subsubheading Synopsis
 
@@ -34979,9 +34979,9 @@ children will be reported.  Otherwise, children starting at @var{from}
 ^done
 @end smallexample
 
-@subheading The @code{-var-set-visualizer} command
 @findex -var-set-visualizer
 @anchor{-var-set-visualizer}
+@subheading The @code{-var-set-visualizer} command
 
 @subsubheading Synopsis
 
@@ -35058,8 +35058,8 @@ For details about what an addressable memory unit is,
 @c @subsubheading Example
 @c N.A.
 
-@subheading The @code{-data-disassemble} Command
 @findex -data-disassemble
+@subheading The @code{-data-disassemble} Command
 
 @subsubheading Synopsis
 
@@ -35314,8 +35314,8 @@ inst="sethi  %hi(0x11800), %o2"@}]@}]
 @end smallexample
 
 
-@subheading The @code{-data-evaluate-expression} Command
 @findex -data-evaluate-expression
+@subheading The @code{-data-evaluate-expression} Command
 
 @subsubheading Synopsis
 
@@ -35356,8 +35356,8 @@ output.
 @end smallexample
 
 
-@subheading The @code{-data-list-changed-registers} Command
 @findex -data-list-changed-registers
+@subheading The @code{-data-list-changed-registers} Command
 
 @subsubheading Synopsis
 
@@ -35394,8 +35394,8 @@ line="5",arch="powerpc"@}
 @end smallexample
 
 
-@subheading The @code{-data-list-register-names} Command
 @findex -data-list-register-names
+@subheading The @code{-data-list-register-names} Command
 
 @subsubheading Synopsis
 
@@ -35435,8 +35435,8 @@ For the PPC MBX board:
 (gdb)
 @end smallexample
 
-@subheading The @code{-data-list-register-values} Command
 @findex -data-list-register-values
+@subheading The @code{-data-list-register-values} Command
 
 @subsubheading Synopsis
 
@@ -35526,8 +35526,8 @@ don't appear in the actual output):
 @end smallexample
 
 
-@subheading The @code{-data-read-memory} Command
 @findex -data-read-memory
+@subheading The @code{-data-read-memory} Command
 
 This command is deprecated, use @code{-data-read-memory-bytes} instead.
 
@@ -35642,8 +35642,8 @@ next-page="0x000013c0",prev-page="0x00001380",memory=[
 (gdb)
 @end smallexample
 
-@subheading The @code{-data-read-memory-bytes} Command
 @findex -data-read-memory-bytes
+@subheading The @code{-data-read-memory-bytes} Command
 
 @subsubheading Synopsis
 
@@ -35728,8 +35728,8 @@ The corresponding @value{GDBN} command is @samp{x}.
 @end smallexample
 
 
-@subheading The @code{-data-write-memory-bytes} Command
 @findex -data-write-memory-bytes
+@subheading The @code{-data-write-memory-bytes} Command
 
 @subsubheading Synopsis
 
@@ -35786,8 +35786,8 @@ There's no corresponding @value{GDBN} command.
 The commands defined in this section implement MI support for
 tracepoints.  For detailed introduction, see @ref{Tracepoints}.
 
-@subheading The @code{-trace-find} Command
 @findex -trace-find
+@subheading The @code{-trace-find} Command
 
 @subsubheading Synopsis
 
@@ -35861,8 +35861,8 @@ frame.  This field is present only if a trace frame was found.
 
 The corresponding @value{GDBN} command is @samp{tfind}.
 
-@subheading -trace-define-variable
 @findex -trace-define-variable
+@subheading -trace-define-variable
 
 @subsubheading Synopsis
 
@@ -35879,8 +35879,8 @@ with the @samp{$} character.
 
 The corresponding @value{GDBN} command is @samp{tvariable}.
 
-@subheading The @code{-trace-frame-collected} Command
 @findex -trace-frame-collected
+@subheading The @code{-trace-frame-collected} Command
 
 @subsubheading Synopsis
 
@@ -35996,8 +35996,8 @@ There is no corresponding @value{GDBN} command.
 
 @subsubheading Example
 
-@subheading -trace-list-variables
 @findex -trace-list-variables
+@subheading -trace-list-variables
 
 @subsubheading Synopsis
 
@@ -36042,8 +36042,8 @@ body=[variable=@{name="$trace_timestamp",initial="0"@}
 (gdb)
 @end smallexample
 
-@subheading -trace-save
 @findex -trace-save
+@subheading -trace-save
 
 @subsubheading Synopsis
 
@@ -36065,8 +36065,8 @@ supply the optional @samp{-ctf} argument to save it the CTF format. See
 The corresponding @value{GDBN} command is @samp{tsave}.
 
 
-@subheading -trace-start
 @findex -trace-start
+@subheading -trace-start
 
 @subsubheading Synopsis
 
@@ -36081,8 +36081,8 @@ have any fields.
 
 The corresponding @value{GDBN} command is @samp{tstart}.
 
-@subheading -trace-status
 @findex -trace-status
+@subheading -trace-status
 
 @subsubheading Synopsis
 
@@ -36156,8 +36156,8 @@ optional, and only present when examining a trace file.
 
 The corresponding @value{GDBN} command is @samp{tstatus}.
 
-@subheading -trace-stop
 @findex -trace-stop
+@subheading -trace-stop
 
 @subsubheading Synopsis
 
@@ -36180,8 +36180,8 @@ The corresponding @value{GDBN} command is @samp{tstop}.
 
 
 @ignore
-@subheading The @code{-symbol-info-address} Command
 @findex -symbol-info-address
+@subheading The @code{-symbol-info-address} Command
 
 @subsubheading Synopsis
 
@@ -36199,8 +36199,8 @@ The corresponding @value{GDBN} command is @samp{info address}.
 N.A.
 
 
-@subheading The @code{-symbol-info-file} Command
 @findex -symbol-info-file
+@subheading The @code{-symbol-info-file} Command
 
 @subsubheading Synopsis
 
@@ -36219,9 +36219,9 @@ There's no equivalent @value{GDBN} command.  @code{gdbtk} has
 N.A.
 @end ignore
 
-@subheading The @code{-symbol-info-functions} Command
 @findex -symbol-info-functions
 @anchor{-symbol-info-functions}
+@subheading The @code{-symbol-info-functions} Command
 
 @subsubheading Synopsis
 
@@ -36331,9 +36331,9 @@ The corresponding @value{GDBN} command is @samp{info functions}.
 @end group
 @end smallexample
 
-@subheading The @code{-symbol-info-module-functions} Command
 @findex -symbol-info-module-functions
 @anchor{-symbol-info-module-functions}
+@subheading The @code{-symbol-info-module-functions} Command
 
 @subsubheading Synopsis
 
@@ -36397,9 +36397,9 @@ The corresponding @value{GDBN} command is @samp{info module functions}.
 @end group
 @end smallexample
 
-@subheading The @code{-symbol-info-module-variables} Command
 @findex -symbol-info-module-variables
 @anchor{-symbol-info-module-variables}
+@subheading The @code{-symbol-info-module-variables} Command
 
 @subsubheading Synopsis
 
@@ -36473,9 +36473,9 @@ The corresponding @value{GDBN} command is @samp{info module variables}.
 @end group
 @end smallexample
 
-@subheading The @code{-symbol-info-modules} Command
 @findex -symbol-info-modules
 @anchor{-symbol-info-modules}
+@subheading The @code{-symbol-info-modules} Command
 
 @subsubheading Synopsis
 
@@ -36534,9 +36534,9 @@ The corresponding @value{GDBN} command is @samp{info modules}.
 @end group
 @end smallexample
 
-@subheading The @code{-symbol-info-types} Command
 @findex -symbol-info-types
 @anchor{-symbol-info-types}
+@subheading The @code{-symbol-info-types} Command
 
 @subsubheading Synopsis
 
@@ -36599,9 +36599,9 @@ The corresponding @value{GDBN} command is @samp{info types}.
 @end group
 @end smallexample
 
-@subheading The @code{-symbol-info-variables} Command
 @findex -symbol-info-variables
 @anchor{-symbol-info-variables}
+@subheading The @code{-symbol-info-variables} Command
 
 @subsubheading Synopsis
 
@@ -36717,8 +36717,8 @@ The corresponding @value{GDBN} command is @samp{info variables}.
 @end smallexample
 
 @ignore
-@subheading The @code{-symbol-info-line} Command
 @findex -symbol-info-line
+@subheading The @code{-symbol-info-line} Command
 
 @subsubheading Synopsis
 
@@ -36737,8 +36737,8 @@ The corresponding @value{GDBN} command is @samp{info line}.
 N.A.
 
 
-@subheading The @code{-symbol-info-symbol} Command
 @findex -symbol-info-symbol
+@subheading The @code{-symbol-info-symbol} Command
 
 @subsubheading Synopsis
 
@@ -36756,8 +36756,8 @@ The corresponding @value{GDBN} command is @samp{info symbol}.
 N.A.
 
 
-@subheading The @code{-symbol-list-functions} Command
 @findex -symbol-list-functions
+@subheading The @code{-symbol-list-functions} Command
 
 @subsubheading Synopsis
 
@@ -36777,8 +36777,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-symbol-list-lines} Command
 @findex -symbol-list-lines
+@subheading The @code{-symbol-list-lines} Command
 
 @subsubheading Synopsis
 
@@ -36804,8 +36804,8 @@ There is no corresponding @value{GDBN} command.
 
 
 @ignore
-@subheading The @code{-symbol-list-types} Command
 @findex -symbol-list-types
+@subheading The @code{-symbol-list-types} Command
 
 @subsubheading Synopsis
 
@@ -36824,8 +36824,8 @@ The corresponding commands are @samp{info types} in @value{GDBN},
 N.A.
 
 
-@subheading The @code{-symbol-list-variables} Command
 @findex -symbol-list-variables
+@subheading The @code{-symbol-list-variables} Command
 
 @subsubheading Synopsis
 
@@ -36843,8 +36843,8 @@ List all the global and static variable names.
 N.A.
 
 
-@subheading The @code{-symbol-locate} Command
 @findex -symbol-locate
+@subheading The @code{-symbol-locate} Command
 
 @subsubheading Synopsis
 
@@ -36860,8 +36860,8 @@ N.A.
 N.A.
 
 
-@subheading The @code{-symbol-type} Command
 @findex -symbol-type
+@subheading The @code{-symbol-type} Command
 
 @subsubheading Synopsis
 
@@ -36888,8 +36888,8 @@ N.A.
 This section describes the GDB/MI commands to specify executable file names
 and to read in and obtain symbol table information.
 
-@subheading The @code{-file-exec-and-symbols} Command
 @findex -file-exec-and-symbols
+@subheading The @code{-file-exec-and-symbols} Command
 
 @subsubheading Synopsis
 
@@ -36918,8 +36918,8 @@ The corresponding @value{GDBN} command is @samp{file}.
 @end smallexample
 
 
-@subheading The @code{-file-exec-file} Command
 @findex -file-exec-file
+@subheading The @code{-file-exec-file} Command
 
 @subsubheading Synopsis
 
@@ -36948,8 +36948,8 @@ The corresponding @value{GDBN} command is @samp{exec-file}.
 
 
 @ignore
-@subheading The @code{-file-list-exec-sections} Command
 @findex -file-list-exec-sections
+@subheading The @code{-file-list-exec-sections} Command
 
 @subsubheading Synopsis
 
@@ -36970,8 +36970,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-file-list-exec-source-file} Command
 @findex -file-list-exec-source-file
+@subheading The @code{-file-list-exec-source-file} Command
 
 @subsubheading Synopsis
 
@@ -36998,9 +36998,9 @@ The @value{GDBN} equivalent is @samp{info source}
 @end smallexample
 
 
+@findex -file-list-exec-source-files
 @subheading The @code{-file-list-exec-source-files} Command
 @kindex info sources
-@findex -file-list-exec-source-files
 
 @subsubheading Synopsis
 
@@ -37138,8 +37138,8 @@ The @value{GDBN} equivalent is @samp{info sources}.
               sources=[]@}]
 @end smallexample
 
-@subheading The @code{-file-list-shared-libraries} Command
 @findex -file-list-shared-libraries
+@subheading The @code{-file-list-shared-libraries} Command
 
 @subsubheading Synopsis
 
@@ -37177,8 +37177,8 @@ The address defining the exclusive upper bound of the segment.
 
 
 @ignore
-@subheading The @code{-file-list-symbol-files} Command
 @findex -file-list-symbol-files
+@subheading The @code{-file-list-symbol-files} Command
 
 @subsubheading Synopsis
 
@@ -37197,8 +37197,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-file-symbol-file} Command
 @findex -file-symbol-file
+@subheading The @code{-file-symbol-file} Command
 
 @subsubheading Synopsis
 
@@ -37263,8 +37263,8 @@ Signal handling commands are not implemented.
 @section @sc{gdb/mi} Target Manipulation Commands
 
 
-@subheading The @code{-target-attach} Command
 @findex -target-attach
+@subheading The @code{-target-attach} Command
 
 @subsubheading Synopsis
 
@@ -37292,8 +37292,8 @@ The corresponding @value{GDBN} command is @samp{attach}.
 @end smallexample
 
 @ignore
-@subheading The @code{-target-compare-sections} Command
 @findex -target-compare-sections
+@subheading The @code{-target-compare-sections} Command
 
 @subsubheading Synopsis
 
@@ -37313,8 +37313,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-target-detach} Command
 @findex -target-detach
+@subheading The @code{-target-detach} Command
 
 @subsubheading Synopsis
 
@@ -37340,8 +37340,8 @@ The corresponding @value{GDBN} command is @samp{detach}.
 @end smallexample
 
 
-@subheading The @code{-target-disconnect} Command
 @findex -target-disconnect
+@subheading The @code{-target-disconnect} Command
 
 @subsubheading Synopsis
 
@@ -37366,8 +37366,8 @@ The corresponding @value{GDBN} command is @samp{disconnect}.
 @end smallexample
 
 
-@subheading The @code{-target-download} Command
 @findex -target-download
+@subheading The @code{-target-download} Command
 
 @subsubheading Synopsis
 
@@ -37471,8 +37471,8 @@ write-rate="429"
 
 
 @ignore
-@subheading The @code{-target-exec-status} Command
 @findex -target-exec-status
+@subheading The @code{-target-exec-status} Command
 
 @subsubheading Synopsis
 
@@ -37491,8 +37491,8 @@ There's no equivalent @value{GDBN} command.
 N.A.
 
 
-@subheading The @code{-target-list-available-targets} Command
 @findex -target-list-available-targets
+@subheading The @code{-target-list-available-targets} Command
 
 @subsubheading Synopsis
 
@@ -37510,8 +37510,8 @@ The corresponding @value{GDBN} command is @samp{help target}.
 N.A.
 
 
-@subheading The @code{-target-list-current-targets} Command
 @findex -target-list-current-targets
+@subheading The @code{-target-list-current-targets} Command
 
 @subsubheading Synopsis
 
@@ -37530,8 +37530,8 @@ other things).
 N.A.
 
 
-@subheading The @code{-target-list-parameters} Command
 @findex -target-list-parameters
+@subheading The @code{-target-list-parameters} Command
 
 @subsubheading Synopsis
 
@@ -37549,8 +37549,8 @@ No equivalent.
 @subsubheading Example
 N.A.
 
-@subheading The @code{-target-flash-erase} Command
 @findex -target-flash-erase
+@subheading The @code{-target-flash-erase} Command
 
 @subsubheading Synopsis
 
@@ -37572,8 +37572,8 @@ addresses and memory region sizes.
 (gdb)
 @end smallexample
 
-@subheading The @code{-target-select} Command
 @findex -target-select
+@subheading The @code{-target-select} Command
 
 @subsubheading Synopsis
 
@@ -37617,8 +37617,8 @@ The corresponding @value{GDBN} command is @samp{target}.
 @section @sc{gdb/mi} File Transfer Commands
 
 
-@subheading The @code{-target-file-put} Command
 @findex -target-file-put
+@subheading The @code{-target-file-put} Command
 
 @subsubheading Synopsis
 
@@ -37643,8 +37643,8 @@ The corresponding @value{GDBN} command is @samp{remote put}.
 @end smallexample
 
 
-@subheading The @code{-target-file-get} Command
 @findex -target-file-get
+@subheading The @code{-target-file-get} Command
 
 @subsubheading Synopsis
 
@@ -37669,8 +37669,8 @@ The corresponding @value{GDBN} command is @samp{remote get}.
 @end smallexample
 
 
-@subheading The @code{-target-file-delete} Command
 @findex -target-file-delete
+@subheading The @code{-target-file-delete} Command
 
 @subsubheading Synopsis
 
@@ -37698,8 +37698,8 @@ The corresponding @value{GDBN} command is @samp{remote delete}.
 @node GDB/MI Ada Exceptions Commands
 @section Ada Exceptions @sc{gdb/mi} Commands
 
-@subheading The @code{-info-ada-exceptions} Command
 @findex -info-ada-exceptions
+@subheading The @code{-info-ada-exceptions} Command
 
 @subsubheading Synopsis
 
@@ -37756,9 +37756,9 @@ some commands are available to help front-ends query the debugger
 about support for these capabilities.  Similarly, it is also possible
 to query @value{GDBN} about target support of certain features.
 
-@subheading The @code{-info-gdb-mi-command} Command
 @cindex @code{-info-gdb-mi-command}
 @findex -info-gdb-mi-command
+@subheading The @code{-info-gdb-mi-command} Command
 
 @subsubheading Synopsis
 
@@ -37807,9 +37807,9 @@ to the debugger:
 ^done,command=@{exists="true"@}
 @end smallexample
 
-@subheading The @code{-list-features} Command
 @findex -list-features
 @cindex supported @sc{gdb/mi} features, list
+@subheading The @code{-list-features} Command
 
 Returns a list of particular features of the MI protocol that
 this version of gdb implements.  A feature can be a command,
@@ -37871,8 +37871,8 @@ Indicates that the @code{-data-disassemble} command supports the @option{-a}
 option (@pxref{GDB/MI Data Manipulation}).
 @end ftable
 
-@subheading The @code{-list-target-features} Command
 @findex -list-target-features
+@subheading The @code{-list-target-features} Command
 
 Returns a list of particular features that are supported by the
 target.  Those features affect the permitted MI commands, but 
@@ -37908,8 +37908,8 @@ Indicates that the target is capable of reverse execution.
 
 @c @subheading -gdb-complete
 
-@subheading The @code{-gdb-exit} Command
 @findex -gdb-exit
+@subheading The @code{-gdb-exit} Command
 
 @subsubheading Synopsis
 
@@ -37933,8 +37933,8 @@ Approximately corresponds to @samp{quit}.
 
 
 @ignore
-@subheading The @code{-exec-abort} Command
 @findex -exec-abort
+@subheading The @code{-exec-abort} Command
 
 @subsubheading Synopsis
 
@@ -37953,8 +37953,8 @@ N.A.
 @end ignore
 
 
-@subheading The @code{-gdb-set} Command
 @findex -gdb-set
+@subheading The @code{-gdb-set} Command
 
 @subsubheading Synopsis
 
@@ -37979,8 +37979,8 @@ The corresponding @value{GDBN} command is @samp{set}.
 @end smallexample
 
 
-@subheading The @code{-gdb-show} Command
 @findex -gdb-show
+@subheading The @code{-gdb-show} Command
 
 @subsubheading Synopsis
 
@@ -38006,8 +38006,8 @@ The corresponding @value{GDBN} command is @samp{show}.
 @c @subheading -gdb-source
 
 
-@subheading The @code{-gdb-version} Command
 @findex -gdb-version
+@subheading The @code{-gdb-version} Command
 
 @subsubheading Synopsis
 
@@ -38043,8 +38043,8 @@ default shows this information when you start an interactive session.
 (gdb)
 @end smallexample
 
-@subheading The @code{-list-thread-groups} Command
 @findex -list-thread-groups
+@subheading The @code{-list-thread-groups} Command
 
 @subheading Synopsis
 
@@ -38164,8 +38164,8 @@ and only if there is a corresponding executable file.
                         @{id="2",target-id="Thread 0xb7e14b90",cores=[2]@}]@},...]
 @end smallexample
 
-@subheading The @code{-info-os} Command
 @findex -info-os
+@subheading The @code{-info-os} Command
 
 @subsubheading Synopsis
 
@@ -38239,8 +38239,8 @@ for MI clients that want to enumerate the types of data, such as in a
 popup menu, but is needless clutter on the command line, and
 @code{info os} omits it.)
 
-@subheading The @code{-add-inferior} Command
 @findex -add-inferior
+@subheading The @code{-add-inferior} Command
 
 @subheading Synopsis
 
@@ -38293,8 +38293,8 @@ The corresponding @value{GDBN} command is @samp{add-inferior}
 ^done,inferior="i3"
 @end smallexample
 
-@subheading The @code{-interpreter-exec} Command
 @findex -interpreter-exec
+@subheading The @code{-interpreter-exec} Command
 
 @subheading Synopsis
 
@@ -38321,8 +38321,8 @@ The corresponding @value{GDBN} command is @samp{interpreter-exec}.
 (gdb)
 @end smallexample
 
-@subheading The @code{-inferior-tty-set} Command
 @findex -inferior-tty-set
+@subheading The @code{-inferior-tty-set} Command
 
 @subheading Synopsis
 
@@ -38345,8 +38345,8 @@ The corresponding @value{GDBN} command is @samp{set inferior-tty} /dev/pts/1.
 (gdb)
 @end smallexample
 
-@subheading The @code{-inferior-tty-show} Command
 @findex -inferior-tty-show
+@subheading The @code{-inferior-tty-show} Command
 
 @subheading Synopsis
 
@@ -38372,8 +38372,8 @@ The corresponding @value{GDBN} command is @samp{show inferior-tty}.
 (gdb)
 @end smallexample
 
-@subheading The @code{-enable-timings} Command
 @findex -enable-timings
+@subheading The @code{-enable-timings} Command
 
 @subheading Synopsis
 
@@ -38417,8 +38417,8 @@ fullname="/home/nickrob/myprog.c",line="73",arch="i386:x86_64"@}
 (gdb)
 @end smallexample
 
-@subheading The @code{-complete} Command
 @findex -complete
+@subheading The @code{-complete} Command
 
 @subheading Synopsis
 

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
-- 
2.36.0

Re: [PATCH] gdb/manual: Move @findex entries (was: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function)

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 13 Feb 2023 16:47:19 +0000
> 
> > In an Info reader, type "i NAME" where NAME is the name of a function,
> > and see where it lands you.
> 
> Thanks, I see now.
> 
> Here's a patch fixing that throughout the manual.  Turns out that most if not
> all @findex calls in gdb.texinfo needed to be fixed.
> 
> WDYT?

LGTM, thanks.

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Pedro Alves]

On 2023-02-13 3:36 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>

>>>> +Invoke a standard shell to execute @var{command-string}.  Returns the
>>>           ^^^^^^^^^^^^^^^^
>>> "the standard system shell" is better, I think.
>>
>> Note this is the same wording used when describing the "shell" command:
>>
>>  @table @code
>>  @kindex shell
>>  @kindex !
>>  @cindex shell escape
>>  @item shell @var{command-string}
>>  @itemx !@var{command-string}
>>  Invoke a standard shell to execute @var{command-string}.
>>  Note that no space is needed between @code{!} and @var{command-string}.
>>  On GNU and Unix systems, the environment variable @env{SHELL}, if it
>>  exists, determines which shell to run.  Otherwise @value{GDBN} uses
>>  the default shell (@file{/bin/sh} on GNU and Unix systems,
>>  @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
>>  @end table
>>
>>
>> Maybe the intention was to avoid "system" as @env{SHELL} may point
>> to some shell the user installed, which doesn't come with or from
>> the "system"?  So I'd prefer to use the same terminology in $_shell too.
> 
> I thought 'system' is not affected by $SHELL, but if it is, then
> something like "Invoke a shell to execute @var{command-string}", I
> think.  The explanation of how $SHELL affects the shell describes the
> details.

I made that change, to both shell command and $_shell function.

But note, I meant, maybe the existing description of the "shell" command avoids
using the word "system" (not the 'system' function), as $SHELL may point to
a non-system shell.

But I see now value in saying "standard" either, so I'm going with your
suggestion, just saying "shell" should be sufficient.

> 
>>> Also, this text could benefit from a cross-reference to where we
>>> describe the commands that convert from signal numbers to their
>>> mnemonics, since that is system-dependent.  Maybe "info signal N" is
>>> the right tool here?  If so, a cross-reference to "Signals" is what we
>>> should have here.  (Is there a better way of asking GDB about signal
>>> number N?)
>>
>> I don't think it's the right tool here, because "info signal" and
>> "handle", etc., are about target signals, signals that trigger in the
>> inferior, which GDB is able to intercept.  OTOH, the $_shell function
>> and the "shell" command run the shell on the host, so any fatal signal code
>> is a host signal number, and doesn't go via any translation at all.
> 
> For native debugging, they are the same, of course.  So mentioning
> that with the caveat of native debugging will help in some cases.
> 
>> I'll note that where we document $_shell_exitsignal, the convenience
>> variable that the "shell" command sets, we don't describe the host signals
>> either.
> 
> Then maybe we should have some text to tell the user how to map
> numbers to signal mnemonics in the non-native case.

We really have no such mapping documented anywhere, not even for target signals,
and I don't think it's feasible to have it, as each different OS will have a
different mapping.

I think this is the best we can do:

 ...
 +shells.  Note that @var{N} is a host signal number, not a target
 +signal number.  If you're native debugging, they will be the same, but
 +if cross debugging, the host vs target signal numbers may be
 +completely unrelated.  Please consult your host operating system's
 +documentation for the mapping between host signal numbers and signal
 +names.  The shell to run is determined in the same way as for the
 ...

> 
>> I've moved the "The shell runs on the host machine, the machine @value{GDBN} is
>> running on." sentence earlier, in case that helps set expectations, and
>> added a couple sentences about host vs target signal numbers.  
>>
>> I had also forgotten to document that the argument must be a string.
>>
>> Here is the result:
>>
>> "Invoke a standard shell to execute @var{command-string}.
>> @var{command-string} must be a string.  The shell runs on the host
>> machine, the machine @value{GDBN} is running on.  Returns the
>> command's exit status.  On Unix systems, a command which exits with a
>> zero exit status has succeeded, and non-zero exit status indicates
>> failure.  When a command terminates on a fatal signal whose number is
>> @var{N}, @value{GDBN} uses the value 128+@var{N} as the exit status,
>> as is standard in Unix shells.  Note that @var{N} is a host signal
>> number, not a target signal number.  If you're cross debugging, the
>> host vs target signal numbers may be completely unrelated.  The shell
>> to run is determined in the same way as for the @code{shell} command.
>> @xref{Shell Commands, ,Shell Commands}."
>>
>> WDYT?
> 
> OK, with the above nits.
Would you mind taking another look?  Here's what it looks like currently:

From: Pedro Alves <pedro@palves.net>
Subject: [PATCH] Add new "$_shell(CMD)" internal function

For testing a following patch, I wanted a way to send a SIGINT to GDB
from a breakpoint condition.  And I didn't want to do it from a Python
breakpoint or Python function, as I wanted to exercise non-Python code
paths.  So I thought I'd add a new $_shell internal function, that
runs a command under the shell, and returns the exit code.  With this,
I could write:

  (gdb) b foo if $_shell("kill -SIGINT $gdb_pid") != 0 || <other condition>

I think this is generally useful, hence I'm proposing it here.

Here's the new function in action:

 (gdb) p $_shell("true")
 $1 = 0
 (gdb) p $_shell("false")
 $2 = 1
 (gdb) p $_shell("echo hello")
 hello
 $3 = 0
 (gdb) p $_shell("foobar")
 bash: line 1: foobar: command not found
 $4 = 127
 (gdb) help function _shell
 $_shell - execute a shell command and returns the result.
 Usage: $_shell (command)
 Returns the command's exit code: zero on success, non-zero otherwise.
 (gdb)

NEWS and manual changes included.

Reviewed-by: Eli Zaretskii <eliz@gnu.org>
Change-Id: I7e36d451ee6b428cbf41fded415ae2d6b4efaa4e
---
 gdb/NEWS                           | 10 ++++
 gdb/cli/cli-cmds.c                 | 96 ++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo                | 60 ++++++++++++++++++-
 gdb/testsuite/gdb.base/default.exp |  1 +
 gdb/testsuite/gdb.base/shell.exp   | 36 +++++++++++
 5 files changed, 198 insertions(+), 5 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b85923cf80d..0d3445438b1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -56,6 +56,16 @@ maintenance info frame-unwinders
   List the frame unwinders currently in effect, starting with the highest
   priority.
 
+* New convenience function "$_shell", to execute a shell command and
+  return the result.  This lets you run shell commands in expressions.
+  Some examples:
+
+   (gdb) p $_shell("true")
+   $1 = 0
+   (gdb) p $_shell("false")
+   $2 = 1
+   (gdb) break func if $_shell("some command") == 0
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 6c0d780face..7607fe59b05 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -39,6 +39,7 @@
 #include "gdbsupport/filestuff.h"
 #include "location.h"
 #include "block.h"
+#include "valprint.h"
 
 #include "ui-out.h"
 #include "interps.h"
@@ -873,6 +874,9 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
+
+  /* Keep the logic here in sync with shell_internal_fn.  */
+
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 #ifdef __MINGW32__
@@ -893,8 +897,11 @@ exit_status_set_internal_vars (int exit_status)
     warning (_("unexpected shell command exit status %d"), exit_status);
 }
 
-static void
-shell_escape (const char *arg, int from_tty)
+/* Run ARG under the shell, and return the exit status.  If ARG is
+   NULL, run an interactive shell.  */
+
+static int
+run_under_shell (const char *arg, int from_tty)
 {
 #if defined(CANT_FORK) || \
       (!defined(HAVE_WORKING_VFORK) && !defined(HAVE_WORKING_FORK))
@@ -915,7 +922,7 @@ shell_escape (const char *arg, int from_tty)
      the shell command we just ran changed it.  */
   chdir (current_directory);
 #endif
-  exit_status_set_internal_vars (rc);
+  return rc;
 #else /* Can fork.  */
   int status, pid;
 
@@ -942,10 +949,21 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
-  exit_status_set_internal_vars (status);
+  return status;
 #endif /* Can fork.  */
 }
 
+/* Escape out to the shell to run ARG.  If ARG is NULL, launch and
+   interactive shell.  Sets $_shell_exitcode and $_shell_exitsignal
+   convenience variables based on the exits status.  */
+
+static void
+shell_escape (const char *arg, int from_tty)
+{
+  int status = run_under_shell (arg, from_tty);
+  exit_status_set_internal_vars (status);
+}
+
 /* Implementation of the "shell" command.  */
 
 static void
@@ -2417,6 +2435,63 @@ gdb_maint_setting_str_internal_fn (struct gdbarch *gdbarch,
   return str_value_from_setting (*show_cmd->var, gdbarch);
 }
 
+/* Implementation of the convenience function $_shell.  */
+
+static struct value *
+shell_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_shell."));
+
+  value *val = argv[0];
+  struct type *type = check_typedef (value_type (val));
+
+  if (!language->is_string_type_p (type))
+    error (_("Argument must be a string."));
+
+  value_print_options opts;
+  get_no_prettyformat_print_options (&opts);
+
+  string_file stream;
+  value_print (val, &stream, &opts);
+
+  /* We should always have two quote chars, which we'll strip.  */
+  gdb_assert (stream.size () >= 2);
+
+  /* Now strip them.  We don't need the original string, so it's
+     cheaper to do it in place, avoiding a string allocation.  */
+  std::string str = stream.release ();
+  str[str.size () - 1] = 0;
+  const char *command = str.c_str () + 1;
+
+  int exit_status = run_under_shell (command, 0);
+
+  struct type *int_type = builtin_type (gdbarch)->builtin_int;
+
+  /* Keep the logic here in sync with
+     exit_status_set_internal_vars.  */
+
+  if (WIFEXITED (exit_status))
+    return value_from_longest (int_type, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* See exit_status_set_internal_vars.  */
+      return value_from_longest (int_type, exit_status);
+    }
+#endif
+  else if (WIFSIGNALED (exit_status))
+    {
+      /* (0x80 | SIGNO) is what most (all?) POSIX-like shells set as
+	 exit code on fatal signal termination.  */
+      return value_from_longest (int_type, 0x80 | WTERMSIG (exit_status));
+    }
+  else
+    return allocate_optimized_out_value (int_type);
+}
+
 void _initialize_cli_cmds ();
 void
 _initialize_cli_cmds ()
@@ -2606,6 +2681,19 @@ Some integer settings accept an unlimited value, returned\n\
 as 0 or -1 depending on the setting."),
 			 gdb_maint_setting_internal_fn, NULL);
 
+  add_internal_function ("_shell", _("\
+$_shell - execute a shell command and return the result.\n\
+\n\
+    Usage: $_shell (COMMAND)\n\
+\n\
+    Arguments:\n\
+\n\
+      COMMAND: The command to execute.  Must be a string.\n\
+\n\
+    Returns:\n\
+      The command's exit code: zero on success, non-zero otherwise."),
+			 shell_internal_fn, NULL);
+
   add_cmd ("commands", no_set_class, show_commands, _("\
 Show the history of commands you typed.\n\
 You can supply a command number to start with, or a `+' to start after\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 79cb06e496e..d78627ad262 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1621,7 +1621,7 @@ just use the @code{shell} command.
 @cindex shell escape
 @item shell @var{command-string}
 @itemx !@var{command-string}
-Invoke a standard shell to execute @var{command-string}.
+Invoke a shell to execute @var{command-string}.
 Note that no space is needed between @code{!} and @var{command-string}.
 On GNU and Unix systems, the environment variable @env{SHELL}, if it
 exists, determines which shell to run.  Otherwise @value{GDBN} uses
@@ -1629,6 +1629,10 @@ the default shell (@file{/bin/sh} on GNU and Unix systems,
 @file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
+You may also invoke shell commands from expressions, using the
+@code{$_shell} convenience function.  @xref{$_shell convenience
+function}.
+
 The utility @code{make} is often needed in development environments.
 You do not have to use the @code{shell} command for this purpose in
 @value{GDBN}:
@@ -12969,6 +12973,60 @@ Like the @code{$_gdb_setting_str} function, but works with
 Like the @code{$_gdb_setting} function, but works with
 @code{maintenance set} variables.
 
+@anchor{$_shell convenience function}
+@findex $_shell@r{, convenience function}
+@item $_shell (@var{command-string})
+
+Invoke a shell to execute @var{command-string}.  @var{command-string}
+must be a string.  The shell runs on the host machine, the machine
+@value{GDBN} is running on.  Returns the command's exit status.  On
+Unix systems, a command which exits with a zero exit status has
+succeeded, and non-zero exit status indicates failure.  When a command
+terminates on a fatal signal whose number is @var{N}, @value{GDBN}
+uses the value 128+@var{N} as the exit status, as is standard in Unix
+shells.  Note that @var{N} is a host signal number, not a target
+signal number.  If you're native debugging, they will be the same, but
+if cross debugging, the host vs target signal numbers may be
+completely unrelated.  Please consult your host operating system's
+documentation for the mapping between host signal numbers and signal
+names.  The shell to run is determined in the same way as for the
+@code{shell} command.  @xref{Shell Commands, ,Shell Commands}.
+
+@smallexample
+(@value{GDBP}) print $_shell("true")
+$1 = 0
+(@value{GDBP}) print $_shell("false")
+$2 = 1
+(@value{GDBP}) p $_shell("echo hello")
+hello
+$3 = 0
+(@value{GDBP}) p $_shell("foobar")
+bash: line 1: foobar: command not found
+$4 = 127
+@end smallexample
+
+This may also be useful in breakpoint conditions.  For example:
+
+@smallexample
+(@value{GDBP}) break function if $_shell("some command") == 0
+@end smallexample
+
+In this scenario, you'll want to make sure that the shell command you
+run in the breakpoint condition takes the least amount of time
+possible.  For example, avoid running a command that may block
+indefinitely, or that sleeps for a while before exiting.  Prefer a
+command or script which analyzes some state and exits immediately.
+This is important because the debugged program stops for the
+breakpoint every time, and then @value{GDBN} evaluates the breakpoint
+condition.  If the condition is false, the program is re-resumed
+transparently, without informing you of the stop.  A quick shell
+command thus avoids significantly slowing down the debugged program
+unnecessarily.
+
+Note: unlike the @code{shell} command, the @code{$_shell} convenience
+function does not affect the @code{$_shell_exitcode} and
+@code{$_shell_exitsignal} convenience variables.
+
 @end table
 
 The following functions require @value{GDBN} to be configured with
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index d0789a64401..7e73db0576a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,7 @@ set show_conv_list \
 	{$_cimag = <internal function _cimag>} \
 	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
+	{$_shell = <internal function _shell>} \
 	{$_gdb_maint_setting_str = <internal function _gdb_maint_setting_str>} \
 	{$_gdb_maint_setting = <internal function _gdb_maint_setting>} \
 	{$_gdb_setting_str = <internal function _gdb_setting_str>} \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 31cdcb41af5..ba1691ea2b0 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -41,6 +41,42 @@ if { ! [ishost *-*-mingw*] } {
     gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
 }
 
+# Test the $_shell convenience function.
+
+with_test_prefix "\$_shell convenience function" {
+    # Simple commands, check the result code.
+    gdb_test "p \$_shell(\"true\")" " = 0"
+    gdb_test "p \$_shell(\"false\")" " = 1"
+
+    # Test command with arguments.
+    gdb_test "p \$_shell(\"echo foo\")" "foo\r\n\\$${decimal} = 0"
+
+    # Check the type of the result.
+    gdb_test "ptype \$_shell(\"true\")" "type = int"
+
+    # Test passing a non-literal string as command name.
+    gdb_test "p \$cmd = \"echo bar\"" " = \"echo bar\""
+    gdb_test "p \$_shell(\$cmd)" "bar\r\n\\$${decimal} = 0"
+
+    # Test executing a non-existing command.  The result is
+    # shell-dependent, but most (all?) POSIX-like shells return 127 in
+    # this case.
+    gdb_test "p \$_shell(\"non-existing-command-foo-bar-qux\")" " = 127"
+
+    gdb_test "p \$_shell" \
+	" = <internal function _shell>"
+    gdb_test "ptype \$_shell" \
+	"type = <internal function>"
+
+    # Test error scenarios.
+    gdb_test "p \$_shell()" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(\"a\", \"b\")" \
+	"You must provide one argument for \\\$_shell\\\."
+    gdb_test "p \$_shell(1)" \
+	"Argument must be a string\\\."
+}
+
 # Define the user command "foo", used to test "pipe" command.
 gdb_test_multiple "define foo" "define foo" {
     -re "End with"  {

base-commit: 5036bde964bc1a18282dde536a95aecd0d2c08fb
-- 
2.36.0

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Mon, 13 Feb 2023 17:27:39 +0000
> 
> > OK, with the above nits.
> Would you mind taking another look?  Here's what it looks like currently:

Thanks, LGTM.

Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> From: Pedro Alves <pedro@palves.net>
Pedro> Subject: [PATCH] Add new "$_shell(CMD)" internal function
[...]

Looks good, thank you.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 4/6] Don't throw quit while handling inferior events

[Pedro Alves]

This implements what I suggested here:

 https://inbox.sourceware.org/gdb-patches/ab97c553-f406-b094-cdf3-ba031fdea925@palves.net/

Here is the current default quit_handler, a function that ends up
called by the QUIT macro:

 void
 default_quit_handler (void)
 {
   if (check_quit_flag ())
     {
       if (target_terminal::is_ours ())
 	quit ();
       else
 	target_pass_ctrlc ();
      }
 }

As we can see above, when the inferior is running in the foreground,
then a Ctrl-C is translated into a call to target_pass_ctrlc().

The target_terminal::is_ours() case above is there to handle the
scenario where GDB has the terminal, meaning it is handling some
command the user typed, like "list", or "p a + b" or some such.

However, when the inferior is running on the background, say with
"c&", GDB also has the terminal.  Run control handling is now done in
the "background".  The CLI is responsive to user commands.  If users
type Ctrl-C, they're expecting it to interrupt whatever command they
next type in the CLI, which again, could be "list", "p a + b", etc.
It's as if background run control was handled by a separate thread,
and the Ctrl-C is meant to go to the main thread, handling the CLI.

However, when handling an event, inside fetch_inferior_event &
friends, a Ctrl-C _also_ results in a Quit exception, from the same
default_quit_handler function shown above.  This quit aborts run
control handling, breakpoint condition evaluation, etc., and may even
leave run control in an inconsistent state.

The testcase added by this patch illustrates this.  The test program
just loops a number of times calling the "foo" function.

The idea is to set a breakpoint in the "foo" function with a condition
that sends SIGINT to GDB, and then evaluates to false, which results
in the program being re-resumed in the background.  The SIGINT-sending
emulates pressing Ctrl-C just while GDB was evaluating the breakpoint
condition, except, it's more deterministic.

It looks like this:

  (gdb) p $counter = 0
  $1 = 0
  (gdb) b foo if $counter++ == 10 || $_shell("kill -SIGINT `pidof gdb`") != 0
  Breakpoint 2 at 0x555555555131: file gdb.base/bg-exec-sigint-bp-cond.c, line 21.
  (gdb) c&
  Continuing.
  (gdb)

After that background continue, the breakpoint should be hit 10 times,
and we should see 10 "Quit" being printed on the screen.  As if the
user typed Ctrl-C on the prompt a number of times with no inferior
running:

  (gdb)        <<< Ctrl-C
  (gdb) Quit   <<< Ctrl-C
  (gdb) Quit   <<< Ctrl-C
  (gdb)

However, here's what you see instead:

  (gdb) c&
  Continuing.
  (gdb) Quit
  (gdb)

Just one Quit, and nothing else.  If we look at the thread's state, we see:

  (gdb) info threads
    Id   Target Id                                            Frame
  * 1    Thread 0x7ffff7d6f740 (LWP 112192) "bg-exec-sigint-" foo () at gdb.base/bg-exec-sigint-bp-cond.c:21

So the thread stopped, but we didn't report a stop...

Issuing another continue shows the same immediate-and-silent-stop:

  (gdb) c&
  Continuing.
  (gdb) Quit
  (gdb) p $counter
  $2 = 2

As mentioned, since the run control handling, and breakpoint and
watchpoint evaluation, etc. are running in the background from the
perspective of the CLI, when users type Ctrl-C in this situation,
they're thinking of aborting whatever other command they were typing
or running at the prompt, not the run control side, not the previous
"c&" command.

So I think that we should install a custom quit_handler while inside
fetch_inferior_event, where we already disable pagination and other
things for a similar reason.  This custom quit handler does nothing if
GDB has the terminal, and forwards Ctrl-C to the inferior otherwise.

With the patch implementing that, and the same testcase, here's what
you see instead:

 (gdb) p $counter = 0
 $1 = 0
 (gdb) b foo if $counter++ == 10 || $_shell("kill -SIGINT `pidof gdb`") != 0
 Breakpoint 2 at 0x555555555131: file gdb.base/bg-exec-sigint-bp-cond.c, line 21.
 (gdb) c&
 Continuing.
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb) Quit
 (gdb)
 Breakpoint 2, foo () at gdb.base/bg-exec-sigint-bp-cond.c:21
 21        return 0;

Change-Id: I1f10d99496a7d67c94b258e45963e83e439e1778
---
 gdb/infrun.c                                  | 45 +++++++++++
 .../gdb.base/bg-exec-sigint-bp-cond.c         | 33 ++++++++
 .../gdb.base/bg-exec-sigint-bp-cond.exp       | 77 +++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
 create mode 100644 gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 87ab73c47a4..f5827fd3829 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4105,6 +4105,44 @@ all_uis_on_sync_execution_starting (void)
     }
 }
 
+/* A quit_handler callback installed while we're handling inferior
+   events.  */
+
+static void
+infrun_quit_handler ()
+{
+  if (target_terminal::is_ours ())
+    {
+      /* Do nothing.
+
+	 default_quit_handler would throw a quit in this case, but if
+	 we're handling an event while we have the terminal, it means
+	 the target is running a background execution command, and
+	 thus when users press Ctrl-C, they're wanting to interrupt
+	 whatever command they were executing in the command line.
+	 E.g.:
+
+	  (gdb) c&
+	  (gdb) foo bar whatever<ctrl-c>
+
+	 That Ctrl-C should clear the input line, not interrupt event
+	 handling if it happens that the user types Ctrl-C at just the
+	 "wrong" time!
+
+	 It's as-if background event handling was handled by a
+	 separate background thread.
+
+	 To be clear, the Ctrl-C is not lost -- it will be processed
+	 by the next QUIT call once we're out of fetch_inferior_event
+	 again.  */
+    }
+  else
+    {
+      if (check_quit_flag ())
+	target_pass_ctrlc ();
+    }
+}
+
 /* Asynchronous version of wait_for_inferior.  It is called by the
    event loop whenever a change of state is detected on the file
    descriptor corresponding to the target.  It can be called more than
@@ -4133,6 +4171,13 @@ fetch_inferior_event ()
   scoped_restore save_pagination
     = make_scoped_restore (&pagination_enabled, false);
 
+  /* Install a quit handler that does nothing if we have the terminal
+     (meaning the target is running a background execution command),
+     so that Ctrl-C never interrupts GDB before the event is fully
+     handled.  */
+  scoped_restore restore_quit_handler
+    = make_scoped_restore (&quit_handler, infrun_quit_handler);
+
   /* End up with readline processing input, if necessary.  */
   {
     SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); };
diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
new file mode 100644
index 00000000000..b1cf35361f8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+foo (void)
+{
+  return 0;
+}
+
+int
+main (void)
+{
+  int i;
+
+  for (i = 0; i < 30; i++)
+    foo ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
new file mode 100644
index 00000000000..257efb337f9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
@@ -0,0 +1,77 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that sending GDB a SIGINT while handling execution control
+# does not interrupt the execution control.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Run the test.  Sets a breakpoint with a condition that sends a
+# SIGINT to GDB, and ensures that that doesn't make the breakpoint hit
+# cause a premature stop.  This emulates pressing Ctrl-C just while
+# GDB is evaluating the breakpoint condition.
+proc test {} {
+    clean_restart $::binfile
+
+    if {![runto_main]} {
+	return
+    }
+
+    delete_breakpoints
+
+    set gdb_pid [exp_pid -i [board_info host fileid]]
+
+    # Number of times the breakpoint should be hit before stopping.
+    set num_hits 10
+
+    # A counter used in the breakpoint's condition to ensure that it
+    # causes a stop after NUM_HITS hits.
+    gdb_test "p \$hit_count = 0" " = 0" "reset hit counter"
+
+    # Set a breakpoint with a condition that sends a SIGINT to GDB.  This
+    # emulates pressing Ctrl-C just while GDB is evaluating the breakpoint
+    # condition.
+    gdb_test \
+	"break foo if \$hit_count\+\+ == $num_hits || \$_shell(\"kill -SIGINT $gdb_pid\") != 0" \
+	"Breakpoint .*" \
+	"break foo if <condition>"
+
+    # Number of times we've seen GDB print "Quit" followed by the
+    # prompt.  We should see that exactly $NUM_HITS times.
+    set quit_count 0
+
+    gdb_test_multiple "c&" "SIGINT does not interrupt background execution" {
+	-re "^c&\r\nContinuing\\.\r\n$::gdb_prompt " {
+	    exp_continue
+	}
+	-re "^Quit\r\n$::gdb_prompt " {
+	    incr quit_count
+	    verbose -log "quit_count=$quit_count"
+	    exp_continue
+	}
+	-re "^\r\nBreakpoint .*return 0;" {
+	    gdb_assert {$quit_count == $num_hits} $gdb_test_name
+	}
+	-re ".*Asynchronous execution not supported on this target\..*" {
+	    unsupported "$gdb_test_name (asynchronous execution not supported)"
+	}
+    }
+}
+
+test
-- 
2.36.0

Re: [PATCH 4/6] Don't throw quit while handling inferior events

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This implements what I suggested here:
Pedro>  https://inbox.sourceware.org/gdb-patches/ab97c553-f406-b094-cdf3-ba031fdea925@palves.net/
...
Pedro> So I think that we should install a custom quit_handler while inside
Pedro> fetch_inferior_event, where we already disable pagination and other
Pedro> things for a similar reason.  This custom quit handler does nothing if
Pedro> GDB has the terminal, and forwards Ctrl-C to the inferior otherwise.

Thanks for doing this.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 5/6] GC get_active_ext_lang

[Pedro Alves]

get_active_ext_lang is not used anywhere.  Delete it.

Change-Id: I4c2b6d0d11291103c098e4db1d6ea449875c96b7
---
 gdb/extension-priv.h | 2 --
 gdb/extension.c      | 8 --------
 2 files changed, 10 deletions(-)

diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 119377b4024..23a9f646d12 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -303,8 +303,6 @@ struct active_ext_lang_state
   struct signal_handler sigint_handler;
 };
 
-extern const struct extension_language_defn *get_active_ext_lang (void);
-
 extern struct active_ext_lang_state *set_active_ext_lang
   (const struct extension_language_defn *);
 
diff --git a/gdb/extension.c b/gdb/extension.c
index 0a9803a3023..1e52afc4da2 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -647,14 +647,6 @@ static int quit_flag;
 static const struct extension_language_defn *active_ext_lang
   = &extension_language_gdb;
 
-/* Return the currently active extension language.  */
-
-const struct extension_language_defn *
-get_active_ext_lang (void)
-{
-  return active_ext_lang;
-}
-
 /* Install a SIGINT handler.  */
 
 static void
-- 
2.36.0

Re: [PATCH 5/6] GC get_active_ext_lang

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> get_active_ext_lang is not used anywhere.  Delete it.
Pedro> Change-Id: I4c2b6d0d11291103c098e4db1d6ea449875c96b7

Seems obvious.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

[PATCH 6/6] Don't throw quit while handling inferior events, part II

[Pedro Alves]

I noticed that if Ctrl-C was typed just while GDB is evaluating a
breakpoint condition in the background, and GDB ends up reaching out
to the Python interpreter, then the breakpoint condition would still
fail, like:

  c&
  Continuing.
  (gdb) Error in testing breakpoint condition:
  Quit

That happens because while evaluating the breakpoint condition, we
enter Python, and end up calling PyErr_SetInterrupt (it's called by
gdbpy_set_quit_flag, in frame #0):

 (top-gdb) bt
 #0  gdbpy_set_quit_flag (extlang=0x558c68f81900 <extension_language_python>) at ../../src/gdb/python/python.c:288
 #1  0x0000558c6845f049 in set_quit_flag () at ../../src/gdb/extension.c:785
 #2  0x0000558c6845ef98 in set_active_ext_lang (now_active=0x558c68f81900 <extension_language_python>) at ../../src/gdb/extension.c:743
 #3  0x0000558c686d3e56 in gdbpy_enter::gdbpy_enter (this=0x7fff2b70bb90, gdbarch=0x558c6ab9eac0, language=0x0) at ../../src/gdb/python/python.c:212
 #4  0x0000558c68695d49 in python_on_memory_change (inferior=0x558c6a830b00, addr=0x555555558014, len=4, data=0x558c6af8a610 "") at ../../src/gdb/python/py-inferior.c:146
 #5  0x0000558c6823a071 in std::__invoke_impl<void, void (*&)(inferior*, unsigned long, long, unsigned char const*), inferior*, unsigned long, long, unsigned char const*> (__f=@0x558c6a8ecd98: 0x558c68695d01 <python_on_memory_change(inferior*, CORE_ADDR, ssize_t, bfd_byte const*)>) at /usr/include/c++/11/bits/invoke.h:61
 #6  0x0000558c68237591 in std::__invoke_r<void, void (*&)(inferior*, unsigned long, long, unsigned char const*), inferior*, unsigned long, long, unsigned char const*> (__fn=@0x558c6a8ecd98: 0x558c68695d01 <python_on_memory_change(inferior*, CORE_ADDR, ssize_t, bfd_byte const*)>) at /usr/include/c++/11/bits/invoke.h:111
 #7  0x0000558c68233e64 in std::_Function_handler<void (inferior*, unsigned long, long, unsigned char const*), void (*)(inferior*, unsigned long, long, unsigned char const*)>::_M_invoke(std::_Any_data const&, inferior*&&, unsigned long&&, long&&, unsigned char const*&&) (__functor=..., __args#0=@0x7fff2b70bd40: 0x558c6a830b00, __args#1=@0x7fff2b70bd38: 93824992247828, __args#2=@0x7fff2b70bd30: 4, __args#3=@0x7fff2b70bd28: 0x558c6af8a610 "") at /usr/include/c++/11/bits/std_function.h:290
 #8  0x0000558c6830a96e in std::function<void (inferior*, unsigned long, long, unsigned char const*)>::operator()(inferior*, unsigned long, long, unsigned char const*) const (this=0x558c6a8ecd98, __args#0=0x558c6a830b00, __args#1=93824992247828, __args#2=4, __args#3=0x558c6af8a610 "") at /usr/include/c++/11/bits/std_function.h:590
 #9  0x0000558c6830a620 in gdb::observers::observable<inferior*, unsigned long, long, unsigned char const*>::notify (this=0x558c690828c0 <gdb::observers::memory_changed>, args#0=0x558c6a830b00, args#1=93824992247828, args#2=4, args#3=0x558c6af8a610 "") at ../../src/gdb/../gdbsupport/observable.h:166
 #10 0x0000558c68309d95 in write_memory_with_notification (memaddr=0x555555558014, myaddr=0x558c6af8a610 "", len=4) at ../../src/gdb/corefile.c:363
 #11 0x0000558c68904224 in value_assign (toval=0x558c6afce910, fromval=0x558c6afba6c0) at ../../src/gdb/valops.c:1190
 #12 0x0000558c681e3869 in expr::assign_operation::evaluate (this=0x558c6af8e150, expect_type=0x0, exp=0x558c6afcfe60, noside=EVAL_NORMAL) at ../../src/gdb/expop.h:1902
 #13 0x0000558c68450c89 in expr::logical_or_operation::evaluate (this=0x558c6afab060, expect_type=0x0, exp=0x558c6afcfe60, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:2330
 #14 0x0000558c6844a896 in expression::evaluate (this=0x558c6afcfe60, expect_type=0x0, noside=EVAL_NORMAL) at ../../src/gdb/eval.c:110
 #15 0x0000558c6844a95e in evaluate_expression (exp=0x558c6afcfe60, expect_type=0x0) at ../../src/gdb/eval.c:124
 #16 0x0000558c682061ef in breakpoint_cond_eval (exp=0x558c6afcfe60) at ../../src/gdb/breakpoint.c:4971
 ...


The fix is to disable cooperative SIGINT handling while handling
inferior events, so that SIGINT is saved in the global quit flag, and
not in the extension language, while handling an event.

This commit augments the testcase added by the previous commit to test
this scenario as well.

Change-Id: Idf8ab815774ee6f4b45ca2d0caaf30c9b9f127bb
---
 gdb/extension.c                               | 62 ++++++++++++++++++-
 gdb/extension.h                               | 16 +++++
 gdb/infrun.c                                  |  9 +++
 .../gdb.base/bg-exec-sigint-bp-cond.c         |  2 +
 .../gdb.base/bg-exec-sigint-bp-cond.exp       | 27 +++++++-
 5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/gdb/extension.c b/gdb/extension.c
index 1e52afc4da2..4ac6e0b6732 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -681,6 +681,35 @@ void (*hook_set_active_ext_lang) () = nullptr;
 }
 #endif
 
+/* True if cooperative SIGINT handling is disabled.  This is needed so
+   that calls to set_active_ext_lang do not re-enable cooperative
+   handling, which if enabled would make set_quit_flag store the
+   SIGINT in an extension language.  */
+static bool cooperative_sigint_handling_disabled = false;
+
+scoped_disable_cooperative_sigint_handling::scoped_disable_cooperative_sigint_handling ()
+{
+  /* Force the active extension language to the GDB scripting
+     language.  This ensures that a previously saved SIGINT is moved
+     to the quit_flag global, as well as ensures that future SIGINTs
+     are also saved in the global.  */
+  m_prev_active_ext_lang_state
+    = set_active_ext_lang (&extension_language_gdb);
+
+  /* Set the "cooperative SIGINT handling disabled" global flag, so
+     that a future call to set_active_ext_lang does not re-enable
+     cooperative mode.  */
+  m_prev_cooperative_sigint_handling_disabled
+    = cooperative_sigint_handling_disabled;
+  cooperative_sigint_handling_disabled = true;
+}
+
+scoped_disable_cooperative_sigint_handling::~scoped_disable_cooperative_sigint_handling ()
+{
+  cooperative_sigint_handling_disabled = m_prev_cooperative_sigint_handling_disabled;
+  restore_active_ext_lang (m_prev_active_ext_lang_state);
+}
+
 /* Set the currently active extension language to NOW_ACTIVE.
    The result is a pointer to a malloc'd block of memory to pass to
    restore_active_ext_lang.
@@ -702,7 +731,15 @@ void (*hook_set_active_ext_lang) () = nullptr;
    check_quit_flag is not called, the original SIGINT will be thrown.
    Non-cooperative extension languages are free to install their own SIGINT
    handler but the original must be restored upon return, either itself
-   or via restore_active_ext_lang.  */
+   or via restore_active_ext_lang.
+
+   If cooperative SIGINT handling is force-disabled (e.g., we're in
+   the middle of handling an inferior event), then we don't actually
+   record NOW_ACTIVE as the current active extension language, so that
+   set_quit_flag saves the SIGINT in the global quit flag instead of
+   in the extension language.  The caller does not need to concern
+   itself about this, though.  The currently active extension language
+   concept only exists for cooperative SIGINT handling.  */
 
 struct active_ext_lang_state *
 set_active_ext_lang (const struct extension_language_defn *now_active)
@@ -712,6 +749,22 @@ set_active_ext_lang (const struct extension_language_defn *now_active)
     selftests::hook_set_active_ext_lang ();
 #endif
 
+  /* If cooperative SIGINT handling was previously force-disabled,
+     make sure to not re-enable it (as NOW_ACTIVE could be a language
+     that supports cooperative SIGINT handling).  */
+  if (cooperative_sigint_handling_disabled)
+    {
+      /* Ensure set_quit_flag saves SIGINT in the quit_flag
+	 global.  */
+      gdb_assert (active_ext_lang->ops == nullptr
+		  || active_ext_lang->ops->check_quit_flag == nullptr);
+
+      /* The only thing the caller can do with the result is pass it
+	 to restore_active_ext_lang, which expects NULL when
+	 cooperative SIGINT handling is disabled.  */
+      return nullptr;
+    }
+
   struct active_ext_lang_state *previous
     = XCNEW (struct active_ext_lang_state);
 
@@ -743,6 +796,13 @@ set_active_ext_lang (const struct extension_language_defn *now_active)
 void
 restore_active_ext_lang (struct active_ext_lang_state *previous)
 {
+  if (cooperative_sigint_handling_disabled)
+    {
+      /* See set_active_ext_lang.  */
+      gdb_assert (previous == nullptr);
+      return;
+    }
+
   active_ext_lang = previous->ext_lang;
 
   if (target_terminal::is_ours ())
diff --git a/gdb/extension.h b/gdb/extension.h
index c7d1df2629f..ab83f9c6a28 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -343,4 +343,20 @@ extern void (*hook_set_active_ext_lang) ();
 }
 #endif
 
+/* Temporarily disable cooperative SIGINT handling.  Needed when we
+   don't want a SIGINT to interrupt the currently active extension
+   language.  */
+class scoped_disable_cooperative_sigint_handling
+{
+public:
+  scoped_disable_cooperative_sigint_handling ();
+  ~scoped_disable_cooperative_sigint_handling ();
+
+  DISABLE_COPY_AND_ASSIGN (scoped_disable_cooperative_sigint_handling);
+
+private:
+  struct active_ext_lang_state *m_prev_active_ext_lang_state;
+  bool m_prev_cooperative_sigint_handling_disabled;
+};
+
 #endif /* EXTENSION_H */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f5827fd3829..b51d7d73ef4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -73,6 +73,7 @@
 #include "test-target.h"
 #include "gdbsupport/common-debug.h"
 #include "gdbsupport/buildargv.h"
+#include "extension.h"
 
 /* Prototypes for local functions */
 
@@ -4178,6 +4179,14 @@ fetch_inferior_event ()
   scoped_restore restore_quit_handler
     = make_scoped_restore (&quit_handler, infrun_quit_handler);
 
+  /* Make sure a SIGINT does not interrupt an extension language while
+     we're handling an event.  That could interrupt a Python unwinder
+     or a Python observer or some such.  A Ctrl-C should either be
+     forwarded to the inferior if the inferior has the terminal, or,
+     if GDB has the terminal, should interrupt the command the user is
+     typing in the CLI.  */
+  scoped_disable_cooperative_sigint_handling restore_coop_sigint;
+
   /* End up with readline processing input, if necessary.  */
   {
     SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); };
diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
index b1cf35361f8..df418ddb18d 100644
--- a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
+++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.c
@@ -15,6 +15,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+int global;
+
 int
 foo (void)
 {
diff --git a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
index 257efb337f9..a8764a4e5ea 100644
--- a/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
+++ b/gdb/testsuite/gdb.base/bg-exec-sigint-bp-cond.exp
@@ -26,7 +26,10 @@ if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
 # SIGINT to GDB, and ensures that that doesn't make the breakpoint hit
 # cause a premature stop.  This emulates pressing Ctrl-C just while
 # GDB is evaluating the breakpoint condition.
-proc test {} {
+#
+# AFTER_KILL_COND is appended to the breakpoint condition, after "kill
+# -SIGINT $gdb_pid".
+proc test { {after_kill_cond ""} } {
     clean_restart $::binfile
 
     if {![runto_main]} {
@@ -48,7 +51,7 @@ proc test {} {
     # emulates pressing Ctrl-C just while GDB is evaluating the breakpoint
     # condition.
     gdb_test \
-	"break foo if \$hit_count\+\+ == $num_hits || \$_shell(\"kill -SIGINT $gdb_pid\") != 0" \
+	"break foo if \$hit_count\+\+ == $num_hits || \$_shell(\"kill -SIGINT $gdb_pid\") != 0 $after_kill_cond" \
 	"Breakpoint .*" \
 	"break foo if <condition>"
 
@@ -74,4 +77,22 @@ proc test {} {
     }
 }
 
-test
+# Test without writing to memory after killing GDB.  This does not
+# take any Python path at the time of writing.
+with_test_prefix "no force memory write" {
+    test
+}
+
+# Writing to memory from the condition makes GDB enter Python for
+# reporting a memory changed event.  Thus this tests that GDB doesn't
+# forward the SIGINT to Python, interrupting Python, causing the
+# breakpoint to prematurely stop like:
+#
+#  c&
+#  Continuing.
+#  (gdb) Error in testing breakpoint condition:
+#  Quit
+#
+with_test_prefix "force memory write" {
+    test " || (global = 0)"
+}
-- 
2.36.0

Re: [PATCH 6/6] Don't throw quit while handling inferior events, part II

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> I noticed that if Ctrl-C was typed just while GDB is evaluating a
Pedro> breakpoint condition in the background, and GDB ends up reaching out
Pedro> to the Python interpreter, then the breakpoint condition would still
Pedro> fail, like:

Pedro> The fix is to disable cooperative SIGINT handling while handling
Pedro> inferior events, so that SIGINT is saved in the global quit flag, and
Pedro> not in the extension language, while handling an event.

I wonder if there's an open PR about this.
It sounds vaguely familiar.

Thanks for doing this, and for the very clear comments.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 6/6] Don't throw quit while handling inferior events, part II

[Pedro Alves]

On 2023-02-14 3:54 p.m., Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> I noticed that if Ctrl-C was typed just while GDB is evaluating a
> Pedro> breakpoint condition in the background, and GDB ends up reaching out
> Pedro> to the Python interpreter, then the breakpoint condition would still
> Pedro> fail, like:
> 
> Pedro> The fix is to disable cooperative SIGINT handling while handling
> Pedro> inferior events, so that SIGINT is saved in the global quit flag, and
> Pedro> not in the extension language, while handling an event.
> 
> I wonder if there's an open PR about this.
> It sounds vaguely familiar.
> 

Hmm, I didn't see it the first time, but looking again, this one:

 https://sourceware.org/bugzilla/show_bug.cgi?id=26266

I think ends up fixed by the series, since normal_stop is called from
within fetch_inferior_events, while the quit_handler and cooperative
sigint handling are hijacked.

I won't close that PR as in the linked discussion Andrew suggested
a good idea for a testcase.

Re: [PATCH 6/6] Don't throw quit while handling inferior events, part II

[Pedro Alves]

On 2023-02-15 9:16 p.m., Pedro Alves wrote:
> On 2023-02-14 3:54 p.m., Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>>
>> Pedro> I noticed that if Ctrl-C was typed just while GDB is evaluating a
>> Pedro> breakpoint condition in the background, and GDB ends up reaching out
>> Pedro> to the Python interpreter, then the breakpoint condition would still
>> Pedro> fail, like:
>>
>> Pedro> The fix is to disable cooperative SIGINT handling while handling
>> Pedro> inferior events, so that SIGINT is saved in the global quit flag, and
>> Pedro> not in the extension language, while handling an event.
>>
>> I wonder if there's an open PR about this.
>> It sounds vaguely familiar.
>>
> 
> Hmm, I didn't see it the first time, but looking again, this one:
> 
>  https://sourceware.org/bugzilla/show_bug.cgi?id=26266
> 
> I think ends up fixed by the series, since normal_stop is called from
> within fetch_inferior_events, while the quit_handler and cooperative
> sigint handling are hijacked.
> 
> I won't close that PR as in the linked discussion Andrew suggested
> a good idea for a testcase.
> 

Eh, and this one, in this comment:

   https://sourceware.org/bugzilla/show_bug.cgi?id=15297#c6

... my older self touches on the problem that this patch fixes.  I had
completely forgotten that.