Hi Adhemerval,

> On 14/10/2020 12:56, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >   
> >> On 14/10/2020 11:14, Lukasz Majewski wrote:  
> >>> Hi Adhemerval,
> >>>     
> >>>> On 14/10/2020 10:00, Lukasz Majewski wrote:    
> >>>>> In the installed struct stat{64} the __ino64_t st_ino member is
> >>>>> placed in the end. This patch moves it to the same position as
> >>>>> in the aforementioned, exported structures as it allows less
> >>>>> #ifdefs for __USE_TIME_BITS64 support use case.      
> >>>>
> >>>> Why exactly this is required? Afaik this is glibc defined type
> >>>> not currently only used internally and all member accesses are
> >>>> set directly (no use or memcpy or offsetof).  Is this related to
> >>>> the issue you saw on the arm environment?     
> >>>
> >>> This is required to make the minimal changes to provide
> >>> -D_TIME_SIZE=64 support for {f}stat{at} as in:
> >>> https://github.com/lmajewski/y2038_glibc/commit/26aa2ac07246682a505d85dac1c269689964b79b
> >>>    
> >>
> >> But the idea it to decouple y2038 stat from the generic stat which
> >> support non-LFS and make it support solely LFS.  
> > 
> > And this idea is correct.
> >   
> >> As Joseph has pointed
> >> out, the __stat64_t64 should be architecture-independent and I
> >> think it simplifies its definition to untie from struct_stat.h
> >> (which has multiple definitions due historically each architecture 
> >> to tie with the selected kernel ABI).  
> > 
> > This is also correct assumption.
> > 
> > Please find my explanation of the problem as I may not been
> > correctly understood:
> > 
> > For example the __fstat64_time64 defined in
> > sysdeps/unix/sysv/linux/fstat64.c has following signature:
> > 
> > int __fstat64_time64 (int fd, struct __stat64_t64 *buf)
> > 
> > for:
> > - __WORDSIZE==64 is struct __stat64_t64 is aliased to struct
> > stat64,  
> 
> Not __WORDSIZE==64 but rather XSTAT_IS_XSTAT64, since mips64 and
> mips64-n32 provides non-LFS interfaces. For XSTAT_IS_XSTAT64 ABIs,
> __stat64_t64 is also essentially stat and the {f,l}stat{at}64 is
> aliased to {f,l}stat{at}.

Ok.

> 
> > - __WORDSIZE==32 (Non Y2038) uses __fstat64 with struct stat64
> > - __WORDSIZE==32 (Y2038) will redirect fstat calls to
> > __fstat64_time64 and as a result it will have to accept *buf which
> > must be binary compatible with struct stat{64} and __stat64_t64.  
> 
> Yes, fstat on __WORDSIZE==32 with XSTAT_IS_XSTAT64 will be an alias
> to __fstat64_time64 (and this now assumed that any new 32-bit ABI
> will have to only provide LFS interfaces and 64-bit time_t).

I do agree.

> 
> Also for such cases __stat64_t64 *won't* be exported, its struct stat
> *already* has 64-bit time fields.

Yes, correct.

> 
> > 
> > By binary compatible I mean the layout and fields must be identical
> > as passing struct stat{64} to __fstat64_time64 will force casting
> > of the data pointed by *buf.  
> 
> I think there is a confusion here, for currently y2038 enabled ABIs
> you will have either the first situation or the second you described.
> 
> For future y2038 ABIs (basically all 32-bit that still supports
> 32-bit time_t) __stat64_t64 and stat64 do *not* require to be binary
> compatible.

Hmm.... 

> 
> In this case you will have:
> 
>   1. non-LFS {f,l}stat{64} that accepts 'struct stat'.

By LFS I guess you mean the -D_LARGEFILE64_SOURCE compilation switch
(which is optional and enables struct stat64 support).

>   2. LFS with 32-bit time fields {f,l}stat{64} that accepts 'struct
> stat64'. 

Is this scenario valid? For Y2038 support on ARM usage of
-D_TIME_BITS=64 also mandatory implies -D_FILE_OFFSET_BITS=64

> 3. LFS with 64-bit time fields {f,l}fstat64_t64 (or any other
>      name for the exported ABI) with accepts 'struct __stat64_t64'.

Ok. This is the case on which I'm working on.

> 
> This is *different* than the current 32-bit y2038 enabled ABIs
> (arc, riscv32) which does not need to support neither 1. nor 2..

Ok. I know. Let's keep it aside as they support LFS and Y2038 from the
outset.

> For this architecture the exported ABI is similar to current majority
> 64-bit architectures (modules mips64...), where they exports
> {f,l}stat{at} with {f,l}stat{at}{64} being just aliases. 

Ok.

> 
> So for instance on ARM, to actually use the y2038 calls we will need
> to export the __{f,l}stat{at}64_time64 along with the 'struct
> __stat64_t64' *and* redirects the LFS calls {f,l}stat{at}64 calls to
> _{f,l}stat{at}64_time64 for _TIME_BITS=64 

I think I'm following the above guidelines in my WIP patch, which adds
support for -D_TIME_BITS=64.

> (along with redefine the
> 'struct stat64' to 'struct __stat64_t64').

Please correct me if I'm wrong, but I shall be doable to:
(in unix/sysv/linux/bits/struct_stat.h)

#ifdef __USE_TIME_BITS64
# include <struct_stat_time64.h>
# define stat __stat64_t64
#else
struct stat
  {
  ...
  }
#endif /* __USE_TIME_BITS64  */


And the same pattern for struct stat64.



> 
> > 
> > With current situation we do have
> > (at sysdeps/unix/sysv/linux/struct_stat_time64.h):
> > 
> > struct __stat64_t64
> > {
> >   __dev_t st_dev;
> >   __ino64_t st_ino;
> > 
> > 
> > Which is placed just after st_dev. 
> > 
> > In the struct stat{64} the st_ino is the last member.
> > 
> > As a result we do have a mismatch when passing binary data (the
> > offsets are different for st_ino).
> > 
> > Code from this patch moves st_ino to the last place, which saves a
> > lot of extra code - i.e. one for alleviating the casting of data.
> > 
> > Due to that also the patch in 
> > https://github.com/lmajewski/y2038_glibc/commit/26aa2ac07246682a505d85dac1c269689964b79b
> > 
> > can be small.
> > 
> >   
> >>
> >> It leads to define the struct __stat64_t64 on its own header
> >> instead of trying to accommodate it on stat.h header.  The idea it
> >> to eventually export struct_stat_time64.h as an installed header
> >> and make the required redirections on stat.h header for
> >> -D_TIME_SIZE=64 (so stat64 redirects to __stat_time64).  
> > 
> > As it is now - those two structures are not binary compatible due to
> > different st_ino placement (of course if we _only_ copy data with
> > direct assignment, then we are safe). 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma@denx.de 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de