open64: Force O_LARGEFILE on 32-bit architectures
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
When running tests on OpenRISC which has 32-bit wordsize but 64-bit
timesize it was found that O_LARGEFILE is not being set when calling
open64. For 64-bit architectures the O_LARGEFILE flag is generally
implied by the kernel according to force_o_largefile. However, for
32-bit architectures this is not done.
For this patch I add an extra condition to the EXTRA_OPEN_FLAGS ifdef
logic to allow setting O_LARGEFILE on 32-bit architectures.
Tested on the OpenRISC the build works and timezone/tst-tzset passes
which was failing before. I would expect this also would fix arc.
---
sysdeps/unix/sysv/linux/open64.c | 3 +--
sysdeps/unix/sysv/linux/open64_nocancel.c | 2 +-
sysdeps/unix/sysv/linux/openat64.c | 2 +-
sysdeps/unix/sysv/linux/openat64_nocancel.c | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
Comments
On 26/12/2021 04:20, Stafford Horne via Libc-alpha wrote:
> When running tests on OpenRISC which has 32-bit wordsize but 64-bit
> timesize it was found that O_LARGEFILE is not being set when calling
> open64. For 64-bit architectures the O_LARGEFILE flag is generally
> implied by the kernel according to force_o_largefile. However, for
> 32-bit architectures this is not done.
>
> For this patch I add an extra condition to the EXTRA_OPEN_FLAGS ifdef
> logic to allow setting O_LARGEFILE on 32-bit architectures.
>
> Tested on the OpenRISC the build works and timezone/tst-tzset passes
> which was failing before. I would expect this also would fix arc.
The O_LARGEFILE is implicit for architectures that defines ARCH_32BIT_OFF_T,
which are usually old legacy ABIs (it is not defined for newer 32-bit ABIs).
However, Linux will also internally adds the flag for such cases
(force_o_largefile macro), which means that passing it, even for ABIs that
do not require it, is ok.
So I think it would be simpler to just remove EXTRA_OPEN_FLAGS and
use O_LARGEFILE unconditionally.
> ---
> sysdeps/unix/sysv/linux/open64.c | 3 +--
> sysdeps/unix/sysv/linux/open64_nocancel.c | 2 +-
> sysdeps/unix/sysv/linux/openat64.c | 2 +-
> sysdeps/unix/sysv/linux/openat64_nocancel.c | 2 +-
> 4 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
> index 0904696973..5ae5e98b57 100644
> --- a/sysdeps/unix/sysv/linux/open64.c
> +++ b/sysdeps/unix/sysv/linux/open64.c
> @@ -23,8 +23,7 @@
> #include <sysdep-cancel.h>
> #include <shlib-compat.h>
>
> -
> -#ifdef __OFF_T_MATCHES_OFF64_T
> +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> # define EXTRA_OPEN_FLAGS 0
> #else
> # define EXTRA_OPEN_FLAGS O_LARGEFILE
> diff --git a/sysdeps/unix/sysv/linux/open64_nocancel.c b/sysdeps/unix/sysv/linux/open64_nocancel.c
> index d7f35656a1..0fe41175ac 100644
> --- a/sysdeps/unix/sysv/linux/open64_nocancel.c
> +++ b/sysdeps/unix/sysv/linux/open64_nocancel.c
> @@ -23,7 +23,7 @@
>
> #include <not-cancel.h>
>
> -#ifdef __OFF_T_MATCHES_OFF64_T
> +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> # define EXTRA_OPEN_FLAGS 0
> #else
> # define EXTRA_OPEN_FLAGS O_LARGEFILE
> diff --git a/sysdeps/unix/sysv/linux/openat64.c b/sysdeps/unix/sysv/linux/openat64.c
> index dc226567c1..1019fc13ac 100644
> --- a/sysdeps/unix/sysv/linux/openat64.c
> +++ b/sysdeps/unix/sysv/linux/openat64.c
> @@ -21,7 +21,7 @@
>
> #include <sysdep-cancel.h>
>
> -#ifdef __OFF_T_MATCHES_OFF64_T
> +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> # define EXTRA_OPEN_FLAGS 0
> #else
> # define EXTRA_OPEN_FLAGS O_LARGEFILE
> diff --git a/sysdeps/unix/sysv/linux/openat64_nocancel.c b/sysdeps/unix/sysv/linux/openat64_nocancel.c
> index 51377aea45..324f719ea4 100644
> --- a/sysdeps/unix/sysv/linux/openat64_nocancel.c
> +++ b/sysdeps/unix/sysv/linux/openat64_nocancel.c
> @@ -22,7 +22,7 @@
> #include <sysdep-cancel.h>
> #include <not-cancel.h>
>
> -#ifdef __OFF_T_MATCHES_OFF64_T
> +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> # define EXTRA_OPEN_FLAGS 0
> #else
> # define EXTRA_OPEN_FLAGS O_LARGEFILE
On Tue, Dec 28, 2021 at 04:43:05PM -0300, Adhemerval Zanella wrote:
>
>
> On 26/12/2021 04:20, Stafford Horne via Libc-alpha wrote:
> > When running tests on OpenRISC which has 32-bit wordsize but 64-bit
> > timesize it was found that O_LARGEFILE is not being set when calling
> > open64. For 64-bit architectures the O_LARGEFILE flag is generally
> > implied by the kernel according to force_o_largefile. However, for
> > 32-bit architectures this is not done.
> >
> > For this patch I add an extra condition to the EXTRA_OPEN_FLAGS ifdef
> > logic to allow setting O_LARGEFILE on 32-bit architectures.
> >
> > Tested on the OpenRISC the build works and timezone/tst-tzset passes
> > which was failing before. I would expect this also would fix arc.
>
> The O_LARGEFILE is implicit for architectures that defines ARCH_32BIT_OFF_T,
> which are usually old legacy ABIs (it is not defined for newer 32-bit ABIs).
>
> However, Linux will also internally adds the flag for such cases
> (force_o_largefile macro), which means that passing it, even for ABIs that
> do not require it, is ok.
>
> So I think it would be simpler to just remove EXTRA_OPEN_FLAGS and
> use O_LARGEFILE unconditionally.
Yes, I agree on this. I thought maybe there was some reason for having the
ifdef there, like avoiding the extra instructions to set the flag. But, I don't
think it's going to show up on any profiles.
I'll send a v2.
> > ---
> > sysdeps/unix/sysv/linux/open64.c | 3 +--
> > sysdeps/unix/sysv/linux/open64_nocancel.c | 2 +-
> > sysdeps/unix/sysv/linux/openat64.c | 2 +-
> > sysdeps/unix/sysv/linux/openat64_nocancel.c | 2 +-
> > 4 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
> > index 0904696973..5ae5e98b57 100644
> > --- a/sysdeps/unix/sysv/linux/open64.c
> > +++ b/sysdeps/unix/sysv/linux/open64.c
> > @@ -23,8 +23,7 @@
> > #include <sysdep-cancel.h>
> > #include <shlib-compat.h>
> >
> > -
> > -#ifdef __OFF_T_MATCHES_OFF64_T
> > +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> > # define EXTRA_OPEN_FLAGS 0
> > #else
> > # define EXTRA_OPEN_FLAGS O_LARGEFILE
> > diff --git a/sysdeps/unix/sysv/linux/open64_nocancel.c b/sysdeps/unix/sysv/linux/open64_nocancel.c
> > index d7f35656a1..0fe41175ac 100644
> > --- a/sysdeps/unix/sysv/linux/open64_nocancel.c
> > +++ b/sysdeps/unix/sysv/linux/open64_nocancel.c
> > @@ -23,7 +23,7 @@
> >
> > #include <not-cancel.h>
> >
> > -#ifdef __OFF_T_MATCHES_OFF64_T
> > +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> > # define EXTRA_OPEN_FLAGS 0
> > #else
> > # define EXTRA_OPEN_FLAGS O_LARGEFILE
> > diff --git a/sysdeps/unix/sysv/linux/openat64.c b/sysdeps/unix/sysv/linux/openat64.c
> > index dc226567c1..1019fc13ac 100644
> > --- a/sysdeps/unix/sysv/linux/openat64.c
> > +++ b/sysdeps/unix/sysv/linux/openat64.c
> > @@ -21,7 +21,7 @@
> >
> > #include <sysdep-cancel.h>
> >
> > -#ifdef __OFF_T_MATCHES_OFF64_T
> > +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> > # define EXTRA_OPEN_FLAGS 0
> > #else
> > # define EXTRA_OPEN_FLAGS O_LARGEFILE
> > diff --git a/sysdeps/unix/sysv/linux/openat64_nocancel.c b/sysdeps/unix/sysv/linux/openat64_nocancel.c
> > index 51377aea45..324f719ea4 100644
> > --- a/sysdeps/unix/sysv/linux/openat64_nocancel.c
> > +++ b/sysdeps/unix/sysv/linux/openat64_nocancel.c
> > @@ -22,7 +22,7 @@
> > #include <sysdep-cancel.h>
> > #include <not-cancel.h>
> >
> > -#ifdef __OFF_T_MATCHES_OFF64_T
> > +#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
> > # define EXTRA_OPEN_FLAGS 0
> > #else
> > # define EXTRA_OPEN_FLAGS O_LARGEFILE
@@ -23,8 +23,7 @@
#include <sysdep-cancel.h>
#include <shlib-compat.h>
-
-#ifdef __OFF_T_MATCHES_OFF64_T
+#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
# define EXTRA_OPEN_FLAGS 0
#else
# define EXTRA_OPEN_FLAGS O_LARGEFILE
@@ -23,7 +23,7 @@
#include <not-cancel.h>
-#ifdef __OFF_T_MATCHES_OFF64_T
+#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
# define EXTRA_OPEN_FLAGS 0
#else
# define EXTRA_OPEN_FLAGS O_LARGEFILE
@@ -21,7 +21,7 @@
#include <sysdep-cancel.h>
-#ifdef __OFF_T_MATCHES_OFF64_T
+#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
# define EXTRA_OPEN_FLAGS 0
#else
# define EXTRA_OPEN_FLAGS O_LARGEFILE
@@ -22,7 +22,7 @@
#include <sysdep-cancel.h>
#include <not-cancel.h>
-#ifdef __OFF_T_MATCHES_OFF64_T
+#if defined __OFF_T_MATCHES_OFF64_T && (__WORDSIZE != 32)
# define EXTRA_OPEN_FLAGS 0
#else
# define EXTRA_OPEN_FLAGS O_LARGEFILE