Re: [RFC v4 08/24] sysdeps/stat: Copy the statx struct to stat instead of stat64

[Alistair Francis <alistair23 at gmail dot com>]

On Mon, Aug 12, 2019 at 1:01 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Fri, 9 Aug 2019, Alistair Francis wrote:
>
> > When copying the statx struct to the stat stuct use the original stat
> > struct instead of the stat64 struct. As the padding in the original is
> > type 'unsigned short int' but the padding in the stat64 is 'unsigned int'
> > the copy  can result in misallgined data. This would then incorrectly
> > trigger the stat_overflow() failure.
>
> This indicates there's something else wrong with the port.  By design, the
> following apply for linux/generic/wordsize-32 ports of glibc:
>
> * The layout of struct stat and struct stat64 is identical, except that
> some bytes that are padding in struct stat serve as high parts of fields
> that are wider in struct stat64 (and thus have endian-dependent positions
> as determined by the __field64 macro in bits/stat.h).

__ino_t is a 64-bit value in RV32, so in both stat and stat64 it's the
same thing. The __pad1 changes in size between the two structs as one
is a short and one is just an int. I don't see how they are identical.

>
> * Conversions from statx have to go to stat64, including setting those
> high parts as appropriate, so that the subsequent overflow checks (which
> work by examining those padding fields) can correctly detect whether
> overflow occurred and set errno to EOVERFLOW accordingly.  See my C-SKY
> port reviews that resulted in the code we have now
> <https://sourceware.org/ml/libc-alpha/2018-11/msg00624.html>
> <https://sourceware.org/ml/libc-alpha/2018-11/msg00668.html>.

Ok, this makes sense.

>
> > This would be very obvious when using a 64-bit ino_t type on a 32-bit
> > system, such as the RV32 port.
>
> If those types are 64-bit, you should not have padding around them in
> struct stat, so as to preserve the property that struct stat and struct
> stat64 have the same layout.  I suppose this means bits/stat.h needs to
> check further macros such as __OFF_T_MATCHES_OFF64_T.

Changing the padding would fix the problem. Just to be clear is that
what you are suggesting?

>
> You'll also need to ensure that XSTAT_IS_XSTAT64 is defined to 1.  And
> you'll need to make wordsize-32/overflow.h define trivial versions of the
> *_overflow functions in cases where the types match (this should be done
> in that file, rather than making an RV32-specific copy, to benefit future
> ports that make the same choices as RV32).

I think I tested that and it didn't fix the problem so I dropped it.
I'll add the XSTAT_IS_XSTAT64 define back.

I'm not sure what you mean by then the types match?

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com