Hi Ulrich, 

Thanks for looking.

Abhay Kandpal <abhay@linux.ibm.com> wrote:

The F-packet calls cannot use qSupported, since the buffer is read
directly by the target.  But each request names the call it wants, so
add two new calls, Fstat64 and Ffstat64, which are identical to Fstat
and Ffstat except that they write struct fio_stat_wide.  Existing
targets keep using Fstat and Ffstat and see no change.  A GDB which
does not know the new calls already replies with ENOSYS, so a target
can try Fstat64 and fall back to Fstat.

This makes sense to me, if support for this is needed.  I'm not
really sure who -if anyone- is using these F-packets; gdbserver
doesn't seem to do so and there's no test in the test suite ...

Andrew, do you have any thoughts on this?

One user is QEMU: its semihosting code (semihosting/syscalls.c)
sends F-packet requests for every call in the protocol, including
both stat and fstat. I added 2/2 because Andrew preferred both
directions to be fixed, and I'm happy to go either way.

The console case in the fstat handler stored 1 into fst_dev, but that
was then overwritten when the struct was packed, so the target always
saw zero.  Drop the dead store; behaviour is unchanged.

Interesting.  Seems this accidentally changed here:
https://sourceware.org/pipermail/gdb-patches/2015-March/122143.html

which says:
	(remote_fileio_mode_to_target): Moved to common-remote-
fileio.c.
	(remote_fileio_to_fio_mode): Likewise.
	(remote_fileio_to_fio_ulong): Likewise.
	(remote_fileio_to_fio_stat): Likewise.

but in addition to the move, changed:

-static void
-remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)
-{
-  LONGEST blksize;
-
-  /* `st_dev' is set in the calling function.  */
-  remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);

to:

+void
+remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)
+{
+  LONGEST blksize;
+
+  remote_fileio_to_fio_uint ((long) st->st_dev, fst->fst_dev);
+  remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);


So up until 2015 fst_dev was indeed 1 for the console and
0 for everything else, and since then it's been 0 always.
Given that long time, it may be fine to keep it at 0 now ...

On the other hand, the documentation still says:

@item st_dev
A value of 0 represents a file, 1 the console.

So we can either fix the code again, or else update the
documentation to match the behavior over the last 10 years.
Thoughts?

There is a second mismatch as well. For a regular file, the fstat
handler never sets fst_dev, so the host's real st_dev is passed
through rather than the documented 0. Today only stat matches
the documentation:

stat, file:        documented 0, sends 0
fstat, console:    documented 1, sends 0
fstat, file:       documented 0, sends the host's st_dev

My preference would be to fix the code to match the documentation,
since the 2015 change looks accidental. I'd do that as a separate
patch, so it doesn't depend on the stat64 series.

Thanks
Abhay

Bye,
Ulrich