From patchwork Wed Dec 30 00:18:20 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Frysinger X-Patchwork-Id: 10168 X-Patchwork-Delegate: vapier@gentoo.org Received: (qmail 124315 invoked by alias); 30 Dec 2015 00:18:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124293 invoked by uid 89); 30 Dec 2015 00:18:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=processor, stdout, 3747, eliminates X-HELO: smtp.gentoo.org Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 30 Dec 2015 00:18:23 +0000 Received: from vapier.lan (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with SMTP id 4C02C3407D0; Wed, 30 Dec 2015 00:18:21 +0000 (UTC) Date: Tue, 29 Dec 2015 19:18:20 -0500 From: Mike Frysinger To: Kevin Buettner Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] PPC sim: Don't close file descriptors 0, 1, or 2 Message-ID: <20151230001820.GR25803@vapier.lan> Mail-Followup-To: Kevin Buettner , gdb-patches@sourceware.org References: <20151116145807.4aedd84f@pinnacle.lan> <20151116235317.GF31395@vapier.lan> <20151116210533.058520d2@pinnacle.lan> <20151117054133.GI31395@vapier.lan> <20151117133154.72b61a52@pinnacle.lan> <20151117210540.GM31395@vapier.lan> <20151207132910.71bf8289@pinnacle.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20151207132910.71bf8289@pinnacle.lan> X-IsSubscribed: yes On 07 Dec 2015 13:29, Kevin Buettner wrote: > 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. the linux code was slightly broken (forgot to set fd_close), but i fixed that and pushed it. final commit is below. thanks ! -mike From bd9de4340c0ae6c96b2d32df15a6f355154f05e2 Mon Sep 17 00:00:00 2001 From: Kevin Buettner Date: Mon, 16 Nov 2015 14:58:07 -0700 Subject: [PATCH] sim: ppc: 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. This patch eliminates all of the "unresolved testcases" when testing GDB against the powerpc simulator. This occurs because the powerpc simulator closes, on behalf of the testcase, the file descriptors associated with stdin, stdout, and stderr. GDB still needs these descriptors to communicate with the user or, in this case, with the testing framework. --- sim/ppc/ChangeLog | 14 +++++++++ sim/ppc/emul_netbsd.c | 78 +++++++++++++++++++++++++++++++++++++++++-------- sim/ppc/emul_unix.c | 81 ++++++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 151 insertions(+), 22 deletions(-) diff --git a/sim/ppc/ChangeLog b/sim/ppc/ChangeLog index 413a99d..7ce9a47 100644 --- a/sim/ppc/ChangeLog +++ b/sim/ppc/ChangeLog @@ -1,3 +1,17 @@ +2015-12-29 Kevin Buettner + + * 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'. + 2015-12-26 Mike Frysinger * Makefile.in (TCONFIG_H): Delete. diff --git a/sim/ppc/emul_netbsd.c b/sim/ppc/emul_netbsd.c index 6bef370..12dfb21 100644 --- a/sim/ppc/emul_netbsd.c +++ b/sim/ppc/emul_netbsd.c @@ -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,8 +470,20 @@ do_close(os_emul_data *emul, SYS(close); - /* Can't combine these statements, cuz close sets errno. */ - 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); } @@ -549,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]) @@ -640,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 @@ -681,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]) @@ -711,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 @@ -796,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) @@ -849,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); } @@ -951,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 { @@ -1450,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 diff --git a/sim/ppc/emul_unix.c b/sim/ppc/emul_unix.c index d72525d..1475474 100644 --- a/sim/ppc/emul_unix.c +++ b/sim/ppc/emul_unix.c @@ -195,6 +195,30 @@ 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 +256,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 +292,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,7 +338,20 @@ do_unix_close(os_emul_data *emul, if (WITH_TRACE && ppc_trace[trace_os_emul]) printf_filtered ("%d", d); - 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); } @@ -490,7 +531,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]) @@ -512,7 +553,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]) @@ -540,7 +581,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 @@ -1196,7 +1239,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); @@ -1433,6 +1478,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 */ @@ -1474,6 +1523,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]) @@ -1925,7 +1975,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 @@ -2124,7 +2176,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); @@ -2388,6 +2442,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 */ @@ -2429,6 +2487,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]) @@ -2795,7 +2854,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 -- 2.6.2