stat() broken on CRIS with _FILE_OFFSET_BITS == 64

[Peter Kjellerstedt]

> -----Original Message-----
> From: uclibc-bounces at uclibc.org 
> [mailto:uclibc-bounces at uclibc.org] On Behalf Of Peter Kjellerstedt
> Sent: Tuesday, January 30, 2007 14:19
> To: uclibc at uclibc.org
> Subject: stat() broken on CRIS with _FILE_OFFSET_BITS == 64
> 
> [ If the mail below seems somewhat unstructured it is because
>   I continued to debug while I was writing the mail and thus
>   found out new facts as I carried along. ]
> 
> I was compiling an application which uses large file support for
> the CRIS architecture, and it failed mysteriously. After some 
> debugging I found the culprit in stat(). The attached test 
> program (compiled with -O0) gives me the following output:
>  
> stat(): 0x000802F8
> stat64(): 0x000802F8
> sizeof(struct stat): 92
> sizeof(struct stat64): 96
> sizeof file_info: 92
> file_info.st_size: 2334846631540
> ((struct stat64*)&file_info)->st_size: 543
> magic1: 0x00000000
> magic2: 0xDEADBEEF
> 
> As can be seen above both stat() and stat64() refers to the same 
> function (i.e., stat64()), but struct stat and struct stat64 are
> not the same. Thus the call to stat() with a struct stat argument
> will lead to incorrect data and even overwritten memory (as seen 
> by the magic1 variable).
> 
> I believe the problem originates in include/sys/stat.h (line 
> 205-224) which looks like this:
> 
> #ifndef __USE_FILE_OFFSET64
> /* Get file attributes for FILE and put them in BUF.  */
> extern int stat (__const char *__restrict __file,
> 		 struct stat *__restrict __buf) __THROW __nonnull ((1,
2));
> 
> /* Get file attributes for the file, device, pipe, or socket
>    that file descriptor FD is open on and put them in BUF.  */
> extern int fstat (int __fd, struct stat *__buf) __THROW __nonnull
((2));
> #else
> # ifdef __REDIRECT_NTH
> extern int __REDIRECT_NTH (stat, (__const char *__restrict __file,
> 				  struct stat *__restrict __buf),
stat64)
>      __nonnull ((1, 2));
> extern int __REDIRECT_NTH (fstat, (int __fd, struct stat *__buf),
fstat64)
>      __nonnull ((2));
> # else
> #  define stat stat64
> #  define fstat fstat64
> # endif
> #endif
> 
> As can be seen above, if __USE_FILE_OFFSET64 is defined together 
> with __REDIRECT_NTH then stat() will be aliased to stat64(). 
> However, struct stat will _not_ be the same as struct stat64!
> 
> Then if one further looks in sysdeps/linux/common/bits/stat.h one 
> notices that the declaration of struct stat contains a number of 
> #ifndef __USE_FILE_OFFSET64 which seem to try to make the struct
> look the same as struct stat64 when __USE_FILE_OFFSET64 is defined.
> However, there is a minor difference: the two padding fields are
> unsigned short int in struct stat and unsigned int in struct stat64.
> 
> The reason that this is not a problem on i386 is that the compiler
> actually pads the two pad fields so they are actually four bytes.
> On CRIS, however there is no reason for the compiler to do this
> padding and the resulting structures are thus not the same...
> 
> The attached patch fixes this. I think it should be ok to apply 
> since uClibc already provide both stat() and stat64() so applications
> which call either should continue to work. And those that thought
> they called stat() but actually called stat64() will continue to
> be broken (on architectures with packed structures like CRIS), until 
> they are recompiled when they will magically start to work...
> 
> An alternative approach would be to change the two pad fields in 
> struct stat to be unsigned long. This should still generate the 
> same binary object on i386, but also work for architectures with 
> packed structures (after recompilation).
> 
> What I do not understand is why that padding is there in the first
> place. My guess is that it is a remnant from when dev_t was two 
> bytes in size, but today it is eight bytes, and that padding 
> actually makes the structure fields unaligned...
> 
> So the best would have been to remove the padding altogether. 
> This is of course not binary compatible with previous releases...
> 
> //Peter

To further follow up on this, one of my colleagues pointed me to this 
mail conversation at 
http://sourceware.org/ml/libc-alpha/2003-06/msg00006.html where the 
exact same patch I proposed was suggested for glibc with the same 
rationale. Why the patch has still not been applied to glibc 3.5 years 
later I can only attribute to Hans-Peter giving up fighting the glibc 
management, especially as there did not seem to be any objections to 
the patch.

So, if I do not get any objections as to why this patch should not be
applied, I will do so in a day or two.

//Peter