diff mbox series

[RFC,3/3] time64.conf: Include to enable 64 bit time flags

Message ID 20221208071136.1967696-4-olani@axis.com
State Accepted, archived
Commit 3aae5d1fd81b53d496da0287b29379b74bd5e8e1
Headers show
Series [RFC,1/3] glibc: Add ppoll fortify symbol for 64 bit time_t | expand

Commit Message

Ola x Nilsson Dec. 8, 2022, 7:11 a.m. UTC
Signed-off-by: Ola x Nilsson <olani@axis.com>
---
 meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 meta/conf/distro/time64.conf

Comments

Alexander Kanavin Dec. 8, 2022, 10 a.m. UTC | #1
On Thu, 8 Dec 2022 at 08:11, Ola x Nilsson <olani@axis.com> wrote:
> +# TODO: Only for 32-bit architectures?
> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"

On the contrary, why not set this globally?

> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> +# Needs further investigation
> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> +
> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so] = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64 setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime localtime_r"
> +
> +# libpulsedsp.so is a preload-library that hooks libc functions
> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"

Some of these perhaps need a ticket in bugzilla, and there should be a
meta-ticket for Y2038 that tracks individual issues (by depending on
them). Can you arrange that please?

Alex
Richard Purdie Dec. 8, 2022, 10:10 a.m. UTC | #2
On Thu, 2022-12-08 at 11:00 +0100, Alexander Kanavin wrote:
> On Thu, 8 Dec 2022 at 08:11, Ola x Nilsson <olani@axis.com> wrote:
> > +# TODO: Only for 32-bit architectures?
> > +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> > +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> > +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> 
> On the contrary, why not set this globally?

The 64 bit architectures don't need this and should "just work" so it
is probably better just to set this where needed. At the very least
that will keep the compiler command lines a bit cleaner.

Cheers,

Richard
Khem Raj Dec. 9, 2022, 12:23 a.m. UTC | #3
Thanks for the patches

On 12/7/22 23:11, Ola x Nilsson wrote:
> Signed-off-by: Ola x Nilsson <olani@axis.com>
> ---
>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>   create mode 100644 meta/conf/distro/time64.conf
> 
> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> new file mode 100644
> index 0000000000..99eb06dc0f
> --- /dev/null
> +++ b/meta/conf/distro/time64.conf
> @@ -0,0 +1,23 @@
> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> +
> +# TODO: Only for 32-bit architectures?
> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"

We should enable it across all 32bit systems
something like

TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if 
d.getVar('SITEINFO_BITS') == '32' else ''}"

would do it.

Also adding -D_LARGEFILE_SOURCE will ensure needed functions that LFS 
needs is enabled as well, usually its automatically defined when 
-D_GNU_SOURCE is set but that may not be case all the time.

> +
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> +# Needs further investigation
> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> +
> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so] = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64 setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime localtime_r"
> +
> +# libpulsedsp.so is a preload-library that hooks libc functions
> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
> +
> +
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174389): https://lists.openembedded.org/g/openembedded-core/message/174389
> Mute This Topic: https://lists.openembedded.org/mt/95533494/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Ola x Nilsson Dec. 9, 2022, 9:55 a.m. UTC | #4
On Fri, Dec 09 2022, Khem Raj wrote:

> Thanks for the patches
>
> On 12/7/22 23:11, Ola x Nilsson wrote:
>> Signed-off-by: Ola x Nilsson <olani@axis.com>
>> ---
>>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>   create mode 100644 meta/conf/distro/time64.conf
>> 
>> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
>> new file mode 100644
>> index 0000000000..99eb06dc0f
>> --- /dev/null
>> +++ b/meta/conf/distro/time64.conf
>> @@ -0,0 +1,23 @@
>> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
>> +
>> +# TODO: Only for 32-bit architectures?
>> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
>> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
>> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
>
> We should enable it across all 32bit systems
> something like
>
> TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if 
> d.getVar('SITEINFO_BITS') == '32' else ''}"
>
> would do it.

I was looking for a way to identify 32-bit platforms but did not find
SITEINFO_BITS.  I'll look into using that instead.

> Also adding -D_LARGEFILE_SOURCE will ensure needed functions that LFS 
> needs is enabled as well, usually its automatically defined when 
> -D_GNU_SOURCE is set but that may not be case all the time.

As far as I can tell, _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE are
independent of _TIME_BITS=64 and _FILE_OFFSET_BITS=64 (while
_TIME_BITS=64 requiress _FILE_OFFSET_BITS=64).

I thought the _LARGEFILE* options were controlled by the largefile
DISTRO_FEATURE but there seems to be some confusion about that.

It looks like you (Khem) made largefile default instead of using the
distro feature in all of the OE-core recipes, and the largefile distro
feature is no longer mentioned in docs.  It is mentioned as removed in
the transition notes.  But it is still in poky-tiny.conf and
POKY_DEFAULT_DISTRO_FEATURES.  Probably an oversight?

I do not see the _LARGEFILE*_SOURCE options defined globally anywhere.

Am I missing something?

>> +
>> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
>> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
>> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
>> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
>> +# Needs further investigation
>> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
>> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
>> +
>> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so]
>> = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl
>> lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
>> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
>> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64
>> setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm
>> fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime
>> localtime_r"
>> +
>> +# libpulsedsp.so is a preload-library that hooks libc functions
>> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
>> +
>> +
>>
Ola x Nilsson Dec. 9, 2022, 10:31 a.m. UTC | #5
On Thu, Dec 08 2022, Richard Purdie wrote:

> On Thu, 2022-12-08 at 11:00 +0100, Alexander Kanavin wrote:
>> On Thu, 8 Dec 2022 at 08:11, Ola x Nilsson <olani@axis.com> wrote:
>> > +# TODO: Only for 32-bit architectures?
>> > +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
>> > +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
>> > +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
>> 
>> On the contrary, why not set this globally?
>
> The 64 bit architectures don't need this and should "just work" so it
> is probably better just to set this where needed. At the very least
> that will keep the compiler command lines a bit cleaner.

It also changes the hashes for no good reason, right?
Khem suggested using SITEINFO_BITS, I'll see if that works.
Ola x Nilsson Dec. 9, 2022, 10:36 a.m. UTC | #6
On Thu, Dec 08 2022, Alexander Kanavin wrote:

> On Thu, 8 Dec 2022 at 08:11, Ola x Nilsson <olani@axis.com> wrote:
>> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
>> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
>> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
>> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
>> +# Needs further investigation
>> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
>> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
>> +
>> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so]
>> = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl
>> lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
>> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
>> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64
>> setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm
>> fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime
>> localtime_r"
>> +
>> +# libpulsedsp.so is a preload-library that hooks libc functions
>> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
>
> Some of these perhaps need a ticket in bugzilla, and there should be a
> meta-ticket for Y2038 that tracks individual issues (by depending on
> them). Can you arrange that please?

I can file tickets if that's the way we want to go.  I'm not really at
that point yet, these are just for the packages that were included in
our products.  I've just done one plain poky build to make sure this
stuff worked so far.
Richard Purdie Dec. 9, 2022, 12:35 p.m. UTC | #7
On Fri, 2022-12-09 at 10:55 +0100, Ola x Nilsson wrote:
> On Fri, Dec 09 2022, Khem Raj wrote:
> 
> > Thanks for the patches
> > 
> > On 12/7/22 23:11, Ola x Nilsson wrote:
> > > Signed-off-by: Ola x Nilsson <olani@axis.com>
> > > ---
> > >   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> > >   1 file changed, 23 insertions(+)
> > >   create mode 100644 meta/conf/distro/time64.conf
> > > 
> > > diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> > > new file mode 100644
> > > index 0000000000..99eb06dc0f
> > > --- /dev/null
> > > +++ b/meta/conf/distro/time64.conf
> > > @@ -0,0 +1,23 @@
> > > +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> > > +
> > > +# TODO: Only for 32-bit architectures?
> > > +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> > > +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> > > +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> > 
> > We should enable it across all 32bit systems
> > something like
> > 
> > TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if 
> > d.getVar('SITEINFO_BITS') == '32' else ''}"
> > 
> > would do it.
> 
> I was looking for a way to identify 32-bit platforms but did not find
> SITEINFO_BITS.  I'll look into using that instead.
> 
> > Also adding -D_LARGEFILE_SOURCE will ensure needed functions that LFS 
> > needs is enabled as well, usually its automatically defined when 
> > -D_GNU_SOURCE is set but that may not be case all the time.
> 
> As far as I can tell, _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE are
> independent of _TIME_BITS=64 and _FILE_OFFSET_BITS=64 (while
> _TIME_BITS=64 requiress _FILE_OFFSET_BITS=64).
> 
> I thought the _LARGEFILE* options were controlled by the largefile
> DISTRO_FEATURE but there seems to be some confusion about that.
> 
> It looks like you (Khem) made largefile default instead of using the
> distro feature in all of the OE-core recipes, and the largefile distro
> feature is no longer mentioned in docs.  It is mentioned as removed in
> the transition notes.  But it is still in poky-tiny.conf and
> POKY_DEFAULT_DISTRO_FEATURES.  Probably an oversight?

Yes. I just sent a patch to remove that.

> I do not see the _LARGEFILE*_SOURCE options defined globally anywhere.
> 
> Am I missing something?

We used to control configure largefile options with the distro feature,
now we just turn it on anywhere we need to unconditionally. I suspect
we rely on software configuring by default to it in some cases. It
would be good to check if it isn't being enabled anywhere and fix those
cases somehow. Not sure if we need global options or not.

Cheers,

Richard
Alexandre Belloni Dec. 9, 2022, 1:06 p.m. UTC | #8
On 09/12/2022 11:36:03+0100, Ola x Nilsson wrote:
> 
> On Thu, Dec 08 2022, Alexander Kanavin wrote:
> 
> > On Thu, 8 Dec 2022 at 08:11, Ola x Nilsson <olani@axis.com> wrote:
> >> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> >> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> >> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> >> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> >> +# Needs further investigation
> >> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> >> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> >> +
> >> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so]
> >> = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl
> >> lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> >> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> >> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64
> >> setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm
> >> fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime
> >> localtime_r"
> >> +
> >> +# libpulsedsp.so is a preload-library that hooks libc functions
> >> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
> >
> > Some of these perhaps need a ticket in bugzilla, and there should be a
> > meta-ticket for Y2038 that tracks individual issues (by depending on
> > them). Can you arrange that please?
> 
> I can file tickets if that's the way we want to go.  I'm not really at
> that point yet, these are just for the packages that were included in
> our products.  I've just done one plain poky build to make sure this
> stuff worked so far.

I know there is pulseaudio that will fail too. I'll do a build including
time64.conf over the week end and I can file bugs where necessary.

>
Alexandre Belloni Dec. 9, 2022, 1:09 p.m. UTC | #9
On 09/12/2022 10:55:16+0100, Ola x Nilsson wrote:
> 
> On Fri, Dec 09 2022, Khem Raj wrote:
> 
> > Thanks for the patches
> >
> > On 12/7/22 23:11, Ola x Nilsson wrote:
> >> Signed-off-by: Ola x Nilsson <olani@axis.com>
> >> ---
> >>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> >>   1 file changed, 23 insertions(+)
> >>   create mode 100644 meta/conf/distro/time64.conf
> >> 
> >> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> >> new file mode 100644
> >> index 0000000000..99eb06dc0f
> >> --- /dev/null
> >> +++ b/meta/conf/distro/time64.conf
> >> @@ -0,0 +1,23 @@
> >> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> >> +
> >> +# TODO: Only for 32-bit architectures?
> >> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> >> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> >> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> >
> > We should enable it across all 32bit systems
> > something like
> >
> > TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if 
> > d.getVar('SITEINFO_BITS') == '32' else ''}"
> >
> > would do it.
> 
> I was looking for a way to identify 32-bit platforms but did not find
> SITEINFO_BITS.  I'll look into using that instead.
> 

I don't think it is correct to assume all the 32bit platforms are
affected. riscv32 is not affected for example nor would be any newer
32bit architecture.
Alexander Kanavin Dec. 9, 2022, 1:13 p.m. UTC | #10
I'd rather not maintain a list of those that *are* affected, even if
we then include those that aren't. It's only build flags, and they
should be ignored when the platform has already done the right things
from the start.

Alex

On Fri, 9 Dec 2022 at 14:09, Alexandre Belloni via
lists.openembedded.org
<alexandre.belloni=bootlin.com@lists.openembedded.org> wrote:
>
> On 09/12/2022 10:55:16+0100, Ola x Nilsson wrote:
> >
> > On Fri, Dec 09 2022, Khem Raj wrote:
> >
> > > Thanks for the patches
> > >
> > > On 12/7/22 23:11, Ola x Nilsson wrote:
> > >> Signed-off-by: Ola x Nilsson <olani@axis.com>
> > >> ---
> > >>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> > >>   1 file changed, 23 insertions(+)
> > >>   create mode 100644 meta/conf/distro/time64.conf
> > >>
> > >> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> > >> new file mode 100644
> > >> index 0000000000..99eb06dc0f
> > >> --- /dev/null
> > >> +++ b/meta/conf/distro/time64.conf
> > >> @@ -0,0 +1,23 @@
> > >> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> > >> +
> > >> +# TODO: Only for 32-bit architectures?
> > >> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >
> > > We should enable it across all 32bit systems
> > > something like
> > >
> > > TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if
> > > d.getVar('SITEINFO_BITS') == '32' else ''}"
> > >
> > > would do it.
> >
> > I was looking for a way to identify 32-bit platforms but did not find
> > SITEINFO_BITS.  I'll look into using that instead.
> >
>
> I don't think it is correct to assume all the 32bit platforms are
> affected. riscv32 is not affected for example nor would be any newer
> 32bit architecture.
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174450): https://lists.openembedded.org/g/openembedded-core/message/174450
> Mute This Topic: https://lists.openembedded.org/mt/95533494/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie Dec. 9, 2022, 1:29 p.m. UTC | #11
On Fri, 2022-12-09 at 14:13 +0100, Alexander Kanavin wrote:
> I'd rather not maintain a list of those that *are* affected,

This isn't actually too bad since these are architectures, not machines
or tunes and the list should remain static, nobody is creating new 32
bit platforms with the bug. So yes, it is a pain to write but once
written, it shouldn't change much.

>  even if
> we then include those that aren't. It's only build flags, and they
> should be ignored when the platform has already done the right things
> from the start.

They do clutter up the compile logs and I'd prefer not to have them
where they aren't needed, extra noise causes different problems.

Cheers,

Richard
Khem Raj Dec. 9, 2022, 4:36 p.m. UTC | #12
On Fri, Dec 9, 2022 at 5:09 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 09/12/2022 10:55:16+0100, Ola x Nilsson wrote:
> >
> > On Fri, Dec 09 2022, Khem Raj wrote:
> >
> > > Thanks for the patches
> > >
> > > On 12/7/22 23:11, Ola x Nilsson wrote:
> > >> Signed-off-by: Ola x Nilsson <olani@axis.com>
> > >> ---
> > >>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> > >>   1 file changed, 23 insertions(+)
> > >>   create mode 100644 meta/conf/distro/time64.conf
> > >>
> > >> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> > >> new file mode 100644
> > >> index 0000000000..99eb06dc0f
> > >> --- /dev/null
> > >> +++ b/meta/conf/distro/time64.conf
> > >> @@ -0,0 +1,23 @@
> > >> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> > >> +
> > >> +# TODO: Only for 32-bit architectures?
> > >> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> > >
> > > We should enable it across all 32bit systems
> > > something like
> > >
> > > TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if
> > > d.getVar('SITEINFO_BITS') == '32' else ''}"
> > >
> > > would do it.
> >
> > I was looking for a way to identify 32-bit platforms but did not find
> > SITEINFO_BITS.  I'll look into using that instead.
> >
>
> I don't think it is correct to assume all the 32bit platforms are
> affected. riscv32 is not affected for example nor would be any newer
> 32bit architecture.

yeah that's a good point, I overlooked that because these defines will
become redundant in those cases

>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Khem Raj Dec. 9, 2022, 4:47 p.m. UTC | #13
On Fri, Dec 9, 2022 at 2:31 AM Ola x Nilsson <ola.x.nilsson@axis.com> wrote:
>
>
> On Fri, Dec 09 2022, Khem Raj wrote:
>
> > Thanks for the patches
> >
> > On 12/7/22 23:11, Ola x Nilsson wrote:
> >> Signed-off-by: Ola x Nilsson <olani@axis.com>
> >> ---
> >>   meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> >>   1 file changed, 23 insertions(+)
> >>   create mode 100644 meta/conf/distro/time64.conf
> >>
> >> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> >> new file mode 100644
> >> index 0000000000..99eb06dc0f
> >> --- /dev/null
> >> +++ b/meta/conf/distro/time64.conf
> >> @@ -0,0 +1,23 @@
> >> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> >> +
> >> +# TODO: Only for 32-bit architectures?
> >> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> >> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> >> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> >
> > We should enable it across all 32bit systems
> > something like
> >
> > TARGET_CC_ARCH += "${@'-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' if
> > d.getVar('SITEINFO_BITS') == '32' else ''}"
> >
> > would do it.
>
> I was looking for a way to identify 32-bit platforms but did not find
> SITEINFO_BITS.  I'll look into using that instead.
>
> > Also adding -D_LARGEFILE_SOURCE will ensure needed functions that LFS
> > needs is enabled as well, usually its automatically defined when
> > -D_GNU_SOURCE is set but that may not be case all the time.
>
> As far as I can tell, _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE are
> independent of _TIME_BITS=64 and _FILE_OFFSET_BITS=64 (while
> _TIME_BITS=64 requiress _FILE_OFFSET_BITS=64).
>
> I thought the _LARGEFILE* options were controlled by the largefile
> DISTRO_FEATURE but there seems to be some confusion about that.
>

this was transitional it was removed because we wanted to enable it
globally by default
however, we might want to add --enable-largefile unconditionally to
autotools.bbclass
for some autotooled packages which still need it for others we have to define
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64, in this case 64bit off_t
is common need between both LFS and 64bit time_t

Also see this discussion on same topic
https://sourceware.org/pipermail/libc-alpha/2022-November/143386.html

> It looks like you (Khem) made largefile default instead of using the
> distro feature in all of the OE-core recipes, and the largefile distro
> feature is no longer mentioned in docs.  It is mentioned as removed in
> the transition notes.  But it is still in poky-tiny.conf and
> POKY_DEFAULT_DISTRO_FEATURES.  Probably an oversight?

yes seems so, it should be removed from there.
>
> I do not see the _LARGEFILE*_SOURCE options defined globally anywhere.
>
> Am I missing something?

most of packages are enabling it indirectly via enabling _GNU_SOURCE macro
on glibc systems. So we are fine but it will be better to be explicit about it.

>
> >> +
> >> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> >> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> >> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> >> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> >> +# Needs further investigation
> >> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> >> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> >> +
> >> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so]
> >> = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl
> >> lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> >> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> >> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64
> >> setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm
> >> fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime
> >> localtime_r"
> >> +
> >> +# libpulsedsp.so is a preload-library that hooks libc functions
> >> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
> >> +
> >> +
> >>
>
> --
> Ola x Nilsson
Khem Raj Dec. 15, 2022, 4:32 p.m. UTC | #14
On Wed, Dec 7, 2022 at 11:11 PM Ola x Nilsson <olani@axis.com> wrote:
>
> Signed-off-by: Ola x Nilsson <olani@axis.com>
> ---
>  meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 meta/conf/distro/time64.conf
>
> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> new file mode 100644
> index 0000000000..99eb06dc0f
> --- /dev/null
> +++ b/meta/conf/distro/time64.conf

I think this file should be called time64.inc to make it similar to
other distro fragments that distros can use.

> @@ -0,0 +1,23 @@
> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"

Also add -D_LARGEFILE_SOURCE here too so we have all dependent changes
in one place

> +
> +# TODO: Only for 32-bit architectures?
> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"

I think it should be made generic across all 32bit arches as I
mentioned last time.

> +
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> +# Needs further investigation
> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> +
> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so] = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64 setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime localtime_r"
> +
> +# libpulsedsp.so is a preload-library that hooks libc functions
> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
> +

Earlier I thought we won't need it on musl. Ideally that's true, but
many applications do rely on _FILE_OFFSET_BITS=64 so I think
we will need to include it on musl too even though these defines wont
means much from libc point of view.

> +
> --
> 2.30.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174389): https://lists.openembedded.org/g/openembedded-core/message/174389
> Mute This Topic: https://lists.openembedded.org/mt/95533494/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Khem Raj Dec. 16, 2022, 6:44 p.m. UTC | #15
I ran a build with time64 enabled on meta-openembedded + few more
layers enabled on qemuarm/clang using yoe distro settings.
there are some failures as below.

https://errors.yoctoproject.org/Errors/Build/156858/

On Wed, Dec 7, 2022 at 11:11 PM Ola x Nilsson <olani@axis.com> wrote:
>
> Signed-off-by: Ola x Nilsson <olani@axis.com>
> ---
>  meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 meta/conf/distro/time64.conf
>
> diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> new file mode 100644
> index 0000000000..99eb06dc0f
> --- /dev/null
> +++ b/meta/conf/distro/time64.conf
> @@ -0,0 +1,23 @@
> +GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
> +
> +# TODO: Only for 32-bit architectures?
> +TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
> +TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
> +
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
> +# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
> +# both 32 and 64 bit file APIs.  But it does not handle the time side?
> +# Needs further investigation
> +GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
> +GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
> +
> +INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so] = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
> +INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
> +INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64 setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime localtime_r"
> +
> +# libpulsedsp.so is a preload-library that hooks libc functions
> +INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
> +
> +
> --
> 2.30.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174389): https://lists.openembedded.org/g/openembedded-core/message/174389
> Mute This Topic: https://lists.openembedded.org/mt/95533494/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie Dec. 17, 2022, 11:15 a.m. UTC | #16
On Thu, 2022-12-15 at 08:32 -0800, Khem Raj wrote:
> On Wed, Dec 7, 2022 at 11:11 PM Ola x Nilsson <olani@axis.com> wrote:
> > 
> > Signed-off-by: Ola x Nilsson <olani@axis.com>
> > ---
> >  meta/conf/distro/time64.conf | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >  create mode 100644 meta/conf/distro/time64.conf
> > 
> > diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
> > new file mode 100644
> > index 0000000000..99eb06dc0f
> > --- /dev/null
> > +++ b/meta/conf/distro/time64.conf
> 
> I think this file should be called time64.inc to make it similar to
> other distro fragments that distros can use.

Agreed, it also needs to be in the include directory as it isn't a
distro called "time64". I've merged this patch but I moved the file to
distro/include/time64.inc. We can improve it from there with further
patches.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/conf/distro/time64.conf b/meta/conf/distro/time64.conf
new file mode 100644
index 0000000000..99eb06dc0f
--- /dev/null
+++ b/meta/conf/distro/time64.conf
@@ -0,0 +1,23 @@ 
+GLIBC_64BIT_TIME_FLAGS = "-D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64"
+
+# TODO: Only for 32-bit architectures?
+TARGET_CC_ARCH:append:arm = " ${GLIBC_64BIT_TIME_FLAGS}"
+TARGET_CC_ARCH:append:armeb = " ${GLIBC_64BIT_TIME_FLAGS}"
+TARGET_CC_ARCH:append:mips32el = " ${GLIBC_64BIT_TIME_FLAGS}"
+
+GLIBC_64BIT_TIME_FLAGS:pn-glibc = ""
+GLIBC_64BIT_TIME_FLAGS:pn-glibc-tests = ""
+# pipewire-v4l2 explicitly sets _FILE_OFFSET_BITS=32 to get access to
+# both 32 and 64 bit file APIs.  But it does not handle the time side?
+# Needs further investigation
+GLIBC_64BIT_TIME_FLAGS:pn-pipewire = ""
+GLIBC_64BIT_TIME_FLAGS:pn-gcc-sanitizers = ""
+
+INSANE_SKIP:libstd-rs[_usr_lib_rustlib_armv7-poky-linux-gnueabihf_lib_libstd.so] = "clock_gettime gettime fcntl fstat64 fstatat64 getsockopt ioctl lstat64 nanosleep prctl recvmsg sendmsg setsockopt stat64"
+INSANE_SKIP:librsvg[_usr_bin_rsvg-convert] = "fcntl fstat64 prctl stat64 clock_gettime"
+INSANE_SKIP:librsvg[_usr_lib_librsvg-2.so.2.48.0] = "fcntl lstat64 setsockopt sendmsg fstat64 getsockopt ioctl nanosleep timegm fstatat64 prctl mktime gmtime_r recvmsg stat64 clock_gettime localtime_r"
+
+# libpulsedsp.so is a preload-library that hooks libc functions
+INSANE_SKIP:pulseaudio[_usr_lib_pulseaudio_libpulsedsp.so] = "setsockopt fcntl"
+
+