Skip to content
This repository was archived by the owner on Aug 17, 2022. It is now read-only.

Commit 20db9c5

Browse files
committed
File I/O file handles after target closes
A future patch will propose making the remote target's target_ops be heap-allocated (to make it possible to have multiple instances of remote targets, for multiple simultaneous connections), and will delete/destroy the remote target at target_close time. That change trips on a latent problem, though. File I/O handles remain open even after the target is gone, with a dangling pointer to a target that no longer exists. This results in GDB crashing when it calls the target_ops backend associated with the file handle: (gdb) Disconnect Ending remote debugging. * GDB crashes deferencing a dangling pointer Backtrace: #0 0x00007f79338570a0 in main_arena () at /lib64/libc.so.6 #1 0x0000000000858bfe in target_fileio_close(int, int*) (fd=1, target_errno=0x7ffe0499a4c8) at src/gdb/target.c:2980 #2 0x00000000007088bd in gdb_bfd_iovec_fileio_close(bfd*, void*) (abfd=0x1a631b0, stream=0x223c9d0) at src/gdb/gdb_bfd.c:353 #3 0x0000000000930906 in opncls_bclose (abfd=0x1a631b0) at src/bfd/opncls.c:528 #4 0x0000000000930cf9 in bfd_close_all_done (abfd=0x1a631b0) at src/bfd/opncls.c:768 #5 0x0000000000930cb3 in bfd_close (abfd=0x1a631b0) at src/bfd/opncls.c:735 #6 0x0000000000708dc5 in gdb_bfd_close_or_warn(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:511 #7 0x00000000007091a2 in gdb_bfd_unref(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:615 #8 0x000000000079ed8e in objfile::~objfile() (this=0x2154730, __in_chrg=<optimized out>) at src/gdb/objfiles.c:682 #9 0x000000000079fd1a in objfile_purge_solibs() () at src/gdb/objfiles.c:1065 #10 0x00000000008162ca in no_shared_libraries(char const*, int) (ignored=0x0, from_tty=1) at src/gdb/solib.c:1251 #11 0x000000000073b89b in disconnect_command(char const*, int) (args=0x0, from_tty=1) at src/gdb/infcmd.c:3035 This goes unnoticed in current master, because the current remote target's target_ops is never destroyed nowadays, so we end up calling: remote_hostio_close -> remote_hostio_send_command which gracefully fails with FILEIO_ENOSYS if remote_desc is NULL (because the target is closed). Fix this by invalidating a target's file I/O handles when the target is closed. With this change, remote_hostio_send_command no longer needs to handle the case of being called with a closed remote target, originally added here: <https://sourceware.org/ml/gdb-patches/2008-08/msg00359.html>. gdb/ChangeLog: 2018-04-11 Pedro Alves <[email protected]> * target.c (fileio_fh_t::t): Add comment. (target_fileio_pwrite, target_fileio_pread, target_fileio_fstat) (target_fileio_close): Handle a NULL target. (invalidate_fileio_fh): New. (target_close): Call it. * remote.c (remote_hostio_send_command): No longer check whether remote_desc is open.
1 parent 5ff7930 commit 20db9c5

File tree

3 files changed

+40
-5
lines changed

3 files changed

+40
-5
lines changed

gdb/ChangeLog

+10
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2018-04-11 Pedro Alves <[email protected]>
2+
3+
* target.c (fileio_fh_t::t): Add comment.
4+
(target_fileio_pwrite, target_fileio_pread, target_fileio_fstat)
5+
(target_fileio_close): Handle a NULL target.
6+
(invalidate_fileio_fh): New.
7+
(target_close): Call it.
8+
* remote.c (remote_hostio_send_command): No longer check whether
9+
remote_desc is open.
10+
111
2018-04-11 Pedro Alves <[email protected]>
212

313
* target.c (fileio_fh_t): Make it a named struct instead of a

gdb/remote.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -11350,8 +11350,7 @@ remote_hostio_send_command (int command_bytes, int which_packet,
1135011350
int ret, bytes_read;
1135111351
char *attachment_tmp;
1135211352

11353-
if (!rs->remote_desc
11354-
|| packet_support (which_packet) == PACKET_DISABLE)
11353+
if (packet_support (which_packet) == PACKET_DISABLE)
1135511354
{
1135611355
*remote_errno = FILEIO_ENOSYS;
1135711356
return -1;

gdb/target.c

+29-3
Original file line numberDiff line numberDiff line change
@@ -2793,7 +2793,8 @@ default_fileio_target (void)
27932793

27942794
struct fileio_fh_t
27952795
{
2796-
/* The target on which this file is open. */
2796+
/* The target on which this file is open. NULL if the target is
2797+
meanwhile closed while the handle is open. */
27972798
target_ops *target;
27982799

27992800
/* The file descriptor on the target. */
@@ -2818,6 +2819,20 @@ static std::vector<fileio_fh_t> fileio_fhandles;
28182819
list each time a new file is opened. */
28192820
static int lowest_closed_fd;
28202821

2822+
/* Invalidate the target associated with open handles that were open
2823+
on target TARG, since we're about to close (and maybe destroy) the
2824+
target. The handles remain open from the client's perspective, but
2825+
trying to do anything with them other than closing them will fail
2826+
with EIO. */
2827+
2828+
static void
2829+
fileio_handles_invalidate_target (target_ops *targ)
2830+
{
2831+
for (fileio_fh_t &fh : fileio_fhandles)
2832+
if (fh.target == targ)
2833+
fh.target = NULL;
2834+
}
2835+
28212836
/* Acquire a target fileio file descriptor. */
28222837

28232838
static int
@@ -2933,6 +2948,8 @@ target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
29332948

29342949
if (fh->is_closed ())
29352950
*target_errno = EBADF;
2951+
else if (fh->target == NULL)
2952+
*target_errno = EIO;
29362953
else
29372954
ret = fh->target->to_fileio_pwrite (fh->target, fh->target_fd, write_buf,
29382955
len, offset, target_errno);
@@ -2957,6 +2974,8 @@ target_fileio_pread (int fd, gdb_byte *read_buf, int len,
29572974

29582975
if (fh->is_closed ())
29592976
*target_errno = EBADF;
2977+
else if (fh->target == NULL)
2978+
*target_errno = EIO;
29602979
else
29612980
ret = fh->target->to_fileio_pread (fh->target, fh->target_fd, read_buf,
29622981
len, offset, target_errno);
@@ -2980,6 +2999,8 @@ target_fileio_fstat (int fd, struct stat *sb, int *target_errno)
29802999

29813000
if (fh->is_closed ())
29823001
*target_errno = EBADF;
3002+
else if (fh->target == NULL)
3003+
*target_errno = EIO;
29833004
else
29843005
ret = fh->target->to_fileio_fstat (fh->target, fh->target_fd,
29853006
sb, target_errno);
@@ -3003,8 +3024,11 @@ target_fileio_close (int fd, int *target_errno)
30033024
*target_errno = EBADF;
30043025
else
30053026
{
3006-
ret = fh->target->to_fileio_close (fh->target, fh->target_fd,
3007-
target_errno);
3027+
if (fh->target != NULL)
3028+
ret = fh->target->to_fileio_close (fh->target, fh->target_fd,
3029+
target_errno);
3030+
else
3031+
ret = 0;
30083032
release_fileio_fd (fd, fh);
30093033
}
30103034

@@ -3390,6 +3414,8 @@ target_close (struct target_ops *targ)
33903414
{
33913415
gdb_assert (!target_is_pushed (targ));
33923416

3417+
fileio_handles_invalidate_target (targ);
3418+
33933419
if (targ->to_xclose != NULL)
33943420
targ->to_xclose (targ);
33953421
else if (targ->to_close != NULL)

0 commit comments

Comments
 (0)