[RFC,2/6] y2038: stat: {f}stat{at}64_time64 redirection to be used on Y2038 systems

Message ID 20201204233604.7430-3-lukma@denx.de
State Superseded
Headers
Series y2038: Prepare glibc to be Y2038 safe for 32 bit ports |

Commit Message

Lukasz Majewski Dec. 4, 2020, 11:36 p.m. UTC
  ---
 io/Versions   |  4 +++
 io/sys/stat.h | 80 +++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 8 deletions(-)
  

Comments

Joseph Myers Dec. 5, 2020, 12:04 a.m. UTC | #1
On Sat, 5 Dec 2020, Lukasz Majewski wrote:

> --- a/io/Versions
> +++ b/io/Versions
> @@ -148,5 +148,9 @@ libc {
>      __fstat64;
>      __ppoll64;
>      __utime64;
> +    __stat64_time64;
> +    __fstat64_time64;
> +    __lstat64_time64;
> +    __fstatat64_time64;

This is adding to GLIBC_PRIVATE.

> --- a/io/sys/stat.h
> +++ b/io/sys/stat.h
> @@ -210,20 +210,45 @@ extern int stat (const char *__restrict __file,
>  extern int fstat (int __fd, struct stat *__buf) __THROW __nonnull ((2));
>  #else
>  # ifdef __REDIRECT_NTH
> +#  ifdef __USE_TIME_BITS64
> +extern int __REDIRECT_NTH (stat, (const char *__restrict __file,
> +                                    struct stat *__restrict __buf),
> +                           __stat64_time64) __nonnull ((1, 2));
> +extern int __REDIRECT_NTH (fstat, (int __fd, struct stat *__buf),
> +                           __fstat64_time64) __nonnull ((2));

And this is changing an installed header.

An installed header should never reference a GLIBC_PRIVATE symbol.  The 
commit that adds a redirection to a new symbol for 64-bit time_t must 
either be the same commit that adds that symbol to a *public* symbol 
version, or a subsequent commit.  The commit adding it to a public symbol 
version must be the same commit that updates all relevant ABI test 
baselines so that ABI tests pass after every commit.

The same comments apply to other patches in this series.
  
Lukasz Majewski Dec. 7, 2020, 10:28 a.m. UTC | #2
Hi Joseph,

> On Sat, 5 Dec 2020, Lukasz Majewski wrote:
> 
> > --- a/io/Versions
> > +++ b/io/Versions
> > @@ -148,5 +148,9 @@ libc {
> >      __fstat64;
> >      __ppoll64;
> >      __utime64;
> > +    __stat64_time64;
> > +    __fstat64_time64;
> > +    __lstat64_time64;
> > +    __fstatat64_time64;  
> 
> This is adding to GLIBC_PRIVATE.

This shall be added to GLIBC_2.3X.

> 
> > --- a/io/sys/stat.h
> > +++ b/io/sys/stat.h
> > @@ -210,20 +210,45 @@ extern int stat (const char *__restrict
> > __file, extern int fstat (int __fd, struct stat *__buf) __THROW
> > __nonnull ((2)); #else
> >  # ifdef __REDIRECT_NTH
> > +#  ifdef __USE_TIME_BITS64
> > +extern int __REDIRECT_NTH (stat, (const char *__restrict __file,
> > +                                    struct stat *__restrict __buf),
> > +                           __stat64_time64) __nonnull ((1, 2));
> > +extern int __REDIRECT_NTH (fstat, (int __fd, struct stat *__buf),
> > +                           __fstat64_time64) __nonnull ((2));  
> 
> And this is changing an installed header.
> 
> An installed header should never reference a GLIBC_PRIVATE symbol.

A side question:

Is my understanding correct that GLIBC_PRIVATE symbols are only for
cross libraries reference? For example such symbol from glibc could be
referenced in librt?

> The commit that adds a redirection to a new symbol for 64-bit time_t
> must either be the same commit that adds that symbol to a *public*
> symbol version, or a subsequent commit.  The commit adding it to a
> public symbol version must be the same commit that updates all
> relevant ABI test baselines so that ABI tests pass after every commit.
> 
> The same comments apply to other patches in this series.
> 

This commit shall be squashed with the first one - I just defer to do
this as there are still some issues with 'stat{x}'.



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
  
Joseph Myers Dec. 7, 2020, 6:07 p.m. UTC | #3
On Mon, 7 Dec 2020, Lukasz Majewski wrote:

> Is my understanding correct that GLIBC_PRIVATE symbols are only for
> cross libraries reference? For example such symbol from glibc could be
> referenced in librt?

GLIBC_PRIVATE symbols are only for reference by libraries or programs 
installed by glibc.  So, yes, a symbol provided by libc.so for use from 
librt.so could be GLIBC_PRIVATE, if it's not intended for users to link 
their own code against and is not referenced in any installed header or 
*_nonshared.a library or crt*.o.  Sometimes such symbols may be used in 
the programs glibc installs (in usr/bin, usr/sbin, etc.; e.g. nscd), not 
just libraries, though it's less clear if that's a good idea.

GLIBC_PRIVATE symbols do not need to have any kind of a stable ABI, 
because the definition and user of the symbol always come from the same 
set of glibc sources.  However, ABI changes to such symbols can be 
dangerous on release branches, because of the situation where (a) a 
process starts up and loads libc, (b) an updated distribution package of 
glibc libraries is installed, (c) that process dlopens a glibc shared 
library it didn't load at startup, and that library (from the updated 
package) wants a GLIBC_PRIVATE symbol that's not present with the right 
ABI in the (older) libc it loaded at startup.
  

Patch

diff --git a/io/Versions b/io/Versions
index 15616b68f3..ce34180e30 100644
--- a/io/Versions
+++ b/io/Versions
@@ -148,5 +148,9 @@  libc {
     __fstat64;
     __ppoll64;
     __utime64;
+    __stat64_time64;
+    __fstat64_time64;
+    __lstat64_time64;
+    __fstatat64_time64;
   }
 }
diff --git a/io/sys/stat.h b/io/sys/stat.h
index b3f2f0da99..b89b7b1b7d 100644
--- a/io/sys/stat.h
+++ b/io/sys/stat.h
@@ -210,20 +210,45 @@  extern int stat (const char *__restrict __file,
 extern int fstat (int __fd, struct stat *__buf) __THROW __nonnull ((2));
 #else
 # ifdef __REDIRECT_NTH
+#  ifdef __USE_TIME_BITS64
+extern int __REDIRECT_NTH (stat, (const char *__restrict __file,
+                                    struct stat *__restrict __buf),
+                           __stat64_time64) __nonnull ((1, 2));
+extern int __REDIRECT_NTH (fstat, (int __fd, struct stat *__buf),
+                           __fstat64_time64) __nonnull ((2));
+#  else
 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));
+#  endif
 # else
-#  define stat stat64
-#  define fstat fstat64
+#  ifdef __USE_TIME_BITS64
+#   define stat __stat64_time64
+#   define fstat __fstat64_time64
+#  else
+#   define stat stat64
+#   define fstat fstat64
+#  endif
 # endif
 #endif
 #ifdef __USE_LARGEFILE64
 extern int stat64 (const char *__restrict __file,
 		   struct stat64 *__restrict __buf) __THROW __nonnull ((1, 2));
 extern int fstat64 (int __fd, struct stat64 *__buf) __THROW __nonnull ((2));
+# ifdef __USE_TIME_BITS64
+#  if defined(__REDIRECT_NTH)
+extern int __REDIRECT_NTH (stat64, (const char *__restrict __file,
+                                    struct stat64 *__restrict __buf),
+                           __stat64_time64) __nonnull ((1, 2));
+extern int __REDIRECT_NTH (fstat64, (int __fd, struct stat64 *__buf),
+                           __fstat64_time64) __nonnull ((2));
+#  else
+#   define stat64 __stat64_time64
+#   define fstat64 __fstat64_time64
+#  endif
+# endif
 #endif
 
 #ifdef __USE_ATFILE
@@ -236,19 +261,39 @@  extern int fstatat (int __fd, const char *__restrict __file,
      __THROW __nonnull ((2, 3));
 # else
 #  ifdef __REDIRECT_NTH
+#   ifdef __USE_TIME_BITS64
+extern int __REDIRECT_NTH (fstatat, (int __fd, const char *__restrict __file,
+                                     struct stat *__restrict __buf, int __flag),
+                           __fstatat64_time64) __nonnull ((2, 3));
+#   else
 extern int __REDIRECT_NTH (fstatat, (int __fd, const char *__restrict __file,
 				     struct stat *__restrict __buf,
 				     int __flag),
 			   fstatat64) __nonnull ((2, 3));
+#   endif /* __USE_TIME_BITS64 */
 #  else
-#   define fstatat fstatat64
+#   ifdef __USE_TIME_BITS64
+#    define fstatat __fstatat64_time64
+#   else
+#    define fstatat fstatat64
+#   endif
 #  endif
-# endif
+# endif /* __USE_FILE_OFFSET64 */
 
 # ifdef __USE_LARGEFILE64
 extern int fstatat64 (int __fd, const char *__restrict __file,
 		      struct stat64 *__restrict __buf, int __flag)
      __THROW __nonnull ((2, 3));
+#  ifdef __USE_TIME_BITS64
+#   if defined(__REDIRECT_NTH)
+extern int __REDIRECT_NTH (fstatat64,
+                           (int __fd, const char *__restrict __file,
+                            struct stat64 *__restrict __buf, int __flag),
+                           __fstatat64_time64) __nonnull ((2, 3));
+#   else
+#    define fstatat64 __fstatat64_time64
+#   endif
+#  endif /* __USE_TIME_BITS64 */
 # endif
 #endif
 
@@ -260,19 +305,38 @@  extern int lstat (const char *__restrict __file,
 		  struct stat *__restrict __buf) __THROW __nonnull ((1, 2));
 # else
 #  ifdef __REDIRECT_NTH
+#   ifdef __USE_TIME_BITS64
+extern int __REDIRECT_NTH (lstat, (const char *__restrict __file,
+                                   struct stat *__restrict __buf),
+                           __lstat64_time64) __nonnull ((1, 2));
+#   else
 extern int __REDIRECT_NTH (lstat,
 			   (const char *__restrict __file,
 			    struct stat *__restrict __buf), lstat64)
      __nonnull ((1, 2));
+#   endif /* __USE_TIME_BITS64 */
 #  else
-#   define lstat lstat64
-#  endif
-# endif
+#   ifdef __USE_TIME_BITS64
+#    define lstat __lstat64_time64
+#   else
+#    define lstat lstat64
+#   endif
+#  endif /* __REDIRECT_NTH */
+# endif /* __USE_FILE_OFFSET64 */
 # ifdef __USE_LARGEFILE64
 extern int lstat64 (const char *__restrict __file,
 		    struct stat64 *__restrict __buf)
      __THROW __nonnull ((1, 2));
-# endif
+#  ifdef __USE_TIME_BITS64
+#   if defined(__REDIRECT_NTH)
+extern int __REDIRECT_NTH (lstat64, (const char *__restrict __file,
+                                     struct stat64 *__restrict __buf),
+                           __lstat64_time64) __nonnull ((1, 2));
+#   else
+#    define lstat64 __lstat64_time64
+#   endif
+#  endif /* __USE_TIME_BITS64 */
+# endif /* __USE_LARGEFILE64 */
 #endif
 
 /* Set file access permissions for FILE to MODE.