Skip to content

Commit 1f01056

Browse files
committed
GDBserver: Don't assume a current process in D;PID implementation (PR gdb/23377)
This fixes a gdb.base/multi-forks.exp regression with GDBserver. Git commit f2ffa92 ("gdb: Eliminate the 'stop_pc' global") caused the regression by exposing a latent bug in gdbserver. The bug is that GDBserver's implementation of the D;PID packet incorrectly assumes that the selected thread points to the process being detached. This happens via the any_persistent_commands call, which calls current_process: (gdb) bt #0 0x000000000040a57e in internal_error(char const*, int, char const*, ...) (file=0x4a53c0 "src/gdb/gdbserver/inferiors.c", line=212, fmt=0x4a539e "%s: Assertion `%s' failed.") at src/gdb/gdbserver/../common/errors.c:54 #1 0x0000000000420acf in current_process() () at src/gdb/gdbserver/inferiors.c:212 #2 0x00000000004226a0 in any_persistent_commands() () at gdb/gdbserver/mem-break.c:308 #3 0x000000000042cb43 in handle_detach(char*) (own_buf=0x6f0280 "D;62ea") at src/gdb/gdbserver/server.c:1210 #4 0x0000000000433af3 in process_serial_event() () at src/gdb/gdbserver/server.c:4055 #5 0x0000000000434878 in handle_serial_event(int, void*) (err=0, client_data=0x0) The "eliminate stop_pc" commit exposes the problem because before that commit, GDB's switch_to_thread always read the newly-selected thread's PC, and that would end up forcing GDBserver's selected thread to change accordingly as side effect. After that commit, GDB no longer reads the thread's PC, and GDBserver does not switch the thread. Fix this by removing the assumption from GDBserver. gdb/gdbserver/ChangeLog: 2018-07-11 Pedro Alves <[email protected]> PR gdb/23377 * mem-break.c (any_persistent_commands): Add process_info parameter and use it instead of relying on the current process. Change return type to bool. * mem-break.h (any_persistent_commands): Add process_info parameter and change return type to bool. * server.c (handle_detach): Remove require_running_or_return call. Look up the process_info for the process we're about to detach. If not found, return back error to GDB. Adjust any_persistent_commands call to pass down a process pointer.
1 parent 82e080d commit 1f01056

File tree

4 files changed

+40
-24
lines changed

4 files changed

+40
-24
lines changed

gdb/gdbserver/ChangeLog

+13
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2018-07-11 Pedro Alves <[email protected]>
2+
3+
PR gdb/23377
4+
* mem-break.c (any_persistent_commands): Add process_info
5+
parameter and use it instead of relying on the current process.
6+
Change return type to bool.
7+
* mem-break.h (any_persistent_commands): Add process_info
8+
parameter and change return type to bool.
9+
* server.c (handle_detach): Remove require_running_or_return call.
10+
Look up the process_info for the process we're about to detach.
11+
If not found, return back error to GDB. Adjust
12+
any_persistent_commands call to pass down a process pointer.
13+
114
2018-07-11 Pedro Alves <[email protected]>
215

316
* i387-fp.c (i387_cache_to_fsave, cache_to_fxsave)

gdb/gdbserver/mem-break.c

+4-5
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,9 @@ is_gdb_breakpoint (enum bkpt_type type)
302302
|| type == gdb_breakpoint_Z4);
303303
}
304304

305-
int
306-
any_persistent_commands (void)
305+
bool
306+
any_persistent_commands (process_info *proc)
307307
{
308-
struct process_info *proc = current_process ();
309308
struct breakpoint *bp;
310309
struct point_command_list *cl;
311310

@@ -317,11 +316,11 @@ any_persistent_commands (void)
317316

318317
for (cl = gdb_bp->command_list; cl != NULL; cl = cl->next)
319318
if (cl->persistence)
320-
return 1;
319+
return true;
321320
}
322321
}
323322

324-
return 0;
323+
return false;
325324
}
326325

327326
/* Find low-level breakpoint of type TYPE at address ADDR that is not

gdb/gdbserver/mem-break.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ int add_breakpoint_condition (struct gdb_breakpoint *bp,
125125
int add_breakpoint_commands (struct gdb_breakpoint *bp, const char **commands,
126126
int persist);
127127

128-
int any_persistent_commands (void);
128+
/* Return true if PROC has any persistent command. */
129+
bool any_persistent_commands (process_info *proc);
129130

130131
/* Evaluation condition (if any) at breakpoint BP. Return 1 if
131132
true and 0 otherwise. */

gdb/gdbserver/server.c

+21-18
Original file line numberDiff line numberDiff line change
@@ -1195,34 +1195,37 @@ static void
11951195
handle_detach (char *own_buf)
11961196
{
11971197
client_state &cs = get_client_state ();
1198-
require_running_or_return (own_buf);
11991198

1200-
int pid;
1199+
process_info *process;
12011200

12021201
if (cs.multi_process)
12031202
{
12041203
/* skip 'D;' */
1205-
pid = strtol (&own_buf[2], NULL, 16);
1204+
int pid = strtol (&own_buf[2], NULL, 16);
1205+
1206+
process = find_process_pid (pid);
12061207
}
12071208
else
1208-
pid = current_ptid.pid ();
1209-
1210-
if ((tracing && disconnected_tracing) || any_persistent_commands ())
12111209
{
1212-
struct process_info *process = find_process_pid (pid);
1210+
process = (current_thread != nullptr
1211+
? get_thread_process (current_thread)
1212+
: nullptr);
1213+
}
12131214

1214-
if (process == NULL)
1215-
{
1216-
write_enn (own_buf);
1217-
return;
1218-
}
1215+
if (process == NULL)
1216+
{
1217+
write_enn (own_buf);
1218+
return;
1219+
}
12191220

1221+
if ((tracing && disconnected_tracing) || any_persistent_commands (process))
1222+
{
12201223
if (tracing && disconnected_tracing)
12211224
fprintf (stderr,
12221225
"Disconnected tracing in effect, "
12231226
"leaving gdbserver attached to the process\n");
12241227

1225-
if (any_persistent_commands ())
1228+
if (any_persistent_commands (process))
12261229
fprintf (stderr,
12271230
"Persistent commands are present, "
12281231
"leaving gdbserver attached to the process\n");
@@ -1250,13 +1253,13 @@ handle_detach (char *own_buf)
12501253
return;
12511254
}
12521255

1253-
fprintf (stderr, "Detaching from process %d\n", pid);
1256+
fprintf (stderr, "Detaching from process %d\n", process->pid);
12541257
stop_tracing ();
1255-
if (detach_inferior (pid) != 0)
1258+
if (detach_inferior (process->pid) != 0)
12561259
write_enn (own_buf);
12571260
else
12581261
{
1259-
discard_queued_stop_replies (ptid_t (pid));
1262+
discard_queued_stop_replies (ptid_t (process->pid));
12601263
write_ok (own_buf);
12611264

12621265
if (extended_protocol || target_running ())
@@ -1266,7 +1269,7 @@ handle_detach (char *own_buf)
12661269
and instead treat this like a normal program exit. */
12671270
cs.last_status.kind = TARGET_WAITKIND_EXITED;
12681271
cs.last_status.value.integer = 0;
1269-
cs.last_ptid = ptid_t (pid);
1272+
cs.last_ptid = ptid_t (process->pid);
12701273

12711274
current_thread = NULL;
12721275
}
@@ -1278,7 +1281,7 @@ handle_detach (char *own_buf)
12781281
/* If we are attached, then we can exit. Otherwise, we
12791282
need to hang around doing nothing, until the child is
12801283
gone. */
1281-
join_inferior (pid);
1284+
join_inferior (process->pid);
12821285
exit (0);
12831286
}
12841287
}

0 commit comments

Comments
 (0)