On Tue, 17 Nov 2015 16:05:40 -0500
Mike Frysinger <vapier@gentoo.org> wrote:
> On 17 Nov 2015 13:31, Kevin Buettner wrote:
> > On Tue, 17 Nov 2015 00:41:33 -0500 Mike Frysinger wrote:
> > > > So sim/common is doing the same thing as my proposed patch for ppc;
> > > > sim/common is just using a more elegant mechanism to avoid calling
> > > > close() on these three file descriptors.
> > >
> > > the difference is that this code sequence misbehaves after your change:
> > > close(1);
> > > write(1, "foo", 3);
> > > under the common sim, the write will return EBADF.
> > >
> > > considering how much of common/ came from ppc/ i'm a little surprised
> > > virtualization of the fd table didn't.
> > >
> > > it would be nice if we could at least hide these three fds (a static
> > > array of 3 bools maybe?), but i won't push hard for you to do that.
> >
> > Do you mean an array which indicates the open / closed status of each
> > of stdin, stdout, and stderr? This status would then be used to
> > return EBADF in the right places when the descriptor is "closed".
>
> correct. maybe add a helper func like the common code does, and then
> have every wrapper (read, write, etc...) check that before making the
> actual call. i don't think we have to get fancy and preserve exact
> behavior since the standard does not require it; i.e. this code:
> close(2)
> int fd = open(...)
> fd would normally be 2, but since we haven't really closed it, you'd
> get back a higher fd.
Below is a patch which adds and uses the helper function, fdbad, which
mimics (as much as makes sense) what is done in sim/common.
This adds on to my earlier patch:
https://sourceware.org/ml/gdb-patches/2015-11/msg00354.html
I.e. my earlier patch must be applied first before applying the patch
below.
ppc sim: Track closed state of file descriptors 0, 1, and 2.
This change tracks the "closed" state of file descriptors 0, 1, and 2,
introducing the function fdbad() to emul_netbsd.c and emul_unix.c.
Note that a function of the same name and purpose exists in
sim/common/callback.c.
sim/ppc/ChangeLog:
* emul_netbsd.c (fd_closed): New static array.
(fdbad): New function.
(do_read, do_write, do_close, do_dup, do_ioctl, do_dup2, do_fcntl)
(do_fstatfs, do_fstat, do_lseek): Call `fdbad'.
(emul_netbsd_init): Initialize `fd_closed'.
* emul_unix.c (fd_closed): New static array.
(fdbad): New function.
(do_unix_read, do_unix_write, do_unix_close, do_unix_dup)
(do_unix_dup2, do_unix_lseek, do_solaris_fstat, do_solaris_ioctl)
(do_linux_fstat, do_linux_ioctl): Call `fdbad'.
(emul_solaris_init, emul_linux_init): Initialize `fd_closed'.
---
sim/ppc/emul_netbsd.c | 81 +++++++++++++++++++++++++++++++++++++++++----------
sim/ppc/emul_unix.c | 79 ++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 131 insertions(+), 29 deletions(-)
@@ -292,6 +292,31 @@ write_rusage(unsigned_word addr,
}
#endif
+
+/* File descriptors 0, 1, and 2 should not be closed. fd_closed[]
+ tracks whether these descriptors have been closed in do_close()
+ below. */
+
+static int fd_closed[3];
+
+/* Check for some occurrences of bad file descriptors. We only check
+ whether fd 0, 1, or 2 are "closed". By "closed" we mean that these
+ descriptors aren't actually closed, but are considered to be closed
+ by this layer.
+
+ Other checks are performed by the underlying OS call. */
+
+static int
+fdbad (int fd)
+{
+ if (fd >=0 && fd <= 2 && fd_closed[fd])
+ {
+ errno = EBADF;
+ return -1;
+ }
+ return 0;
+}
+
static void
do_exit(os_emul_data *emul,
unsigned call,
@@ -339,7 +364,9 @@ do_read(os_emul_data *emul,
status = -1;
}
#endif
- status = read (d, scratch_buffer, nbytes);
+ status = fdbad (d);
+ if (status == 0)
+ status = read (d, scratch_buffer, nbytes);
emul_write_status(processor, status, errno);
if (status > 0)
@@ -374,7 +401,10 @@ do_write(os_emul_data *emul,
processor, cia);
/* write */
- status = write(d, scratch_buffer, nbytes);
+ status = fdbad (d);
+ if (status == 0)
+ status = write(d, scratch_buffer, nbytes);
+
emul_write_status(processor, status, errno);
free(scratch_buffer);
@@ -440,12 +470,19 @@ do_close(os_emul_data *emul,
SYS(close);
- /* Do not close stdin, stdout, or stderr. GDB may still need access to
- these descriptors. */
- if (d == 0 || d == 1 || d == 2)
- status = 0;
- else
- status = close(d);
+ status = fdbad (d);
+ if (status == 0)
+ {
+ /* Do not close stdin, stdout, or stderr. GDB may still need access to
+ these descriptors. */
+ if (d == 0 || d == 1 || d == 2)
+ {
+ fd_closed[d] = 1;
+ status = 0;
+ }
+ else
+ status = close(d);
+ }
emul_write_status(processor, status, errno);
}
@@ -554,7 +591,7 @@ do_dup(os_emul_data *emul,
unsigned_word cia)
{
int oldd = cpu_registers(processor)->gpr[arg0];
- int status = dup(oldd);
+ int status = (fdbad (oldd) < 0) ? -1 : dup(oldd);
int err = errno;
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -645,7 +682,9 @@ do_ioctl(os_emul_data *emul,
|| dir & IOC_OUT
|| !(dir & IOC_VOID))
error("do_ioctl() read or write of parameter not implemented\n");
- status = ioctl(d, request, NULL);
+ status = fdbad (d);
+ if (status == 0)
+ status = ioctl(d, request, NULL);
emul_write_status(processor, status, errno);
#endif
@@ -686,7 +725,7 @@ do_dup2(os_emul_data *emul,
{
int oldd = cpu_registers(processor)->gpr[arg0];
int newd = cpu_registers(processor)->gpr[arg0+1];
- int status = dup2(oldd, newd);
+ int status = (fdbad (oldd) < 0) ? -1 : dup2(oldd, newd);
int err = errno;
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -716,7 +755,9 @@ do_fcntl(os_emul_data *emul,
printf_filtered ("%d, %d, %d", fd, cmd, arg);
SYS(fcntl);
- status = fcntl(fd, cmd, arg);
+ status = fdbad (fd);
+ if (status == 0)
+ status = fcntl(fd, cmd, arg);
emul_write_status(processor, status, errno);
}
#endif
@@ -801,7 +842,9 @@ do_fstatfs(os_emul_data *emul,
printf_filtered ("%d, 0x%lx", fd, (long)buf_addr);
SYS(fstatfs);
- status = fstatfs(fd, (buf_addr == 0 ? NULL : &buf));
+ status = fdbad (fd);
+ if (status == 0)
+ status = fstatfs(fd, (buf_addr == 0 ? NULL : &buf));
emul_write_status(processor, status, errno);
if (status == 0) {
if (buf_addr != 0)
@@ -854,7 +897,9 @@ do_fstat(os_emul_data *emul,
SYS(fstat);
#endif
/* Can't combine these statements, cuz fstat sets errno. */
- status = fstat(fd, &buf);
+ status = fdbad (fd);
+ if (status == 0)
+ status = fstat(fd, &buf);
emul_write_status(processor, status, errno);
write_stat(stat_buf_addr, buf, processor, cia);
}
@@ -956,7 +1001,9 @@ do_lseek(os_emul_data *emul,
int whence = cpu_registers(processor)->gpr[arg0+4];
off_t status;
SYS(lseek);
- status = lseek(fildes, offset, whence);
+ status = fdbad (fildes);
+ if (status == 0)
+ status = lseek(fildes, offset, whence);
if (status == -1)
emul_write_status(processor, -1, errno);
else {
@@ -1455,7 +1502,9 @@ static void
emul_netbsd_init(os_emul_data *emul_data,
int nr_cpus)
{
- /* nothing yet */
+ fd_closed[0] = 0;
+ fd_closed[1] = 0;
+ fd_closed[2] = 0;
}
static void
@@ -195,6 +195,31 @@ struct unix_rusage {
};
+/* File descriptors 0, 1, and 2 should not be closed. fd_closed[]
+ tracks whether these descriptors have been closed in do_close()
+ below. */
+
+static int fd_closed[3];
+
+/* Check for some occurrences of bad file descriptors. We only check
+ whether fd 0, 1, or 2 are "closed". By "closed" we mean that these
+ descriptors aren't actually closed, but are considered to be closed
+ by this layer.
+
+ Other checks are performed by the underlying OS call. */
+
+static int
+fdbad (int fd)
+{
+ if (fd >=0 && fd <= 2 && fd_closed[fd])
+ {
+ errno = EBADF;
+ return -1;
+ }
+ return 0;
+}
+
+
static void
do_unix_exit(os_emul_data *emul,
unsigned call,
@@ -232,8 +257,10 @@ do_unix_read(os_emul_data *emul,
/* check if buffer exists by reading it */
emul_read_buffer(scratch_buffer, buf, nbytes, processor, cia);
+ status = fdbad (d);
/* read */
- status = read (d, scratch_buffer, nbytes);
+ if (status == 0)
+ status = read (d, scratch_buffer, nbytes);
emul_write_status(processor, status, errno);
if (status > 0)
@@ -266,8 +293,10 @@ do_unix_write(os_emul_data *emul,
emul_read_buffer(scratch_buffer, buf, nbytes,
processor, cia);
+ status = fdbad (d);
/* write */
- status = write(d, scratch_buffer, nbytes);
+ if (status == 0)
+ status = write(d, scratch_buffer, nbytes);
emul_write_status(processor, status, errno);
free(scratch_buffer);
@@ -310,10 +339,14 @@ do_unix_close(os_emul_data *emul,
if (WITH_TRACE && ppc_trace[trace_os_emul])
printf_filtered ("%d", d);
- if (d == 0 || d == 1 || d == 2)
- status = 0;
- else
- status = close(d);
+ status = fdbad (d);
+ if (status == 0)
+ {
+ if (d == 0 || d == 1 || d == 2)
+ status = 0;
+ else
+ status = close(d);
+ }
emul_write_status(processor, status, errno);
}
@@ -494,7 +527,7 @@ do_unix_dup(os_emul_data *emul,
unsigned_word cia)
{
int oldd = cpu_registers(processor)->gpr[arg0];
- int status = dup(oldd);
+ int status = (fdbad (oldd) < 0) ? -1 : dup(oldd);
int err = errno;
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -516,7 +549,7 @@ do_unix_dup2(os_emul_data *emul,
{
int oldd = cpu_registers(processor)->gpr[arg0];
int newd = cpu_registers(processor)->gpr[arg0+1];
- int status = dup2(oldd, newd);
+ int status = (fdbad (oldd) < 0) ? -1 : dup2(oldd, newd);
int err = errno;
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -544,7 +577,9 @@ do_unix_lseek(os_emul_data *emul,
if (WITH_TRACE && ppc_trace[trace_os_emul])
printf_filtered ("%d %ld %d", fildes, (long)offset, whence);
- status = lseek(fildes, offset, whence);
+ status = fdbad (fildes);
+ if (status == 0)
+ status = lseek(fildes, offset, whence);
emul_write_status(processor, (int)status, errno);
}
#endif
@@ -1200,7 +1235,9 @@ do_solaris_fstat(os_emul_data *emul,
if (WITH_TRACE && ppc_trace[trace_os_emul])
printf_filtered ("%d, 0x%lx", fildes, (long)stat_pkt);
- status = fstat (fildes, &buf);
+ status = fdbad (fildes);
+ if (status == 0)
+ status = fstat (fildes, &buf);
if (status == 0)
convert_to_solaris_stat (stat_pkt, &buf, processor, cia);
@@ -1437,6 +1474,10 @@ do_solaris_ioctl(os_emul_data *emul,
#endif
#endif
+ status = fdbad (fildes);
+ if (status != 0)
+ goto done;
+
switch (request)
{
case 0: /* make sure we have at least one case */
@@ -1478,6 +1519,7 @@ do_solaris_ioctl(os_emul_data *emul,
#endif /* HAVE_TERMIOS_STRUCTURE */
}
+done:
emul_write_status(processor, status, errno);
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -1929,7 +1971,9 @@ static void
emul_solaris_init(os_emul_data *emul_data,
int nr_cpus)
{
- /* nothing yet */
+ fd_closed[0] = 0;
+ fd_closed[1] = 0;
+ fd_closed[2] = 0;
}
static void
@@ -2128,7 +2172,9 @@ do_linux_fstat(os_emul_data *emul,
if (WITH_TRACE && ppc_trace[trace_os_emul])
printf_filtered ("%d, 0x%lx", fildes, (long)stat_pkt);
- status = fstat (fildes, &buf);
+ status = fdbad (fildes);
+ if (status == 0)
+ status = fstat (fildes, &buf);
if (status == 0)
convert_to_linux_stat (stat_pkt, &buf, processor, cia);
@@ -2392,6 +2438,10 @@ do_linux_ioctl(os_emul_data *emul,
#endif
#endif
+ status = fdbad (fildes);
+ if (status != 0)
+ goto done;
+
switch (request)
{
case 0: /* make sure we have at least one case */
@@ -2433,6 +2483,7 @@ do_linux_ioctl(os_emul_data *emul,
#endif /* HAVE_TERMIOS_STRUCTURE */
}
+done:
emul_write_status(processor, status, errno);
if (WITH_TRACE && ppc_trace[trace_os_emul])
@@ -2799,7 +2850,9 @@ static void
emul_linux_init(os_emul_data *emul_data,
int nr_cpus)
{
- /* nothing yet */
+ fd_closed[0] = 0;
+ fd_closed[1] = 0;
+ fd_closed[2] = 0;
}
static void