[David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

GnuPG - Libgcrypt - Dev mailing list
* cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
conditionalize on whether __arm__ is defined.

--

When building for i686, configure detects that the assembler can
use the different architectures, so it defined everything under the
HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
block is first and the following x86 block only defines UMUL_ADD_32
if it's not already defined, the lto-wrapper failed during linking
with a pile of "no such instruction: umlal ..." errors.  Gating on
__arm__ prevents that initial defintion and fixes the errors.
---
 cipher/poly1305.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cipher/poly1305.c b/cipher/poly1305.c
index 6cb4d2b7..d9706ced 100644
--- a/cipher/poly1305.c
+++ b/cipher/poly1305.c
@@ -289,7 +289,7 @@ static unsigned int poly1305_final (poly1305_context_t *ctx,
 
 #ifdef USE_MPI_32BIT
 
-#ifdef HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
+#if defined (__arm__) && defined (HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS)
 
 /* HI:LO += A * B (arm) */
 #define UMUL_ADD_32(HI, LO, A, B) \
@@ -308,7 +308,7 @@ static unsigned int poly1305_final (poly1305_context_t *ctx,
        : "r" (B0), "r" (B1), "r" (B2), "r" (B3), "r" (B4) \
        : "cc" )
 
-#endif /* HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS */
+#endif /* __arm__ */
 
 #if defined (__i386__) && __GNUC__ >= 4
 
--
2.26.2




--
* Free Assange and protect free journalism!
* Germany: Sign the Treaty on the Prohibition of Nuclear Weapons!

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel

signature.asc (233 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

Jussi Kivilinna-2
Hello,

On 22.1.2021 13.21, Werner Koch via Gcrypt-devel wrote:

> ForwardedMessage.eml / David Michael <[hidden email]>:
>
> * cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
> conditionalize on whether __arm__ is defined.
>
> --
>
> When building for i686, configure detects that the assembler can
> use the different architectures, so it defined everything under the
> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
> block is first and the following x86 block only defines UMUL_ADD_32
> if it's not already defined, the lto-wrapper failed during linking
> with a pile of "no such instruction: umlal ..." errors.  Gating on
> __arm__ prevents that initial defintion and fixes the errors.
>

HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS should not be defined on x86/i686
in first place.

Problem must be in 'configure.ac' in the HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
check. Does this issue happen with "clang -flto"?

There was another issue reported regarding clang LTO on x86_64
(https://dev.gnupg.org/T5255) and there problem was caused partly by
another 'configure.ac' check not working with clang when LTO enabled.
Fix there was to change assembly check to be done with compile+link
instead of just compile.

-Jussi

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Reply | Threaded
Open this post in threaded view
|

Re: [David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

GnuPG - Libgcrypt - Dev mailing list
On Fri, Jan 22, 2021 at 10:32 AM Jussi Kivilinna <[hidden email]> wrote:

> Hello,
>
> On 22.1.2021 13.21, Werner Koch via Gcrypt-devel wrote:
> > ForwardedMessage.eml / David Michael <[hidden email]>:
> >
> > * cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
> > conditionalize on whether __arm__ is defined.
> >
> > --
> >
> > When building for i686, configure detects that the assembler can
> > use the different architectures, so it defined everything under the
> > HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
> > block is first and the following x86 block only defines UMUL_ADD_32
> > if it's not already defined, the lto-wrapper failed during linking
> > with a pile of "no such instruction: umlal ..." errors.  Gating on
> > __arm__ prevents that initial defintion and fixes the errors.
> >
>
> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS should not be defined on x86/i686
> in first place.

Using i686 GCC has these in config.log:

#define HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS 1
#define HAVE_COMPATIBLE_GCC_AARCH64_PLATFORM_AS 1
#define HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS 1

> Problem must be in 'configure.ac' in the HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
> check. Does this issue happen with "clang -flto"?

If I set CC to i686 clang, it first fails because it doesn't undestand
this line in jitterentropy-base.c

#pragma GCC optimize ("O0")

If I use -O0 in CFLAGS, then it fails due to the ARM instructions
while linking libgcrypt.so again.  It has the same HAVE_COMPATIBLE_GCC
definitions in config.log as with GCC.

<inline asm>:1:2: error: invalid instruction mnemonic 'umlal'
        umlal %ecx, %eax, %edx, %esi
        ^~~~~
LLVM ERROR: Error parsing inline asm

Thanks.

David

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Reply | Threaded
Open this post in threaded view
|

Re: [David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

Jussi Kivilinna-2
On 22.1.2021 18.28, David Michael wrote:

> On Fri, Jan 22, 2021 at 10:32 AM Jussi Kivilinna <[hidden email]> wrote:
>> Hello,
>>
>> On 22.1.2021 13.21, Werner Koch via Gcrypt-devel wrote:
>>> ForwardedMessage.eml / David Michael <[hidden email]>:
>>>
>>> * cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
>>> conditionalize on whether __arm__ is defined.
>>>
>>> --
>>>
>>> When building for i686, configure detects that the assembler can
>>> use the different architectures, so it defined everything under the
>>> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
>>> block is first and the following x86 block only defines UMUL_ADD_32
>>> if it's not already defined, the lto-wrapper failed during linking
>>> with a pile of "no such instruction: umlal ..." errors.  Gating on
>>> __arm__ prevents that initial defintion and fixes the errors.
>>>
>>
>> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS should not be defined on x86/i686
>> in first place.
>
> Using i686 GCC has these in config.log:
>
> #define HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS 1
> #define HAVE_COMPATIBLE_GCC_AARCH64_PLATFORM_AS 1
> #define HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS 1
>
>> Problem must be in 'configure.ac' in the HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
>> check. Does this issue happen with "clang -flto"?
>
Hm, ok. Maybe the LTO/configure.ac problem manifests itself also with GCC
in some system configurations. Does attached patch help?

-Jussi

> If I set CC to i686 clang, it first fails because it doesn't undestand
> this line in jitterentropy-base.c
>
> #pragma GCC optimize ("O0")
>
> If I use -O0 in CFLAGS, then it fails due to the ARM instructions
> while linking libgcrypt.so again.  It has the same HAVE_COMPATIBLE_GCC
> definitions in config.log as with GCC.
>
> <inline asm>:1:2: error: invalid instruction mnemonic 'umlal'
>          umlal %ecx, %eax, %edx, %esi
>          ^~~~~
> LLVM ERROR: Error parsing inline asm
>
> Thanks.
>
> David
>

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel

0001-configure.ac-run-assembler-checks-through-linker-for.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

GnuPG - Libgcrypt - Dev mailing list
On Fri, Jan 22, 2021 at 12:18 PM Jussi Kivilinna <[hidden email]> wrote:

> On 22.1.2021 18.28, David Michael wrote:
> > On Fri, Jan 22, 2021 at 10:32 AM Jussi Kivilinna <[hidden email]> wrote:
> >> Hello,
> >>
> >> On 22.1.2021 13.21, Werner Koch via Gcrypt-devel wrote:
> >>> ForwardedMessage.eml / David Michael <[hidden email]>:
> >>>
> >>> * cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
> >>> conditionalize on whether __arm__ is defined.
> >>>
> >>> --
> >>>
> >>> When building for i686, configure detects that the assembler can
> >>> use the different architectures, so it defined everything under the
> >>> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
> >>> block is first and the following x86 block only defines UMUL_ADD_32
> >>> if it's not already defined, the lto-wrapper failed during linking
> >>> with a pile of "no such instruction: umlal ..." errors.  Gating on
> >>> __arm__ prevents that initial defintion and fixes the errors.
> >>>
> >>
> >> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS should not be defined on x86/i686
> >> in first place.
> >
> > Using i686 GCC has these in config.log:
> >
> > #define HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS 1
> > #define HAVE_COMPATIBLE_GCC_AARCH64_PLATFORM_AS 1
> > #define HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS 1
> >
> >> Problem must be in 'configure.ac' in the HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
> >> check. Does this issue happen with "clang -flto"?
> >
>
> Hm, ok. Maybe the LTO/configure.ac problem manifests itself also with GCC
> in some system configurations. Does attached patch help?

Yes, that seems to result in only
HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS being defined for i686, so it
successfully builds.

Thanks.

David

_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Reply | Threaded
Open this post in threaded view
|

Re: [David Michael] [PATCH libgcypt 2/2] cipher/poly1305: Fix 32-bit x86 compilation

Jussi Kivilinna-2
On 22.1.2021 19.32, David Michael wrote:

> On Fri, Jan 22, 2021 at 12:18 PM Jussi Kivilinna <[hidden email]> wrote:
>> On 22.1.2021 18.28, David Michael wrote:
>>> On Fri, Jan 22, 2021 at 10:32 AM Jussi Kivilinna <[hidden email]> wrote:
>>>> Hello,
>>>>
>>>> On 22.1.2021 13.21, Werner Koch via Gcrypt-devel wrote:
>>>>> ForwardedMessage.eml / David Michael <[hidden email]>:
>>>>>
>>>>> * cipher/poly1305.c [HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS]: Also
>>>>> conditionalize on whether __arm__ is defined.
>>>>>
>>>>> --
>>>>>
>>>>> When building for i686, configure detects that the assembler can
>>>>> use the different architectures, so it defined everything under the
>>>>> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS conditional block.  Since that
>>>>> block is first and the following x86 block only defines UMUL_ADD_32
>>>>> if it's not already defined, the lto-wrapper failed during linking
>>>>> with a pile of "no such instruction: umlal ..." errors.  Gating on
>>>>> __arm__ prevents that initial defintion and fixes the errors.
>>>>>
>>>>
>>>> HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS should not be defined on x86/i686
>>>> in first place.
>>>
>>> Using i686 GCC has these in config.log:
>>>
>>> #define HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS 1
>>> #define HAVE_COMPATIBLE_GCC_AARCH64_PLATFORM_AS 1
>>> #define HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS 1
>>>
>>>> Problem must be in 'configure.ac' in the HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS
>>>> check. Does this issue happen with "clang -flto"?
>>>
>>
>> Hm, ok. Maybe the LTO/configure.ac problem manifests itself also with GCC
>> in some system configurations. Does attached patch help?
>
> Yes, that seems to result in only
> HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS being defined for i686, so it
> successfully builds.

Great! That is the same as I have for non-LTO GCC build:

  $ cat config.h|grep COMPATIBLE
  /* #undef HAVE_COMPATIBLE_CC_PPC_ALTIVEC */
  /* #undef HAVE_COMPATIBLE_CC_PPC_ALTIVEC_WITH_CFLAGS */
  /* #undef HAVE_COMPATIBLE_GCC_AARCH64_PLATFORM_AS */
  #define HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS 1
  /* #undef HAVE_COMPATIBLE_GCC_ARM_PLATFORM_AS */
  /* #undef HAVE_COMPATIBLE_GCC_WIN64_PLATFORM_AS */

HAVE_COMPATIBLE_GCC_AMD64_PLATFORM_AS check currently has 32-bit
instruction to check that system is x86. Therefore check succeeds
on x86_64 and on (some) i686 systems and that is expected.

-Jussi

>
> Thanks.
>
> David
>


_______________________________________________
Gcrypt-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel