[PATCH] Error when gdb_is_target_1 is called without running gdb instance

[PATCH] Error when gdb_is_target_1 is called without running gdb instance

[Keith Seitz]

This is a snafu that I encountered while implementing the previous
patch, which attempted to use gdb_is_target_native.  This proc and
gdb_is_target_remote both rely on gdb_is_target_1, which actually
cannot be called without gdb already running.

This patch adds appropriate warning comments to these procs and
causes gdb_is_target_1 to issue a Tcl error if it is called without a
gdb instance already running.  This should prevent unwitting callers
from using this at the wrong time.
---
 gdb/testsuite/lib/gdb.exp | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0cec46731bb..ca8eba25417 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3872,8 +3872,17 @@ proc skip_compile_feature_tests {} {
 # is a regexp that will match the output of "maint print target-stack" if
 # the target in question is currently pushed.  PROMPT_REGEXP is a regexp
 # matching the expected prompt after the command output.
+#
+# NOTE: GDB must be running BEFORE this procedure is called!
 
 proc gdb_is_target_1 { target_name target_stack_regexp prompt_regexp } {
+    global gdb_spawn_id
+
+    # Throw a Tcl error if gdb isn't already started.
+    if {![info exists gdb_spawn_id]} {
+	error "gdb_is_target_1 called with no running gdb instance"
+    }
+
     set test "probe for target ${target_name}"
     gdb_test_multiple "maint print target-stack" $test \
 	-prompt "$prompt_regexp" {
@@ -3889,6 +3898,8 @@ proc gdb_is_target_1 { target_name target_stack_regexp prompt_regexp } {
 }
 
 # Helper for gdb_is_target_remote where the expected prompt is variable.
+#
+# NOTE: GDB must be running BEFORE this procedure is called!
 
 proc gdb_is_target_remote_prompt { prompt_regexp } {
     return [gdb_is_target_1 "remote" ".*emote target using gdb-specific protocol.*" $prompt_regexp]
@@ -3896,6 +3907,8 @@ proc gdb_is_target_remote_prompt { prompt_regexp } {
 
 # Check whether we're testing with the remote or extended-remote
 # targets.
+#
+# NOTE: GDB must be running BEFORE this procedure is called!
 
 proc gdb_is_target_remote { } {
     global gdb_prompt
@@ -3904,6 +3917,8 @@ proc gdb_is_target_remote { } {
 }
 
 # Check whether we're testing with the native target.
+#
+# NOTE: GDB must be running BEFORE this procedure is called!
 
 proc gdb_is_target_native { } {
     global gdb_prompt
-- 
2.34.1

Re: [PATCH] Error when gdb_is_target_1 is called without running gdb instance

[Tom Tromey]

>>>>> "Keith" == Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:

Keith> This is a snafu that I encountered while implementing the previous
Keith> patch, which attempted to use gdb_is_target_native.  This proc and
Keith> gdb_is_target_remote both rely on gdb_is_target_1, which actually
Keith> cannot be called without gdb already running.

Keith> This patch adds appropriate warning comments to these procs and
Keith> causes gdb_is_target_1 to issue a Tcl error if it is called without a
Keith> gdb instance already running.  This should prevent unwitting callers
Keith> from using this at the wrong time.

Looks good.

This is one thing I really like about the mythical internal AdaCore test
suite: in it, when you start gdb, you get an object back, and so
something like this would be a method on the object -- impossible to
call without it.

Tom

Re: [PATCH] Error when gdb_is_target_1 is called without running gdb instance

[Keith Seitz]

On 2/25/22 08:44, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Keith> This patch adds appropriate warning comments to these procs and
> Keith> causes gdb_is_target_1 to issue a Tcl error if it is called without a
> Keith> gdb instance already running.  This should prevent unwitting callers
> Keith> from using this at the wrong time.
> 
> Looks good.
> 
> This is one thing I really like about the mythical internal AdaCore test
> suite: in it, when you start gdb, you get an object back, and so
> something like this would be a method on the object -- impossible to
> call without it.

I think that's a great way to deal with issues like this. I wonder if
we could piecemeal implement a holistic change such as this, though?
Given the size of our test suite, that would be a *lot* of auditing!

 From where I sit, though, there are "bigger fish" to go after. :-(

Thank you for the review, I've pushed this change.

Keith

Re: [PATCH] Error when gdb_is_target_1 is called without running gdb instance

[Tom Tromey]

>> This is one thing I really like about the mythical internal AdaCore
>> test suite: in it, when you start gdb, you get an object back, and so
>> something like this would be a method on the object -- impossible to
>> call without it.

Keith> I think that's a great way to deal with issues like this. I wonder if
Keith> we could piecemeal implement a holistic change such as this, though?
Keith> Given the size of our test suite, that would be a *lot* of auditing!

Yeah, it's hard to imagine actually doing it.

Tom