OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

File I/O file handles after target closes

Browse Source 
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  <palves@redhat.com>

	* 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.
This commit is contained in:
Pedro Alves 2018-04-11 11:35:58 +01:00
parent 5ff79300ae
commit 20db9c52a2
3 changed files with 40 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
gdb/ChangeLog
View File

@ -1,3 +1,13 @@
2018-04-11 Pedro Alves <palves@redhat.com>
* 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.
2018-04-11 Pedro Alves <palves@redhat.com>
* target.c (fileio_fh_t): Make it a named struct instead of a

3
gdb/remote.c
View File

@ -11350,8 +11350,7 @@ remote_hostio_send_command (int command_bytes, int which_packet,
int ret, bytes_read;
char *attachment_tmp;
if (!rs->remote_desc
|| packet_support (which_packet) == PACKET_DISABLE)
if (packet_support (which_packet) == PACKET_DISABLE)
{
*remote_errno = FILEIO_ENOSYS;
return -1;

28
gdb/target.c
View File

@ -2793,7 +2793,8 @@ default_fileio_target (void)
struct fileio_fh_t
{
/* The target on which this file is open. */
/* The target on which this file is open. NULL if the target is
meanwhile closed while the handle is open. */
target_ops *target;
/* The file descriptor on the target. */
@ -2818,6 +2819,20 @@ static std::vector<fileio_fh_t> fileio_fhandles;
list each time a new file is opened. */
static int lowest_closed_fd;
/* Invalidate the target associated with open handles that were open
on target TARG, since we're about to close (and maybe destroy) the
target. The handles remain open from the client's perspective, but
trying to do anything with them other than closing them will fail
with EIO. */
static void
fileio_handles_invalidate_target (target_ops *targ)
{
for (fileio_fh_t &fh : fileio_fhandles)
if (fh.target == targ)
fh.target = NULL;
}
/* Acquire a target fileio file descriptor. */
static int
@ -2933,6 +2948,8 @@ target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
if (fh->is_closed ())
*target_errno = EBADF;
else if (fh->target == NULL)
*target_errno = EIO;
else
ret = fh->target->to_fileio_pwrite (fh->target, fh->target_fd, write_buf,
len, offset, target_errno);
@ -2957,6 +2974,8 @@ target_fileio_pread (int fd, gdb_byte *read_buf, int len,
if (fh->is_closed ())
*target_errno = EBADF;
else if (fh->target == NULL)
*target_errno = EIO;
else
ret = fh->target->to_fileio_pread (fh->target, fh->target_fd, read_buf,
len, offset, target_errno);
@ -2980,6 +2999,8 @@ target_fileio_fstat (int fd, struct stat *sb, int *target_errno)
if (fh->is_closed ())
*target_errno = EBADF;
else if (fh->target == NULL)
*target_errno = EIO;
else
ret = fh->target->to_fileio_fstat (fh->target, fh->target_fd,
sb, target_errno);
@ -3003,8 +3024,11 @@ target_fileio_close (int fd, int *target_errno)
*target_errno = EBADF;
else
{
if (fh->target != NULL)
ret = fh->target->to_fileio_close (fh->target, fh->target_fd,
target_errno);
else
ret = 0;
release_fileio_fd (fd, fh);
}
@ -3390,6 +3414,8 @@ target_close (struct target_ops *targ)
{
gdb_assert (!target_is_pushed (targ));
fileio_handles_invalidate_target (targ);
if (targ->to_xclose != NULL)
targ->to_xclose (targ);
else if (targ->to_close != NULL)