[commit] Adjust ia64_linux_xfer_partial following to_xfer_partial API change.

[Joel Brobecker]

> > +      gdb_byte *tmp_buf = alloca (offset + len);
> > +      ULONGEST xfered;
> > +
> > +      xfered = syscall (__NR_getunwind, readbuf, offset + len);
> 
> It seems this should have passed tmp_buf instead of readbuf?

Yes, indeed. Hasty implementation which, as you found out,
gets luck (at best).

> Also, I don't think the "offset + len" size is the
> right size to pass down to the syscall.
> 
> From:
> 
> http://lxr.free-electrons.com/source/arch/ia64/kernel/unwind.c#L2313

I thought it was OK. Basically, we were asked to get len bytes at
offset. The idea was that, since the syscall doesn't handle offsets,
we have to fetch from zero again, which means getting offset bytes
(which were going to be thrown away) + len bytes (which we'll return).

> So it looks like this code is getting lucky and the caller
> is requesting enough bytes:
> 
>   x = target_read_alloc (&current_target, TARGET_OBJECT_UNWIND_TABLE,
> 			 NULL, buf_p);

Right. I noticed that also, because I couldn't explain at the beginning
why the syscall was only performed with offset == 0 in the previous
implementation.

> But to_xfer_partial implementations should not assume that.  E.g.,
> change target_read_alloc_1 to start by requesting a byte at a time
> instead of 4096, and this will fail.
> 
> > +      if (xfered <= 0)
> 
> Note that because xfered is ULONGEST, this won't actually
> detect errors.

Indeed. I had assumed that a return value of 0 indicated an error
already, and wrote it that way; and then I thought, hey, negative
values should never happen either, right? Let's handle that too.
Duh...

> > +      else if (xfered <= offset)
> > +	return TARGET_XFER_EOF;
>
> Not sure I follow this one.

The theory was the following:

   1. First xfer request, ask ~4k at offset zero,
      return xxx bytes and OK

   2. The caller finds an OK, but only xxx bytes < 4k.
      So round #2, asks for 4k - xxx bytes at offset xxx.
      We fetch unwind table again, found that xferd is xxx again.
      Since offset is xxx, it means we're done. Return EOF.

> I suggest something along the lines of the below instead.
> Should handle partial requests, and clearer to read, I think.
> Completely untested, though.

It does look better indeed. A little more elaborate, and also
a little more cogniscent of what the syscall actually does.
I based my implementation a lot on the previous one, with
the idea in the back of my mind that I wouldn't be spending
too much time on it, because we would probably want to remove
that code one day - as it's based on a deprecated feature.
(and for good measure, I don't believe in ia64's future either)

Attached is what I checked in. There were a couple of tweaks
I needed to make (TARGET_XFER_EIO -> TARGET_XFER_E_IO, and
res was not assigned). I'm a little nervous about the gdb_assert,
as I usually prefer to use them only when everything is under
our control and we can't recover from the situation. I was
almost ready to see what would happen if we returned E_IO
instead of brutally interruping the debugging session. But
we'll see what happens with the current code.

BTW: Now that I have pushed the patch, I just realized something.
Are you sure the gate_table_size is a constant? >:-o

gdb/ChangeLog:

        * ia64-linux-nat.c (ia64_linux_xfer_partial): Reimplement
        handling of object == TARGET_OBJECT_UNWIND_TABLE.

Thanks,
-- 
Joel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Re-implement-ia64-linux-nat.c-ia64_linux_xfer_partia.patch
Type: text/x-diff
Size: 2742 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20140226/850a194a/attachment.bin>