[PATCH 2/4] gdb/remote: better management of remote_state::starting_up flag
Andrew Burgess
andrew.burgess@embecosm.com
Wed Jun 9 20:06:15 GMT 2021
Use scoped_restore to ensure the starting_up flag is always reset to
false when leaving remote_target::start_remote.
There was no bug that inspired this change, rather, while looking at
how this flag was used I spotted that there are a couple of error()
calls in remote_target::start_remote, which would leave GDB with the
remote_state::starting_up flag set to true, which doesn't seem right.
At the end of remote_target::start_remote we were previously setting
remote_state::starting_up back to false before the end of the
function, I've retained this behaviour for now, it should be harmless
to set the starting_up flag back to false and then (later) have the
scoped_restore also set the flag back to false.
I don't believe there should be any user visible changes after this
commit. In the non-error case I think this is obviously true, the
starting_up flag was set to true on entry to ::start_remote, and was
set back to false before we returned, this is still the case.
In the error () case we would previously leave the ::starting_up flag
set to true, however, this only effected the Ctrl-C handling, thread
creation, or tracepoint download. As throwing an error during
::start_remote would cause the remote_target to be popped from the
target stack, we would never enter any of the code that checked the
::starting_up flag. And so, I claim, we will not see any changes in
the error () case either.
gdb/ChangeLog:
* remote.c (remote_target::start_remote): Use scoped_restore to
manage remote_state::starting_up flag.
---
gdb/ChangeLog | 5 +++++
gdb/remote.c | 13 +++++++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 531d43a692b..97268151a59 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4667,7 +4667,9 @@ remote_target::start_remote (int from_tty, int extended_p)
Ctrl-C before we're connected and synced up can't interrupt the
target. Instead, it offers to drop the (potentially wedged)
connection. */
- rs->starting_up = true;
+ gdb_assert (!rs->starting_up);
+ scoped_restore starting_up_restore
+ = make_scoped_restore (&rs->starting_up, true);
QUIT;
@@ -4808,7 +4810,6 @@ remote_target::start_remote (int from_tty, int extended_p)
/* We're connected, but not running. Drop out before we
call start_remote. */
- rs->starting_up = false;
return;
}
else
@@ -4923,7 +4924,6 @@ remote_target::start_remote (int from_tty, int extended_p)
/* We're connected, but not running. Drop out before we
call start_remote. */
- rs->starting_up = false;
return;
}
@@ -4967,7 +4967,12 @@ remote_target::start_remote (int from_tty, int extended_p)
/* The thread and inferior lists are now synchronized with the
target, our symbols have been relocated, and we're merged the
target's tracepoints with ours. We're done with basic start
- up. */
+ up.
+
+ We manually set the starting_up flag here despite having
+ STARTING_UP_RESTORE which will do this for us at the end of our
+ scope, now we are fully connected we want things like Ctrl-C handling
+ to behave as normal during the following code. */
rs->starting_up = false;
/* Maybe breakpoints are global and need to be inserted now. */
--
2.25.4
More information about the Gdb-patches
mailing list