On Wed, Aug 18, 2021 at 11:38 PM Stijn Tintel stijn@linux-ipv6.be wrote:
On 18/08/2021 22:29, Maamoun TK wrote:
On Tue, Aug 17, 2021 at 9:40 AM Niels Möller <nisse@lysator.liu.se mailto:nisse@lysator.liu.se> wrote:
The configuration where it didn't work was powerpc64-openwrt-linux-musl. I'd like Nettle to work on embedded systems whenever practical. But support depends on assistance from users of those systems. As I understood it, this system needs to use the v2 ABI. I would hope it's easy to detect the abi used by the configured C compiler, and then select the same prologue sequence as is currently used for little-endian. I.e., one more configure test, and changing the "ifelse(WORDS_BIGENDIAN,no," condition in powerpc64/machine.m4 to check a different configure variable.I skipped processing the assembly files with a different approach, I made the configuration check for musl and endianness variant to trigger assembly processing. You can check the fix in this branch https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix Apparently, the bug reporter uses a cross-compiler for powerpc arch. It would be great to run this fix at the same bug environment since I tested the patch in different circumstances.
Your patch has no effect in my environment (OpenWrt build system), as host_os is linux-gnu, according to config.log, so it doesn't match *musl. See [1] for config.log and [2] for full compile log.
The output of powerpc64-openwrt-linux-musl-gcc -E -dM - </dev/null | sort, which was requested earlier in this thread can be seen at [3].
Thanks, Stijn
[1] https://gist.github.com/9e0ecb025033dda1d0d58094da84c308 [2] https://gist.github.com/stintel/0e7046df511cf4d1ca20edb56df50b1b [3] https://gist.github.com/stintel/b3651a7db87edea9e8bd0aef242bcdae
config.guess detects the C standard library based on a result from the compiler defined in "CC_FOR_BUILD" variable, for some reason OpenWrt build system failed to set that variable properly, from your config.log I can see CC_FOR_BUILD='gcc -O -g' but when I use bare musl tools I get CC_FOR_BUILD='musl-gcc'
There is nothing specific in the output of powerpc64-openwrt-linux-musl-gcc -E -dM log as I can see. In musl libc FAQ, they stated that there is no __MUSL__ in the preprocessor macros https://wiki.musl-libc.org/faq.html
regards, Mamone
On Thu, Aug 19, 2021 at 12:40 AM Maamoun TK maamoun.tk@googlemail.com wrote:
On Wed, Aug 18, 2021 at 11:38 PM Stijn Tintel stijn@linux-ipv6.be wrote:
On 18/08/2021 22:29, Maamoun TK wrote:
On Tue, Aug 17, 2021 at 9:40 AM Niels Möller <nisse@lysator.liu.se mailto:nisse@lysator.liu.se> wrote:
The configuration where it didn't work was powerpc64-openwrt-linux-musl. I'd like Nettle to work on embedded systems whenever practical. But support depends on assistance from users of those systems. As I understood it, this system needs to use the v2 ABI. I wouldhope
it's easy to detect the abi used by the configured C compiler, and then select the same prologue sequence as is currently used for little-endian. I.e., one more configure test, and changing the "ifelse(WORDS_BIGENDIAN,no," condition in powerpc64/machine.m4 to check a different configure variable.I skipped processing the assembly files with a different approach, I made the configuration check for musl and endianness variant to trigger assembly processing. You can check the fix in this branch https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix Apparently, the bug reporter uses a cross-compiler for powerpc arch. It would be great to run this fix at the same bug environment since I tested the patch in different circumstances.
Your patch has no effect in my environment (OpenWrt build system), as host_os is linux-gnu, according to config.log, so it doesn't match *musl. See [1] for config.log and [2] for full compile log.
The output of powerpc64-openwrt-linux-musl-gcc -E -dM - </dev/null | sort, which was requested earlier in this thread can be seen at [3].
Thanks, Stijn
[1] https://gist.github.com/9e0ecb025033dda1d0d58094da84c308 [2] https://gist.github.com/stintel/0e7046df511cf4d1ca20edb56df50b1b [3] https://gist.github.com/stintel/b3651a7db87edea9e8bd0aef242bcdae
config.guess detects the C standard library based on a result from the compiler defined in "CC_FOR_BUILD" variable, for some reason OpenWrt build system failed to set that variable properly, from your config.log I can see CC_FOR_BUILD='gcc -O -g' but when I use bare musl tools I get CC_FOR_BUILD='musl-gcc'
The macro GMP_PROG_CC_FOR_BUILD in aclocal.m4 sets "CC_FOR_BUIL=gcc -O -g" rather than the actual compiler by checking if the library is configured with cross-compiling, I wonder if there is a workaround for that.
Maamoun TK maamoun.tk@googlemail.com writes:
config.guess detects the C standard library based on a result from the compiler defined in "CC_FOR_BUILD" variable, for some reason OpenWrt build system failed to set that variable properly, from your config.log I can see CC_FOR_BUILD='gcc -O -g' but when I use bare musl tools I get CC_FOR_BUILD='musl-gcc'
In Nettle's Makefiles, CC_FOR_BUILD is intended to be a compiler targetting the *build* system, used to compile things like eccdata.c that are run on the build system as part of the build. It's intended to be different from CC when cross compiling.
Not entirely sure how CC_FOR_BUILD is used in config.guess, but I think it is used to detect the system type of the build system.
There is nothing specific in the output of powerpc64-openwrt-linux-musl-gcc -E -dM log as I can see. In musl libc FAQ, they stated that there is no __MUSL__ in the preprocessor macros https://wiki.musl-libc.org/faq.html
The interesting thing I see is
#define _CALL_ELF 2
I hope this can be used to distinguish from other big-endian systems, that use ELFv1 abi?
Regards, /Niels
On Thu, Aug 19, 2021 at 9:04 AM Niels Möller nisse@lysator.liu.se wrote:
Maamoun TK maamoun.tk@googlemail.com writes:
config.guess detects the C standard library based on a result from the compiler defined in "CC_FOR_BUILD" variable, for some reason OpenWrt
build
system failed to set that variable properly, from your config.log I can
see
CC_FOR_BUILD='gcc -O -g' but when I use bare musl tools I get CC_FOR_BUILD='musl-gcc'
In Nettle's Makefiles, CC_FOR_BUILD is intended to be a compiler targetting the *build* system, used to compile things like eccdata.c that are run on the build system as part of the build. It's intended to be different from CC when cross compiling.
Not entirely sure how CC_FOR_BUILD is used in config.guess, but I think it is used to detect the system type of the build system.
There is nothing specific in the output of
powerpc64-openwrt-linux-musl-gcc
-E -dM log as I can see. In musl libc FAQ, they stated that there is no __MUSL__ in the preprocessor macros https://wiki.musl-libc.org/faq.html
The interesting thing I see is
#define _CALL_ELF 2
I hope this can be used to distinguish from other big-endian systems, that use ELFv1 abi?
That's right, in little-endian systems I got "#define _CALL_ELF 2" while in big-endian ones that value is 1 except when using musl. I've updated the patch in the branch https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix to exploit this distinction. Testing the fix again by the bug reporter will be appreciated since I don't have OpenWrt build system in my power system.
regards, Mamone
Maamoun TK maamoun.tk@googlemail.com writes:
That's right, in little-endian systems I got "#define _CALL_ELF 2" while in big-endian ones that value is 1 except when using musl.
That's good.
I've updated the patch in the branch https://git.lysator.liu.se/mamonet/nettle/-/tree/ppc64_musl_fix to exploit this distinction.
I've tried a different approach on branch https://git.lysator.liu.se/nettle/nettle/-/tree/ppc64-efv2-check. Patch below. (It makes sense to me to have the new check together with the ABI check, but on second thought, probably a mistake to overload the ABI variable. It would be better to have a separate configure variable, more similar to the W64_ABI).
Unfortunaly, the CI cross builds aren't working at the moment (the buildenv images are based on Debian Buster ("stable" at the time images were built), and nettle's ci scripts do apt-get update and apt-get install, which now attempts to get Bullseye packages (new "stable" since a week ago)).
Regards, /Niels
diff --git a/config.m4.in b/config.m4.in index d89325b8..2ac19a84 100644 --- a/config.m4.in +++ b/config.m4.in @@ -5,6 +5,7 @@ define(`COFF_STYLE', `@ASM_COFF_STYLE@')dnl define(`TYPE_FUNCTION', `@ASM_TYPE_FUNCTION@')dnl define(`TYPE_PROGBITS', `@ASM_TYPE_PROGBITS@')dnl define(`ALIGN_LOG', `@ASM_ALIGN_LOG@')dnl +define(`ABI', `@ABI@')dnl define(`W64_ABI', `@W64_ABI@')dnl define(`RODATA', `@ASM_RODATA@')dnl define(`WORDS_BIGENDIAN', `@ASM_WORDS_BIGENDIAN@')dnl diff --git a/configure.ac b/configure.ac index ebec8759..0efa5795 100644 --- a/configure.ac +++ b/configure.ac @@ -353,8 +353,15 @@ case "$host_cpu" in ], [], [ ABI=32 ], [ - ABI=64 - ]) + AC_TRY_COMPILE([ +#if _CALL_ELF == 2 +#error ELFv2 ABI +#endif + ], [], [ + ABI=64v1 + ], [ + ABI=64v2 + ])]) ;; aarch64*) AC_TRY_COMPILE([ @@ -514,7 +521,7 @@ if test "x$enable_assembler" = xyes ; then fi ;; *powerpc64*) - if test "$ABI" = 64 ; then + if test "$ABI" != 32 ; then GMP_ASM_POWERPC_R_REGISTERS asm_path="powerpc64" if test "x$enable_fat" = xyes ; then @@ -1032,6 +1039,7 @@ AC_SUBST(ASM_TYPE_PROGBITS) AC_SUBST(ASM_MARK_NOEXEC_STACK) AC_SUBST(ASM_ALIGN_LOG) AC_SUBST(W64_ABI) +AC_SUBST(ABI) AC_SUBST(ASM_WORDS_BIGENDIAN) AC_SUBST(EMULATOR) AC_SUBST(ASM_X86_ENDBR) diff --git a/powerpc64/machine.m4 b/powerpc64/machine.m4 index 187a49b8..60c7465d 100644 --- a/powerpc64/machine.m4 +++ b/powerpc64/machine.m4 @@ -1,7 +1,7 @@ define(`PROLOGUE', `.globl C_NAME($1) DECLARE_FUNC(C_NAME($1)) -ifelse(WORDS_BIGENDIAN,no, +ifelse(ABI,64v2, `ifdef(`FUNC_ALIGN',`.align FUNC_ALIGN') C_NAME($1): addis 2,12,(.TOC.-C_NAME($1))@ha @@ -17,7 +17,7 @@ ifdef(`FUNC_ALIGN',`.align FUNC_ALIGN') undefine(`FUNC_ALIGN')')
define(`EPILOGUE', -`ifelse(WORDS_BIGENDIAN,no, +`ifelse(ABI,64v2, `.size C_NAME($1), . - C_NAME($1)', `.size .C_NAME($1), . - .C_NAME($1) .size C_NAME($1), . - .C_NAME($1)')')
nisse@lysator.liu.se (Niels Möller) writes:
Unfortunaly, the CI cross builds aren't working at the moment (the buildenv images are based on Debian Buster ("stable" at the time images were built), and nettle's ci scripts do apt-get update and apt-get install, which now attempts to get Bullseye packages (new "stable" since a week ago)).
Images now updated to debian stable (thanks, Daiki!). But we'll have to drop mips tests for now, since current setup assumes archs under tests are available in debian, and mips has been discontinued as a debian release architecture. Other cross builds now work (change to drop mips is on the master-updates branch). If you have ideas on how to revive mips tests, that's welcome, but for now we'll have to do without.
I would like to keep testing on big-endian. s390x is big-endian, right? And so is powerpc64 (non -el). So it would be nice to configure cross tests on one of those platforms configured with --disable-assembler, to test portability of the C code. Are s390x cross tools and qemu-user in good enough shape (it's an official debian release arch), or is powerpc64 a better option?
Regards, /Niels
On Mon, Aug 23, 2021 at 8:59 PM Niels Möller nisse@lysator.liu.se wrote:
nisse@lysator.liu.se (Niels Möller) writes:
Unfortunaly, the CI cross builds aren't working at the moment (the buildenv images are based on Debian Buster ("stable" at the time images were built), and nettle's ci scripts do apt-get update and apt-get install, which now attempts to get Bullseye packages (new "stable" since a week ago)).
Images now updated to debian stable (thanks, Daiki!). But we'll have to drop mips tests for now, since current setup assumes archs under tests are available in debian, and mips has been discontinued as a debian release architecture. Other cross builds now work (change to drop mips is on the master-updates branch). If you have ideas on how to revive mips tests, that's welcome, but for now we'll have to do without.
I would like to keep testing on big-endian. s390x is big-endian, right? And so is powerpc64 (non -el). So it would be nice to configure cross tests on one of those platforms configured with --disable-assembler, to test portability of the C code. Are s390x cross tools and qemu-user in good enough shape (it's an official debian release arch), or is powerpc64 a better option?
Yes, s390x is big-endian and it's good for such purposes. Along being officially supported in debian releases, it runs natively on remote instance in gitlab CI.
regards, Mamone
Maamoun TK maamoun.tk@googlemail.com writes:
On Mon, Aug 23, 2021 at 8:59 PM Niels Möller nisse@lysator.liu.se wrote:
I would like to keep testing on big-endian. s390x is big-endian, right? And so is powerpc64 (non -el). So it would be nice to configure cross tests on one of those platforms configured with --disable-assembler, to test portability of the C code. Are s390x cross tools and qemu-user in good enough shape (it's an official debian release arch), or is powerpc64 a better option?
Yes, s390x is big-endian and it's good for such purposes. Along being officially supported in debian releases, it runs natively on remote instance in gitlab CI.
I've just added an s390x cross-build to the gitlab ci, with --disable-assembler to exercise all #if WORDS_BIGENDIAN.
I noticed that for some of the archs (powerpc64, powerpc64el, s390x, i.e., the ones not used in gnutls tests) we don't have any cross libgmp-dev packages preinstalled in the image, and since we don't explicitly install them either, there's no test coverage of public key functions in these builds. I'll see if I can fix that.
Regards, /Niels
nisse@lysator.liu.se (Niels Möller) writes:
I've tried a different approach on branch https://git.lysator.liu.se/nettle/nettle/-/tree/ppc64-efv2-check. Patch below. (It makes sense to me to have the new check together with the ABI check, but on second thought, probably a mistake to overload the ABI variable. It would be better to have a separate configure variable, more similar to the W64_ABI).
Another iteration, on that branch (sorry for the typo in the branch name), or see patch below.
Stijn, can you try it out and see if it works for you?
Regards, /Niels
diff --git a/config.m4.in b/config.m4.in index d89325b8..b98a5817 100644 --- a/config.m4.in +++ b/config.m4.in @@ -5,6 +5,7 @@ define(`COFF_STYLE', `@ASM_COFF_STYLE@')dnl define(`TYPE_FUNCTION', `@ASM_TYPE_FUNCTION@')dnl define(`TYPE_PROGBITS', `@ASM_TYPE_PROGBITS@')dnl define(`ALIGN_LOG', `@ASM_ALIGN_LOG@')dnl +define(`ELFV2_ABI', `@ELFV2_ABI@')dnl define(`W64_ABI', `@W64_ABI@')dnl define(`RODATA', `@ASM_RODATA@')dnl define(`WORDS_BIGENDIAN', `@ASM_WORDS_BIGENDIAN@')dnl diff --git a/configure.ac b/configure.ac index ebec8759..2ed4ab4e 100644 --- a/configure.ac +++ b/configure.ac @@ -311,6 +311,9 @@ AC_SUBST([GMP_NUMB_BITS]) # Figure out ABI. Currently, configurable only by setting CFLAGS. ABI=standard
+ELFV2_ABI=no # For powerpc64 +W64_ABI=no # For x86_64 windows + case "$host_cpu" in [x86_64 | amd64]) AC_TRY_COMPILE([ @@ -355,6 +358,15 @@ case "$host_cpu" in ], [ ABI=64 ]) + if test "$ABI" = 64 ; then + AC_TRY_COMPILE([ +#if _CALL_ELF == 2 +#error ELFv2 ABI +#endif + ], [], [], [ + ELFV2_ABI=yes + ]) + fi ;; aarch64*) AC_TRY_COMPILE([ @@ -750,7 +762,6 @@ IF_DLL='#' LIBNETTLE_FILE_SRC='$(LIBNETTLE_FORLINK)' LIBHOGWEED_FILE_SRC='$(LIBHOGWEED_FORLINK)' EMULATOR='' -W64_ABI=no
case "$host_os" in mingw32*|cygwin*) @@ -1031,6 +1042,7 @@ AC_SUBST(ASM_TYPE_FUNCTION) AC_SUBST(ASM_TYPE_PROGBITS) AC_SUBST(ASM_MARK_NOEXEC_STACK) AC_SUBST(ASM_ALIGN_LOG) +AC_SUBST(ELFV2_ABI) AC_SUBST(W64_ABI) AC_SUBST(ASM_WORDS_BIGENDIAN) AC_SUBST(EMULATOR) diff --git a/powerpc64/machine.m4 b/powerpc64/machine.m4 index 187a49b8..b59f0863 100644 --- a/powerpc64/machine.m4 +++ b/powerpc64/machine.m4 @@ -1,7 +1,7 @@ define(`PROLOGUE', `.globl C_NAME($1) DECLARE_FUNC(C_NAME($1)) -ifelse(WORDS_BIGENDIAN,no, +ifelse(ELFV2_ABI,yes, `ifdef(`FUNC_ALIGN',`.align FUNC_ALIGN') C_NAME($1): addis 2,12,(.TOC.-C_NAME($1))@ha @@ -17,7 +17,7 @@ ifdef(`FUNC_ALIGN',`.align FUNC_ALIGN') undefine(`FUNC_ALIGN')')
define(`EPILOGUE', -`ifelse(WORDS_BIGENDIAN,no, +`ifelse(ELFV2_ABI,yes, `.size C_NAME($1), . - C_NAME($1)', `.size .C_NAME($1), . - .C_NAME($1) .size C_NAME($1), . - .C_NAME($1)')')
Going through some old mail... From a discussion in September:
nisse@lysator.liu.se (Niels Möller) writes:
nisse@lysator.liu.se (Niels Möller) writes:
I've tried a different approach on branch https://git.lysator.liu.se/nettle/nettle/-/tree/ppc64-efv2-check. Patch below. (It makes sense to me to have the new check together with the ABI check, but on second thought, probably a mistake to overload the ABI variable. It would be better to have a separate configure variable, more similar to the W64_ABI).
Another iteration, on that branch (sorry for the typo in the branch name), or see patch below.
Stijn, can you try it out and see if it works for you?
I haven't seen any response to this, but I've nevertheless just added these changes on the master-updates branch. It would be nice if you can confirm that it solves the problem with musl.
Regards, /Niels
nettle-bugs@lists.lysator.liu.se
-
Maamoun TK -
nisse＠lysator.liu.se