* Re: [SCM] master: Switch to frysk.sys.ptrace. [not found] <20080212185049.22237.qmail@sourceware.org> @ 2008-02-13 8:47 ` Mark Wielaard 2008-02-27 11:59 ` Mark Wielaard 0 siblings, 1 reply; 3+ messages in thread From: Mark Wielaard @ 2008-02-13 8:47 UTC (permalink / raw) To: frysk Hi Andrew, On Tue, 2008-02-12 at 18:50 +0000, cagney@sourceware.org wrote: > - private int peek(long index, byte[] bytes, int offset, int length) > - { > - if (mem != null) > - { > - try > - { > - return mem.pread(index, bytes, offset, length); > - } > - catch (Errno e) > - { > - // try again through address space. > + private void transfer(long index, byte[] bytes, int offset, int length, > + boolean write) { > + if (mem != null && !write) { > + try { > + mem.pread(index, bytes, offset, length); > + return; > + } catch (Errno e) { > + // Give up on mem; and fall back to ptrace. This > + // can happen when /proc isn't mounted, or when a > + // process is terminating and the kernel scrubs > + // the /proc entry before its time. > + mem = null; > } > } Are you sure this change is right? The comment is right about those 2 reasons for pread() possibly failing. But I don't believe that is the only case it can happen. Previously when a read occurred that failed, possibly because following a pointer in the inferior that was NULL or not yet initialized that would just cause a error reading from that particular (uninitialized) memory location. But now whenever that occurs once for an inferior you blow away mem so pread() can never be used anymore and we always fall back to having to do single peeks and pokes through the ptrace address space. Cheers, Mark ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [SCM] master: Switch to frysk.sys.ptrace. 2008-02-13 8:47 ` [SCM] master: Switch to frysk.sys.ptrace Mark Wielaard @ 2008-02-27 11:59 ` Mark Wielaard 2008-03-10 12:02 ` Mark Wielaard 0 siblings, 1 reply; 3+ messages in thread From: Mark Wielaard @ 2008-02-27 11:59 UTC (permalink / raw) To: frysk; +Cc: Andrew Cagney Hi Andrew, On Wed, 2008-02-13 at 09:47 +0100, Mark Wielaard wrote: > On Tue, 2008-02-12 at 18:50 +0000, cagney@sourceware.org wrote: > > - private int peek(long index, byte[] bytes, int offset, int length) > > - { > > - if (mem != null) > > - { > > - try > > - { > > - return mem.pread(index, bytes, offset, length); > > - } > > - catch (Errno e) > > - { > > - // try again through address space. > > + private void transfer(long index, byte[] bytes, int offset, int length, > > + boolean write) { > > + if (mem != null && !write) { > > + try { > > + mem.pread(index, bytes, offset, length); > > + return; > > + } catch (Errno e) { > > + // Give up on mem; and fall back to ptrace. This > > + // can happen when /proc isn't mounted, or when a > > + // process is terminating and the kernel scrubs > > + // the /proc entry before its time. > > + mem = null; > > } > > } > > Are you sure this change is right? The comment is right about those 2 > reasons for pread() possibly failing. But I don't believe that is the > only case it can happen. Previously when a read occurred that failed, > possibly because following a pointer in the inferior that was NULL or > not yet initialized that would just cause a error reading from that > particular (uninitialized) memory location. But now whenever that occurs > once for an inferior you blow away mem so pread() can never be used > anymore and we always fall back to having to do single peeks and pokes > through the ptrace address space. Did you have time to investigate this? If not, shall we revert it to the original implementation till you do have time? Thanks, Mark ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [SCM] master: Switch to frysk.sys.ptrace. 2008-02-27 11:59 ` Mark Wielaard @ 2008-03-10 12:02 ` Mark Wielaard 0 siblings, 0 replies; 3+ messages in thread From: Mark Wielaard @ 2008-03-10 12:02 UTC (permalink / raw) To: frysk; +Cc: Andrew Cagney Hi Andrew, On Wed, 2008-02-27 at 12:59 +0100, Mark Wielaard wrote: > On Wed, 2008-02-13 at 09:47 +0100, Mark Wielaard wrote: > > On Tue, 2008-02-12 at 18:50 +0000, cagney@sourceware.org wrote: > > > - private int peek(long index, byte[] bytes, int offset, int length) > > > - { > > > - if (mem != null) > > > - { > > > - try > > > - { > > > - return mem.pread(index, bytes, offset, length); > > > - } > > > - catch (Errno e) > > > - { > > > - // try again through address space. > > > + private void transfer(long index, byte[] bytes, int offset, int length, > > > + boolean write) { > > > + if (mem != null && !write) { > > > + try { > > > + mem.pread(index, bytes, offset, length); > > > + return; > > > + } catch (Errno e) { > > > + // Give up on mem; and fall back to ptrace. This > > > + // can happen when /proc isn't mounted, or when a > > > + // process is terminating and the kernel scrubs > > > + // the /proc entry before its time. > > > + mem = null; > > > } > > > } > > > > Are you sure this change is right? The comment is right about those 2 > > reasons for pread() possibly failing. But I don't believe that is the > > only case it can happen. Previously when a read occurred that failed, > > possibly because following a pointer in the inferior that was NULL or > > not yet initialized that would just cause a error reading from that > > particular (uninitialized) memory location. But now whenever that occurs > > once for an inferior you blow away mem so pread() can never be used > > anymore and we always fall back to having to do single peeks and pokes > > through the ptrace address space. > > Did you have time to investigate this? If not, shall we revert it to > the > original implementation till you do have time? Ping. Please do take a look. I really would like to see this resolved. I appreciate that it is hard to just revert this hunk if you don't have time right now to finalize it, because it was embedded into another rewrite commit of other stuff. So it would really help if you made separate commits for separate issues in the future. That makes these kind of reviews easier. Thanks, Mark ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-03-10 12:02 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20080212185049.22237.qmail@sourceware.org> 2008-02-13 8:47 ` [SCM] master: Switch to frysk.sys.ptrace Mark Wielaard 2008-02-27 11:59 ` Mark Wielaard 2008-03-10 12:02 ` Mark Wielaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).