diff mbox series

support: Use __utimensat64 to set file access and modification times

Message ID 20210317153810.12070-1-lukma@denx.de
State Superseded
Headers
Series support: Use __utimensat64 to set file access and modification times |

Commit Message

Lukasz Majewski March 17, 2021, 3:38 p.m. UTC 
  Before this patch the ARM port required the __libc_do_syscall
function to be able to call utimensat_time64 syscall required to
check if file system supports 64 bit time.

This patch fixes the following error on ARM 32 bit port:

y2038-glibc/support/support_path_support_time64.c:34: undefined reference to
`__libc_do_syscall'
collect2: error: ld returned 1 exit status

As it now calls the __utimensat64 glibc exported function, which
supports 64 bit time.
---
 support/support_path_support_time64.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

Comments

Florian Weimer March 17, 2021, 6:05 p.m. UTC | #1 
* Lukasz Majewski:

> @@ -49,8 +38,9 @@ support_path_support_time64 (const char *path)
>      { 0x80000001ULL, 0 },
>      { 0x80000002ULL, 0 }
>    };
> -  /* Return is kernel does not support __NR_utimensat_time64.  */
> -  if (utimesat_call (path, tsp) == -1)
> +
> +  /* Return if kernel does not support __NR_utimensat_time64.  */
> +  if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0) == -1)
>      return false;
>  
>    /* Verify if the last access and last modification time match the ones
> @@ -67,7 +57,7 @@ support_path_support_time64 (const char *path)
>      { ostx.stx_atime.tv_sec, ostx.stx_atime.tv_nsec },
>      { ostx.stx_mtime.tv_sec, ostx.stx_mtime.tv_nsec },
>    };
> -  TEST_VERIFY_EXIT (utimesat_call (path, otsp) == 0);
> +  TEST_VERIFY_EXIT (__utimensat64 (AT_FDCWD, path, &otsp[0], 0) == 0);
>  
>    return support;
>  #else

I don't see how this can work given that __utimensat64 is currently a
symbol exported from libc.so.6.  Doesn't this result in link failures
on, say, i386?

Thanks,
Florian
Adhemerval Zanella March 17, 2021, 6:38 p.m. UTC | #2 
On 17/03/2021 15:05, Florian Weimer wrote:
> * Lukasz Majewski:
> 
>> @@ -49,8 +38,9 @@ support_path_support_time64 (const char *path)
>>      { 0x80000001ULL, 0 },
>>      { 0x80000002ULL, 0 }
>>    };
>> -  /* Return is kernel does not support __NR_utimensat_time64.  */
>> -  if (utimesat_call (path, tsp) == -1)
>> +
>> +  /* Return if kernel does not support __NR_utimensat_time64.  */
>> +  if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0) == -1)
>>      return false;
>>  
>>    /* Verify if the last access and last modification time match the ones
>> @@ -67,7 +57,7 @@ support_path_support_time64 (const char *path)
>>      { ostx.stx_atime.tv_sec, ostx.stx_atime.tv_nsec },
>>      { ostx.stx_mtime.tv_sec, ostx.stx_mtime.tv_nsec },
>>    };
>> -  TEST_VERIFY_EXIT (utimesat_call (path, otsp) == 0);
>> +  TEST_VERIFY_EXIT (__utimensat64 (AT_FDCWD, path, &otsp[0], 0) == 0);
>>  
>>    return support;
>>  #else
> 
> I don't see how this can work given that __utimensat64 is currently a
> symbol exported from libc.so.6.  Doesn't this result in link failures
> on, say, i386?

It does not, it either fail with an invalid linking for legacy ABIs:

/libsupport_nonshared.a(support_path_support_time64.oS): in function `support_path_support_time64':
/home/azanella/glibc/glibc-git/support/support_path_support_time64.c:43: undefined reference to `__utimensat64'
/home/azanella/toolchain/install/compilers/arm-linux-gnueabihf/lib/gcc/arm-glibc-linux-gnueabihf/10.2.1/../../../../arm-glibc-linux-gnueabihf/bin/ld: /home/azanella/glibc/glibc-git/support/support_path_su
pport_time64.c:60: undefined reference to `__utimensat64'

Or with the following for abi with default 64 bit support:

In file included from ../sysdeps/generic/hp-timing.h:23,
                 from ../nptl/descr.h:27,
                 from ../sysdeps/aarch64/nptl/tls.h:44,
                 from ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:29,
                 from support_path_support_time64.c:24:
support_path_support_time64.c: In function ‘support_path_support_time64’:
../include/time.h:180:24: error: implicit declaration of function ‘__utimensat’; did you mean ‘utimensat’? [-Werror=implicit-function-declaration]
  180 | # define __utimensat64 __utimensat
      |                        ^~~~~~~~~~~
support_path_support_time64.c:43:7: note: in expansion of macro ‘__utimensat64’
   43 |   if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0) == -1)


I will commit the following fix:

diff --git a/support/support_path_support_time64.c b/support/support_path_support_time64.c
index 74af7d4973..452fedcde5 100644
--- a/support/support_path_support_time64.c
+++ b/support/support_path_support_time64.c
@@ -31,7 +31,7 @@ utimesat_call (const char *path, const struct __timespec64 tsp[2])
 # ifndef __NR_utimensat_time64
 #  define __NR_utimensat_time64 __NR_utimensat
 # endif
-  return INLINE_SYSCALL_CALL (utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
+  return syscall (__NR_utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
 }
 #endif
Florian Weimer March 17, 2021, 6:53 p.m. UTC | #3 
* Adhemerval Zanella:

> I will commit the following fix:
>
> diff --git a/support/support_path_support_time64.c b/support/support_path_support_time64.c
> index 74af7d4973..452fedcde5 100644
> --- a/support/support_path_support_time64.c
> +++ b/support/support_path_support_time64.c
> @@ -31,7 +31,7 @@ utimesat_call (const char *path, const struct __timespec64 tsp[2])
>  # ifndef __NR_utimensat_time64
>  #  define __NR_utimensat_time64 __NR_utimensat
>  # endif
> -  return INLINE_SYSCALL_CALL (utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
> +  return syscall (__NR_utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
>  }
>  #endif
>  

Thanks, that looks okay to me.

Florian
Lukasz Majewski March 17, 2021, 10:32 p.m. UTC | #4 
Hi Adhemerval,

> On 17/03/2021 15:05, Florian Weimer wrote:
> > * Lukasz Majewski:
> >   
> >> @@ -49,8 +38,9 @@ support_path_support_time64 (const char *path)
> >>      { 0x80000001ULL, 0 },
> >>      { 0x80000002ULL, 0 }
> >>    };
> >> -  /* Return is kernel does not support __NR_utimensat_time64.  */
> >> -  if (utimesat_call (path, tsp) == -1)
> >> +
> >> +  /* Return if kernel does not support __NR_utimensat_time64.  */
> >> +  if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0) == -1)
> >>      return false;
> >>  
> >>    /* Verify if the last access and last modification time match
> >> the ones @@ -67,7 +57,7 @@ support_path_support_time64 (const char
> >> *path) { ostx.stx_atime.tv_sec, ostx.stx_atime.tv_nsec },
> >>      { ostx.stx_mtime.tv_sec, ostx.stx_mtime.tv_nsec },
> >>    };
> >> -  TEST_VERIFY_EXIT (utimesat_call (path, otsp) == 0);
> >> +  TEST_VERIFY_EXIT (__utimensat64 (AT_FDCWD, path, &otsp[0], 0)
> >> == 0); 
> >>    return support;
> >>  #else  
> > 
> > I don't see how this can work given that __utimensat64 is currently
> > a symbol exported from libc.so.6.  Doesn't this result in link
> > failures on, say, i386?  
> 
> It does not, it either fail with an invalid linking for legacy ABIs:
> 
> /libsupport_nonshared.a(support_path_support_time64.oS): in function
> `support_path_support_time64':
> /home/azanella/glibc/glibc-git/support/support_path_support_time64.c:43:
> undefined reference to `__utimensat64'
> /home/azanella/toolchain/install/compilers/arm-linux-gnueabihf/lib/gcc/arm-glibc-linux-gnueabihf/10.2.1/../../../../arm-glibc-linux-gnueabihf/bin/ld:
> /home/azanella/glibc/glibc-git/support/support_path_su
> pport_time64.c:60: undefined reference to `__utimensat64'
> 
> Or with the following for abi with default 64 bit support:
> 
> In file included from ../sysdeps/generic/hp-timing.h:23,
>                  from ../nptl/descr.h:27,
>                  from ../sysdeps/aarch64/nptl/tls.h:44,
>                  from ../sysdeps/unix/sysv/linux/aarch64/sysdep.h:29,
>                  from support_path_support_time64.c:24:
> support_path_support_time64.c: In function
> ‘support_path_support_time64’: ../include/time.h:180:24: error:
> implicit declaration of function ‘__utimensat’; did you mean
> ‘utimensat’? [-Werror=implicit-function-declaration] 180 | # define
> __utimensat64 __utimensat |                        ^~~~~~~~~~~
> support_path_support_time64.c:43:7: note: in expansion of macro
> ‘__utimensat64’ 43 |   if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0)
> == -1)
> 
> 
> I will commit the following fix:
> 
> diff --git a/support/support_path_support_time64.c
> b/support/support_path_support_time64.c index 74af7d4973..452fedcde5
> 100644 --- a/support/support_path_support_time64.c
> +++ b/support/support_path_support_time64.c
> @@ -31,7 +31,7 @@ utimesat_call (const char *path, const struct
> __timespec64 tsp[2]) # ifndef __NR_utimensat_time64
>  #  define __NR_utimensat_time64 __NR_utimensat
>  # endif
> -  return INLINE_SYSCALL_CALL (utimensat_time64, AT_FDCWD, path,
> &tsp[0], 0);
> +  return syscall (__NR_utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
>  }
>  #endif
>  

I've checked it with ARM 32 bit. It works without issues. Thanks for
providing this proper fix.

Tested-by: Lukasz Majewski <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
diff mbox series

Patch

diff --git a/support/support_path_support_time64.c b/support/support_path_support_time64.c
index 74af7d4973..e42350fa43 100644
--- a/support/support_path_support_time64.c
+++ b/support/support_path_support_time64.c
@@ -24,17 +24,6 @@ 
 #include <sysdep.h>
 #endif
 
-#ifdef __linux__
-static int
-utimesat_call (const char *path, const struct __timespec64 tsp[2])
-{
-# ifndef __NR_utimensat_time64
-#  define __NR_utimensat_time64 __NR_utimensat
-# endif
-  return INLINE_SYSCALL_CALL (utimensat_time64, AT_FDCWD, path, &tsp[0], 0);
-}
-#endif
-
 bool
 support_path_support_time64 (const char *path)
 {
@@ -49,8 +38,9 @@  support_path_support_time64 (const char *path)
     { 0x80000001ULL, 0 },
     { 0x80000002ULL, 0 }
   };
-  /* Return is kernel does not support __NR_utimensat_time64.  */
-  if (utimesat_call (path, tsp) == -1)
+
+  /* Return if kernel does not support __NR_utimensat_time64.  */
+  if (__utimensat64 (AT_FDCWD, path, &tsp[0], 0) == -1)
     return false;
 
   /* Verify if the last access and last modification time match the ones
@@ -67,7 +57,7 @@  support_path_support_time64 (const char *path)
     { ostx.stx_atime.tv_sec, ostx.stx_atime.tv_nsec },
     { ostx.stx_mtime.tv_sec, ostx.stx_mtime.tv_nsec },
   };
-  TEST_VERIFY_EXIT (utimesat_call (path, otsp) == 0);
+  TEST_VERIFY_EXIT (__utimensat64 (AT_FDCWD, path, &otsp[0], 0) == 0);
 
   return support;
 #else