diff mbox series

open64: Force O_LARGEFILE on 32-bit architectures

Message ID 20211226072008.1575854-1-shorne@gmail.com
State Superseded
Headers
Series 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

Stafford Horne Dec. 26, 2021, 7:20 a.m. UTC 
  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

Adhemerval Zanella Netto Dec. 28, 2021, 7:43 p.m. UTC | #1 
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
Stafford Horne Dec. 28, 2021, 9:53 p.m. UTC | #2 
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
diff mbox series

Patch

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