| Message ID | 20200907202410.12228-1-heiko.thiery@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] package/strace: disable mpers support | expand |
Heiko, Baruch, All, On 2020-09-07 22:24 +0200, Heiko Thiery spake thusly: > On aarch64 With the config option "--enable-mpers=check" the configure.ac The name of that option is misleading. At first, I thought it was about personalities: https://man7.org/linux/man-pages/man2/personality.2.html ... and so I was wondering why another compiler would come into the mix... > script searchs for a 32bit compiler. When a matching compiler is found > in the PATH some compatiblity checks are done. This can fail when the > available kernel headers on host and buildroot target does not match. > > Since buildroot does not support 32bit binaries when building for 64bit > architecture (no -m32 option) we can disable this option unconditionally. ... but indeed, mpers is not about personalities at all; it is about multi-bitness systems. Buildroot does not support building a userland for multi-bitness systems. However, some systems may very well run a 64-bit kernel with a 32-bit userland. In that case, strace should be able to interpret the syscalls made by the process and the replies sent by the kernel, and those are done in the bitness of the process, not of the kernel. So, we always want an strace that is of the same bitness as the userland. And so I indeed think we do not need this ill-named multi-personality support of strace. Note that some people may however run such multi-bitness systems. But in that case, they had to build two userlands, a 32-bit one and a 64-bit one, and somehow aggregate the two after the fact. In that case, they would bear the responsibility to carry an strace for each bitness. (Note: I've left the x32 ABI on the side on purpose: it is not a real thing anymore, if it ever were...) Regards, Yann E. MORIN. > When disabling unconditionally also the configuration for toolchain using > MUSL can be removed. > > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Brandon Maier <brandon.maier@rockwellcollins.com> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > package/strace/strace.mk | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/package/strace/strace.mk b/package/strace/strace.mk > index 8403e09cbd..50e6d90a87 100644 > --- a/package/strace/strace.mk > +++ b/package/strace/strace.mk > @@ -9,15 +9,7 @@ STRACE_SOURCE = strace-$(STRACE_VERSION).tar.xz > STRACE_SITE = https://strace.io/files/$(STRACE_VERSION) > STRACE_LICENSE = LGPL-2.1+ > STRACE_LICENSE_FILES = COPYING LGPL-2.1-or-later > -STRACE_CONF_OPTS = --enable-mpers=check > - > -# strace bundle some kernel headers to build libmpers, this mixes userspace > -# headers and kernel headers which break the build with musl. > -# The stddef.h from gcc is used instead of the one from musl. > -ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) > -STRACE_CONF_OPTS += st_cv_m32_mpers=no \ > - st_cv_mx32_mpers=no > -endif > +STRACE_CONF_OPTS = --enable-mpers=no > > ifeq ($(BR2_PACKAGE_LIBUNWIND),y) > STRACE_DEPENDENCIES += libunwind > -- > 2.20.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
On Tue, 8 Sep 2020 00:56:55 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > script searchs for a 32bit compiler. When a matching compiler is found > > in the PATH some compatiblity checks are done. This can fail when the > > available kernel headers on host and buildroot target does not match. > > > > Since buildroot does not support 32bit binaries when building for 64bit > > architecture (no -m32 option) we can disable this option unconditionally. > > ... but indeed, mpers is not about personalities at all; it is about > multi-bitness systems. Buildroot does not support building a userland > for multi-bitness systems. Absolutely. > However, some systems may very well run a 64-bit kernel with a 32-bit > userland. In that case, strace should be able to interpret the syscalls > made by the process and the replies sent by the kernel, and those are > done in the bitness of the process, not of the kernel. > > So, we always want an strace that is of the same bitness as the > userland. And so I indeed think we do not need this ill-named > multi-personality support of strace. > > Note that some people may however run such multi-bitness systems. But in > that case, they had to build two userlands, a 32-bit one and a 64-bit > one, and somehow aggregate the two after the fact. In that case, they > would bear the responsibility to carry an strace for each bitness. So, is this an Acked-by for this patch ? :-) Thomas
Thomas, All, On 2020-09-08 08:54 +0200, Thomas Petazzoni spake thusly: > On Tue, 8 Sep 2020 00:56:55 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: [--SNIP--] > > However, some systems may very well run a 64-bit kernel with a 32-bit > > userland. In that case, strace should be able to interpret the syscalls > > made by the process and the replies sent by the kernel, and those are > > done in the bitness of the process, not of the kernel. > > > > So, we always want an strace that is of the same bitness as the > > userland. And so I indeed think we do not need this ill-named > > multi-personality support of strace. > > > > Note that some people may however run such multi-bitness systems. But in > > that case, they had to build two userlands, a 32-bit one and a 64-bit > > one, and somehow aggregate the two after the fact. In that case, they > > would bear the responsibility to carry an strace for each bitness. > > So, is this an Acked-by for this patch ? :-) It would, but I'd still like to have some feedback from Baruch, who had initial added of the mpers option, so may have further explanations to provide. Regards, Yann E. MORIN.
Hi Yann On Tue, Sep 08 2020, Yann E. MORIN wrote: > On 2020-09-08 08:54 +0200, Thomas Petazzoni spake thusly: >> On Tue, 8 Sep 2020 00:56:55 +0200 >> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > [--SNIP--] >> > However, some systems may very well run a 64-bit kernel with a 32-bit >> > userland. In that case, strace should be able to interpret the syscalls >> > made by the process and the replies sent by the kernel, and those are >> > done in the bitness of the process, not of the kernel. >> > >> > So, we always want an strace that is of the same bitness as the >> > userland. And so I indeed think we do not need this ill-named >> > multi-personality support of strace. >> > >> > Note that some people may however run such multi-bitness systems. But in >> > that case, they had to build two userlands, a 32-bit one and a 64-bit >> > one, and somehow aggregate the two after the fact. In that case, they >> > would bear the responsibility to carry an strace for each bitness. >> >> So, is this an Acked-by for this patch ? :-) > > It would, but I'd still like to have some feedback from Baruch, who had > initial added of the mpers option, so may have further explanations to > provide. I only touched mpers (commit fc0d31caeedba) to fix build failure I (re)introduced in a previous commit (10883729419). strace enables mpers by default. I am fine with removing mpers support entirely. baruch
Heiko, All, On 2020-09-07 22:24 +0200, Heiko Thiery spake thusly: > On aarch64 With the config option "--enable-mpers=check" the configure.ac > script searchs for a 32bit compiler. When a matching compiler is found > in the PATH some compatiblity checks are done. This can fail when the > available kernel headers on host and buildroot target does not match. > > Since buildroot does not support 32bit binaries when building for 64bit > architecture (no -m32 option) we can disable this option unconditionally. > > When disabling unconditionally also the configuration for toolchain using > MUSL can be removed. > > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Brandon Maier <brandon.maier@rockwellcollins.com> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> Applied to master, thanks. Regards, Yann E. MORIN. > --- > package/strace/strace.mk | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/package/strace/strace.mk b/package/strace/strace.mk > index 8403e09cbd..50e6d90a87 100644 > --- a/package/strace/strace.mk > +++ b/package/strace/strace.mk > @@ -9,15 +9,7 @@ STRACE_SOURCE = strace-$(STRACE_VERSION).tar.xz > STRACE_SITE = https://strace.io/files/$(STRACE_VERSION) > STRACE_LICENSE = LGPL-2.1+ > STRACE_LICENSE_FILES = COPYING LGPL-2.1-or-later > -STRACE_CONF_OPTS = --enable-mpers=check > - > -# strace bundle some kernel headers to build libmpers, this mixes userspace > -# headers and kernel headers which break the build with musl. > -# The stddef.h from gcc is used instead of the one from musl. > -ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) > -STRACE_CONF_OPTS += st_cv_m32_mpers=no \ > - st_cv_mx32_mpers=no > -endif > +STRACE_CONF_OPTS = --enable-mpers=no > > ifeq ($(BR2_PACKAGE_LIBUNWIND),y) > STRACE_DEPENDENCIES += libunwind > -- > 2.20.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
>>>>> "Heiko" == Heiko Thiery <heiko.thiery@gmail.com> writes: > On aarch64 With the config option "--enable-mpers=check" the configure.ac > script searchs for a 32bit compiler. When a matching compiler is found > in the PATH some compatiblity checks are done. This can fail when the > available kernel headers on host and buildroot target does not match. > Since buildroot does not support 32bit binaries when building for 64bit > architecture (no -m32 option) we can disable this option unconditionally. > When disabling unconditionally also the configuration for toolchain using > MUSL can be removed. > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Brandon Maier <brandon.maier@rockwellcollins.com> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> Committed to 2020.02.x, 2020.05.x and 2020.08.x, thanks.
diff --git a/package/strace/strace.mk b/package/strace/strace.mk index 8403e09cbd..50e6d90a87 100644 --- a/package/strace/strace.mk +++ b/package/strace/strace.mk @@ -9,15 +9,7 @@ STRACE_SOURCE = strace-$(STRACE_VERSION).tar.xz STRACE_SITE = https://strace.io/files/$(STRACE_VERSION) STRACE_LICENSE = LGPL-2.1+ STRACE_LICENSE_FILES = COPYING LGPL-2.1-or-later -STRACE_CONF_OPTS = --enable-mpers=check - -# strace bundle some kernel headers to build libmpers, this mixes userspace -# headers and kernel headers which break the build with musl. -# The stddef.h from gcc is used instead of the one from musl. -ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) -STRACE_CONF_OPTS += st_cv_m32_mpers=no \ - st_cv_mx32_mpers=no -endif +STRACE_CONF_OPTS = --enable-mpers=no ifeq ($(BR2_PACKAGE_LIBUNWIND),y) STRACE_DEPENDENCIES += libunwind
On aarch64 With the config option "--enable-mpers=check" the configure.ac script searchs for a 32bit compiler. When a matching compiler is found in the PATH some compatiblity checks are done. This can fail when the available kernel headers on host and buildroot target does not match. Since buildroot does not support 32bit binaries when building for 64bit architecture (no -m32 option) we can disable this option unconditionally. When disabling unconditionally also the configuration for toolchain using MUSL can be removed. Cc: Baruch Siach <baruch@tkos.co.il> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Brandon Maier <brandon.maier@rockwellcollins.com> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> --- package/strace/strace.mk | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)