powerpc: Fix compiler warning on some syscalls
Commit Message
GCC 5.0 emits an warning when using sizeof on array function parameters
and powerpc internal syscall macros add an check for such cases. More
specifically, on powerpc64 and powerpc32 sysdep.h:
if (__builtin_classify_type (__arg3) != 5 && sizeof (__arg3) > 8) \
__illegally_sized_syscall_arg3 (); \
And for sysdeps/unix/sysv/linux/utimensat.c build GCC emits:
error: ‘sizeof’ on array function parameter ‘tsp’ will return size of
‘const struct timespec *’
This patch adds explicit casts to struct pointers to avoid the warnings.
Checked on powerpc64 and powerpc32. Ok to push?
PS: it turned out my earlier checks were incorrect and I didn't see the
issue when I updated my GCC because I used --disable-werror.
--
* sysdeps/unix/sysv/linux/futimens.c (futimens): Cast timespec struct
argument to pointer.
* sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
* sysdeps/unix/sysv/linux/futimesat.c (futimesat): Cast timeval struct
argument to pointer.
* sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
--
Comments
On Tue, Jan 06, 2015 at 11:10:38AM -0200, Adhemerval Zanella wrote:
> GCC 5.0 emits an warning when using sizeof on array function parameters
> and powerpc internal syscall macros add an check for such cases. More
> specifically, on powerpc64 and powerpc32 sysdep.h:
>
> if (__builtin_classify_type (__arg3) != 5 && sizeof (__arg3) > 8) \
> __illegally_sized_syscall_arg3 (); \
>
> And for sysdeps/unix/sysv/linux/utimensat.c build GCC emits:
>
> error: ‘sizeof’ on array function parameter ‘tsp’ will return size of
> ‘const struct timespec *’
>
> This patch adds explicit casts to struct pointers to avoid the warnings.
>
> Checked on powerpc64 and powerpc32. Ok to push?
>
> PS: it turned out my earlier checks were incorrect and I didn't see the
> issue when I updated my GCC because I used --disable-werror.
>
> --
>
> * sysdeps/unix/sysv/linux/futimens.c (futimens): Cast timespec struct
> argument to pointer.
> * sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
> * sysdeps/unix/sysv/linux/futimesat.c (futimesat): Cast timeval struct
> argument to pointer.
> * sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
>
> --
>
> diff --git a/sysdeps/unix/sysv/linux/futimens.c b/sysdeps/unix/sysv/linux/futimens.c
> index c7d03a8..c0e7219 100644
> --- a/sysdeps/unix/sysv/linux/futimens.c
> +++ b/sysdeps/unix/sysv/linux/futimens.c
> @@ -37,7 +37,11 @@ futimens (int fd, const struct timespec tsp[2])
> __set_errno (EBADF);
> return -1;
> }
> - return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
> + /* Some archs (powerpc) add arguments type and size check using sizeof
> + and without a cast the compiler might emit an warning about using
> + sizeof on a struct (where the builtin returns the pointer size). */
> + return INLINE_SYSCALL (utimensat, 4, fd, NULL, (const struct timespec*)tsp,
> + 0);
> #else
> __set_errno (ENOSYS);
> return -1;
> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
> index ac96e2a..f7d5645 100644
> --- a/sysdeps/unix/sysv/linux/futimesat.c
> +++ b/sysdeps/unix/sysv/linux/futimesat.c
> @@ -28,13 +28,13 @@
> /* Change the access time of FILE relative to FD to TVP[0] and
> the modification time of FILE to TVP[1]. */
> int
> -futimesat (fd, file, tvp)
> - int fd;
> - const char *file;
> - const struct timeval tvp[2];
> +futimesat (int fd, const char *file, const struct timeval tvp[2])
> {
> if (file == NULL)
> return __futimes (fd, tvp);
>
> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
> + /* Some archs (powerpc) add arguments type and size check using sizeof
> + and without a cast the compiler might emit an warning about using
> + sizeof on a struct (where the builtin returns the pointer size). */
> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
I think it'd be better to just change the function parameter to const
struct timeval *tvp; the warning is meant to detect a case when a sizeof
is applied to a function parameter declared as an array.
Marek
On 06-01-2015 11:18, Marek Polacek wrote:
> On Tue, Jan 06, 2015 at 11:10:38AM -0200, Adhemerval Zanella wrote:
>> GCC 5.0 emits an warning when using sizeof on array function parameters
>> and powerpc internal syscall macros add an check for such cases. More
>> specifically, on powerpc64 and powerpc32 sysdep.h:
>>
>> if (__builtin_classify_type (__arg3) != 5 && sizeof (__arg3) > 8) \
>> __illegally_sized_syscall_arg3 (); \
>>
>> And for sysdeps/unix/sysv/linux/utimensat.c build GCC emits:
>>
>> error: ‘sizeof’ on array function parameter ‘tsp’ will return size of
>> ‘const struct timespec *’
>>
>> This patch adds explicit casts to struct pointers to avoid the warnings.
>>
>> Checked on powerpc64 and powerpc32. Ok to push?
>>
>> PS: it turned out my earlier checks were incorrect and I didn't see the
>> issue when I updated my GCC because I used --disable-werror.
>>
>> --
>>
>> * sysdeps/unix/sysv/linux/futimens.c (futimens): Cast timespec struct
>> argument to pointer.
>> * sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
>> * sysdeps/unix/sysv/linux/futimesat.c (futimesat): Cast timeval struct
>> argument to pointer.
>> * sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
>>
>> --
>>
>> diff --git a/sysdeps/unix/sysv/linux/futimens.c b/sysdeps/unix/sysv/linux/futimens.c
>> index c7d03a8..c0e7219 100644
>> --- a/sysdeps/unix/sysv/linux/futimens.c
>> +++ b/sysdeps/unix/sysv/linux/futimens.c
>> @@ -37,7 +37,11 @@ futimens (int fd, const struct timespec tsp[2])
>> __set_errno (EBADF);
>> return -1;
>> }
>> - return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
>> + /* Some archs (powerpc) add arguments type and size check using sizeof
>> + and without a cast the compiler might emit an warning about using
>> + sizeof on a struct (where the builtin returns the pointer size). */
>> + return INLINE_SYSCALL (utimensat, 4, fd, NULL, (const struct timespec*)tsp,
>> + 0);
>> #else
>> __set_errno (ENOSYS);
>> return -1;
>> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
>> index ac96e2a..f7d5645 100644
>> --- a/sysdeps/unix/sysv/linux/futimesat.c
>> +++ b/sysdeps/unix/sysv/linux/futimesat.c
>> @@ -28,13 +28,13 @@
>> /* Change the access time of FILE relative to FD to TVP[0] and
>> the modification time of FILE to TVP[1]. */
>> int
>> -futimesat (fd, file, tvp)
>> - int fd;
>> - const char *file;
>> - const struct timeval tvp[2];
>> +futimesat (int fd, const char *file, const struct timeval tvp[2])
>> {
>> if (file == NULL)
>> return __futimes (fd, tvp);
>>
>> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
>> + /* Some archs (powerpc) add arguments type and size check using sizeof
>> + and without a cast the compiler might emit an warning about using
>> + sizeof on a struct (where the builtin returns the pointer size). */
>> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
> I think it'd be better to just change the function parameter to const
> struct timeval *tvp; the warning is meant to detect a case when a sizeof
> is applied to a function parameter declared as an array.
>
> Marek
>
I personally don't have a preference, this suggestion was given by Joseph in a
previous threads requesting for comments. I only see that pushing your suggestion
will require more changes (the headers and manual).
On Tue, Jan 06, 2015 at 11:41:48AM -0200, Adhemerval Zanella wrote:
> On 06-01-2015 11:18, Marek Polacek wrote:
> > On Tue, Jan 06, 2015 at 11:10:38AM -0200, Adhemerval Zanella wrote:
> >> GCC 5.0 emits an warning when using sizeof on array function parameters
> >> and powerpc internal syscall macros add an check for such cases. More
> >> specifically, on powerpc64 and powerpc32 sysdep.h:
> >>
> >> if (__builtin_classify_type (__arg3) != 5 && sizeof (__arg3) > 8) \
> >> __illegally_sized_syscall_arg3 (); \
> >>
> >> And for sysdeps/unix/sysv/linux/utimensat.c build GCC emits:
> >>
> >> error: ‘sizeof’ on array function parameter ‘tsp’ will return size of
> >> ‘const struct timespec *’
> >>
> >> This patch adds explicit casts to struct pointers to avoid the warnings.
> >>
> >> Checked on powerpc64 and powerpc32. Ok to push?
> >>
> >> PS: it turned out my earlier checks were incorrect and I didn't see the
> >> issue when I updated my GCC because I used --disable-werror.
> >>
> >> --
> >>
> >> * sysdeps/unix/sysv/linux/futimens.c (futimens): Cast timespec struct
> >> argument to pointer.
> >> * sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
> >> * sysdeps/unix/sysv/linux/futimesat.c (futimesat): Cast timeval struct
> >> argument to pointer.
> >> * sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
> >>
> >> --
> >>
> >> diff --git a/sysdeps/unix/sysv/linux/futimens.c b/sysdeps/unix/sysv/linux/futimens.c
> >> index c7d03a8..c0e7219 100644
> >> --- a/sysdeps/unix/sysv/linux/futimens.c
> >> +++ b/sysdeps/unix/sysv/linux/futimens.c
> >> @@ -37,7 +37,11 @@ futimens (int fd, const struct timespec tsp[2])
> >> __set_errno (EBADF);
> >> return -1;
> >> }
> >> - return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
> >> + /* Some archs (powerpc) add arguments type and size check using sizeof
> >> + and without a cast the compiler might emit an warning about using
> >> + sizeof on a struct (where the builtin returns the pointer size). */
> >> + return INLINE_SYSCALL (utimensat, 4, fd, NULL, (const struct timespec*)tsp,
> >> + 0);
> >> #else
> >> __set_errno (ENOSYS);
> >> return -1;
> >> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
> >> index ac96e2a..f7d5645 100644
> >> --- a/sysdeps/unix/sysv/linux/futimesat.c
> >> +++ b/sysdeps/unix/sysv/linux/futimesat.c
> >> @@ -28,13 +28,13 @@
> >> /* Change the access time of FILE relative to FD to TVP[0] and
> >> the modification time of FILE to TVP[1]. */
> >> int
> >> -futimesat (fd, file, tvp)
> >> - int fd;
> >> - const char *file;
> >> - const struct timeval tvp[2];
> >> +futimesat (int fd, const char *file, const struct timeval tvp[2])
> >> {
> >> if (file == NULL)
> >> return __futimes (fd, tvp);
> >>
> >> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
> >> + /* Some archs (powerpc) add arguments type and size check using sizeof
> >> + and without a cast the compiler might emit an warning about using
> >> + sizeof on a struct (where the builtin returns the pointer size). */
> >> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
> > I think it'd be better to just change the function parameter to const
> > struct timeval *tvp; the warning is meant to detect a case when a sizeof
> > is applied to a function parameter declared as an array.
> >
> > Marek
> >
> I personally don't have a preference, this suggestion was given by Joseph in a
> previous threads requesting for comments. I only see that pushing your suggestion
> will require more changes (the headers and manual).
Allright, in that case please watch for formatting,
(const struct timeval*)tvp
should be
(const struct timeval *) tvp
Marek
Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes:
> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
> index ac96e2a..f7d5645 100644
> --- a/sysdeps/unix/sysv/linux/futimesat.c
> +++ b/sysdeps/unix/sysv/linux/futimesat.c
> @@ -28,13 +28,13 @@
> /* Change the access time of FILE relative to FD to TVP[0] and
> the modification time of FILE to TVP[1]. */
> int
> -futimesat (fd, file, tvp)
> - int fd;
> - const char *file;
> - const struct timeval tvp[2];
> +futimesat (int fd, const char *file, const struct timeval tvp[2])
> {
> if (file == NULL)
> return __futimes (fd, tvp);
>
> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
> + /* Some archs (powerpc) add arguments type and size check using sizeof
> + and without a cast the compiler might emit an warning about using
> + sizeof on a struct (where the builtin returns the pointer size). */
> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
Does &tvp[0] work instead?
Andreas.
On 06-01-2015 12:29, Andreas Schwab wrote:
> Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes:
>
>> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
>> index ac96e2a..f7d5645 100644
>> --- a/sysdeps/unix/sysv/linux/futimesat.c
>> +++ b/sysdeps/unix/sysv/linux/futimesat.c
>> @@ -28,13 +28,13 @@
>> /* Change the access time of FILE relative to FD to TVP[0] and
>> the modification time of FILE to TVP[1]. */
>> int
>> -futimesat (fd, file, tvp)
>> - int fd;
>> - const char *file;
>> - const struct timeval tvp[2];
>> +futimesat (int fd, const char *file, const struct timeval tvp[2])
>> {
>> if (file == NULL)
>> return __futimes (fd, tvp);
>>
>> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
>> + /* Some archs (powerpc) add arguments type and size check using sizeof
>> + and without a cast the compiler might emit an warning about using
>> + sizeof on a struct (where the builtin returns the pointer size). */
>> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
> Does &tvp[0] work instead?
>
> Andreas.
>
It does and I think using would not require an explicit comment about it
usage. Would it be a preferable solution?
On 01/06/2015 07:10 AM, Adhemerval Zanella wrote:
>> Does &tvp[0] work instead?
>> >
>> >Andreas.
>> >
> It does and I think using would not require an explicit comment about it
> usage. Would it be a preferable solution?
>
Absolutely. Casts are too powerful and can get one into trouble too easily.
@@ -37,7 +37,11 @@ futimens (int fd, const struct timespec tsp[2])
__set_errno (EBADF);
return -1;
}
- return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
+ /* Some archs (powerpc) add arguments type and size check using sizeof
+ and without a cast the compiler might emit an warning about using
+ sizeof on a struct (where the builtin returns the pointer size). */
+ return INLINE_SYSCALL (utimensat, 4, fd, NULL, (const struct timespec*)tsp,
+ 0);
#else
__set_errno (ENOSYS);
return -1;
@@ -28,13 +28,13 @@
/* Change the access time of FILE relative to FD to TVP[0] and
the modification time of FILE to TVP[1]. */
int
-futimesat (fd, file, tvp)
- int fd;
- const char *file;
- const struct timeval tvp[2];
+futimesat (int fd, const char *file, const struct timeval tvp[2])
{
if (file == NULL)
return __futimes (fd, tvp);
- return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
+ /* Some archs (powerpc) add arguments type and size check using sizeof
+ and without a cast the compiler might emit an warning about using
+ sizeof on a struct (where the builtin returns the pointer size). */
+ return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
}
@@ -35,7 +35,11 @@ utimensat (int fd, const char *file, const struct timespec tsp[2],
return -1;
}
#ifdef __NR_utimensat
- return INLINE_SYSCALL (utimensat, 4, fd, file, tsp, flags);
+ /* Some archs (powerpc) add arguments type and size check using sizeof
+ and without a cast the compiler might emit an warning about using
+ sizeof on a struct (where the builtin returns the pointer size). */
+ return INLINE_SYSCALL (utimensat, 4, fd, file, (const struct timespec*)tsp,
+ flags);
#else
__set_errno (ENOSYS);
return -1;
@@ -29,7 +29,10 @@
int
__utimes (const char *file, const struct timeval tvp[2])
{
- return INLINE_SYSCALL (utimes, 2, file, tvp);
+ /* Some archs (powerpc) add arguments type and size check using sizeof
+ and without a cast the compiler might emit an warning about using
+ sizeof on a struct (where the builtin returns the pointer size). */
+ return INLINE_SYSCALL (utimes, 2, file, (const struct timeval*)tvp);
}
weak_alias (__utimes, utimes)