Hi Andrew, Thanks — that all makes sense, and yes, I understand why options 1 and 2 break cross-version communication. I'll go with the negotiated approach. Here is the completed table. The "Actual Size" column is sizeof() of the corresponding struct stat member: | Field Name | Type | Size | Actual | | | | Now | Size | |-------------+-------------+------+--------| | fst_dev | fio_uint_t | 4 | 8 | | fst_ino | fio_uint_t | 4 | 8 | | fst_mode | fio_mode_t | 4 | 4 | | fst_nlink | fio_uint_t | 4 | 8 | | fst_uid | fio_uint_t | 4 | 4 | | fst_gid | fio_uint_t | 4 | 4 | | fst_rdev | fio_uint_t | 4 | 8 | | fst_size | fio_ulong_t | 8 | 8 | | fst_blksize | fio_ulong_t | 8 | 8 | | fst_blocks | fio_ulong_t | 8 | 8 | | fst_atime | fio_time_t | 4 | 8 | | fst_mtime | fio_time_t | 4 | 8 | | fst_ctime | fio_time_t | 4 | 8 | So six fields are too narrow: fst_dev, fst_ino, fst_nlink, fst_rdev and the three timestamps. fst_size, fst_blksize and fst_blocks are already fio_ulong_t. fst_mode, fst_uid and fst_gid are 4 bytes in struct stat as well, so those look correct as they are. I got identical sizes on powerpc64le, powerpc64 big-endian and x86-64, and on both ext4 and xfs, so this appears to come from glibc rather than from the architecture or filesystem. I have not checked a 32-bit build. Whether the truncation is visible depends on how large the filesystem's inode numbers get. Largest inode under /home on the three machines I have: ext4, 2 TB 132907009 passes xfs, 2 TB 3896368632 passes xfs, 86 TB 171801755018 fails The second one is at about 91% of 2^32, so it passes today but would not after the filesystem grew. I'll start on the two-struct approach with qSupported negotiation as you described, converting at the wire boundary in both directions. Thanks, Abhay On 26/08/26 22:51, Andrew Burgess wrote: > Abhay Kandpal writes: > >> Hi Andrew, >> >> I hit a failure in gdb.server/fileio-packets.exp on a machine whose filesystem uses inode numbers above 2^32. >> >> All four stat/lstat checks fail, with every field matching except st_ino: >> >> remote = {..., 'st_ino': 2161233200, ...} >> local = {..., 'st_ino': 122420317488, ...} >> >> 122420317488 = 0x1C80CE9BB0 >> 2161233200 = 0x80CE9BB0 >> >> The field is 4 bytes wide in the protocol struct, while st_ino is 64-bit on Linux: >> >> gdbsupport/fileio.h:130 fio_uint_t fst_ino; >> gdbsupport/fileio.cc:281 host_to_fileio_uint ((long) st->st_ino, fst->fst_ino); >> >> fst_size, fst_blksize and fst_blocks in the same struct already use the 8-byte fio_ulong_t. >> It reproduces on xfs (86 TB, largest inode 171801755018) and passes on ext4 (2 TB, largest inode 132907009), >> so it depends on the filesystem rather than the architecture. >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=34567 >> >> Before writing anything I wanted to ask how you'd prefer this handled. >> >> struct fio_stat is shared between the vFile packets and the older F-packet protocol, >> so widening fst_ino changes the wire layout for both. >> >> I can see three options: >> >> 1. Widen the shared field. > I think this would not be accepted as this would break communication > between different versions of GDB and gdbserver, right? > >> 2. Add a separate wider struct used only by the vFile packets, >> along the lines of your reasoning in c29a37f7417 about those packetsstill being new. > I think only fixing vFile would be a mistake. The 'F' packets are not > an older protocol that has been replaced with vFile. The two systems > offer similar functionality, but in opposite directions. If we're > fixing one direction then we really should fix both. > > In c29a37f7417 I changed the underlying implementation of the 'stat' > packet from using 'lstat' to using 'stat'. The actual on-the-wire bits > didn't change, just what gdbserver did with them. An old GDB can still > talk to a new gdbserver and vice versa. What you're proposing would > break this cross version communication, just like option #1, right? > >> 3. Negotiate the wider format through qSupported. > I think this is the only possible way forward unfortunately. It is > going to be more work, but anything else is going to end up breaking > backward compatibility. > >> The second seems the least disruptive, but I don't have a good sense of who else implements these packets. > That's a huge problem we have. We really have no visibility at all for > how this stuff is used outside the GDB project. We solve this problem > by basically assuming that anything that has been released might be > being used, and so cannot be changed. That's probably not true, but we > just have no way of knowing. > >> Also worth deciding at the same time, if the layout is being revised: >> fst_dev and fst_rdev are fio_uint_t although dev_t is 64-bit, and the three timestamps are 4 bytes. > It sounds like what you're saying is that a whole bunch of fields are > the wrong size. I put together this table: > > | Field Name | Type | Size | Actual | > | | | Now | Size | > |-------------+-------------+------+--------| > | fst_dev | fio_uint_t | 4 | 8 | > | fst_ino | fio_uint_t | 4 | 8 | > | fst_mode | fio_mode_t | 4 | ? | > | fst_nlink | fio_uint_t | 4 | ? | > | fst_uid | fio_uint_t | 4 | ? | > | fst_gid | fio_uint_t | 4 | ? | > | fst_rdev | fio_uint_t | 4 | 8 | > | fst_size | fio_ulong_t | 8 | ? | > | fst_blksize | fio_ulong_t | 8 | ? | > | fst_blocks | fio_ulong_t | 8 | ? | > | fst_atime | fio_time_t | 4 | 8? | > | fst_mtime | fio_time_t | 4 | 8? | > | fst_ctime | fio_time_t | 4 | 8? | > > The 'Size Now' is the current field size in fio_stat, while the 'Actual > Size' is what the fields need to be in order to be correct on your > system. You mention the three timestamps above, but don't say what size > they need to be. I'm assuming 8, but that might not be correct either. > Also there are some fields that you haven't mentioned, maybe they are > all correct, but we should check. > > What I'd suggest is that you finish filling in the above table, then I > think you'll want to have two versions of fio_stat, one with the current > field sizes, and one with the wider field sizes. > > I'd suggest that you update the core throughout to use the struct with > the wider field sizes, but at some point before this struct is sent down > the wire, if the use of the wide struct has not been negotiated, then > you'll need to squeeze the wide struct into the short one, trimming the > fields. I think it's OK to print a warning if non-zero information is > discarded at this point. > > Similarly, when one of these structs is pulled from the wire, if the > incoming packet is the narrow version, I'd expand it into the wide > version. In this way, most GDB/gdbserver code will only need to handle > the version with the wider fields, but the wire protocol remains > unchanged. > > Thanks, > Andrew >