bug in ghash-x86_64.pl for non-AVX assemblers?

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

bug in ghash-x86_64.pl for non-AVX assemblers?

Michael Wojcik
I'm trying to build 1.1.1b on an old RHEL 5.5 machine (for reasons too tiresome to explain). It has a rather ancient version of the Gnu assembler, 2.17.50, which ghash-x86_64.pl decides means it doesn't have AVX support. (The script looks for 2.20 or later, basically. I assume that's correct.)

The problem is that it generates invalid assembly when AVX isn't available. The output from the assembler is:

-----
crypto/modes/ghash-x86_64.s: Assembler messages:
crypto/modes/ghash-x86_64.s:1311: Error: previous CFI entry not closed (missing .cfi_endproc)
crypto/modes/ghash-x86_64.s:1374: Error: open CFI at the end of file; missing .cfi_endproc directive
-----

Looking at ghash-x86_64.s, I can see that indeed there's no .cfi_endproc before line 1311. gcm_init_avx is missing its endproc. And looking at the script, the reason seems obvious, unless I'm missing something:

-----
$code.=<<___;
.globl   gcm_init_avx
.type gcm_init_avx,\@abi-omnipotent
.align   32
gcm_init_avx:
.cfi_startproc
___
if ($avx) {
...
$code.=<<___;
   ret
.cfi_endproc
.size gcm_init_avx,.-gcm_init_avx
___
} else {
$code.=<<___;
   jmp   .L_init_clmul
.size gcm_init_avx,.-gcm_init_avx
___
}
----

There's a .cfi_endproc in the here-doc on the if ($avx) branch, but none in the here-doc on the else branch. So if the assembler doesn't have AVX support, the script doesn't emit the .cfi_endproc directive.

Same thing with gcm_ghash_avx.

Is this a bug in ghash-x86_64.pl, or am I misinterpreting?

I've just started looking at this, so apologies if it's a known issue that's already been fixed in a later release or on the master.

Thanks,
Michael Wojcik
Distinguished Engineer, Micro Focus



Reply | Threaded
Open this post in threaded view
|

Re: bug in ghash-x86_64.pl for non-AVX assemblers?

Richard Levitte - VMS Whacker-2
This bug was found a couple of days after 1.1.1b was released. The fix us found in commit dbb1340314fcec37662d79720e6566fdd5a232e8

The best thing you can do is to update your source to 1.1.1c, which has this fix and more.

Cheers
Richard


Michael Wojcik <[hidden email]> skrev: (2 augusti 2019 16:00:59 CEST)

>I'm trying to build 1.1.1b on an old RHEL 5.5 machine (for reasons too
>tiresome to explain). It has a rather ancient version of the Gnu
>assembler, 2.17.50, which ghash-x86_64.pl decides means it doesn't have
>AVX support. (The script looks for 2.20 or later, basically. I assume
>that's correct.)
>
>The problem is that it generates invalid assembly when AVX isn't
>available. The output from the assembler is:
>
>-----
>crypto/modes/ghash-x86_64.s: Assembler messages:
>crypto/modes/ghash-x86_64.s:1311: Error: previous CFI entry not closed
>(missing .cfi_endproc)
>crypto/modes/ghash-x86_64.s:1374: Error: open CFI at the end of file;
>missing .cfi_endproc directive
>-----
>
>Looking at ghash-x86_64.s, I can see that indeed there's no
>.cfi_endproc before line 1311. gcm_init_avx is missing its endproc. And
>looking at the script, the reason seems obvious, unless I'm missing
>something:
>
>-----
>$code.=<<___;
>.globl   gcm_init_avx
>.type gcm_init_avx,\@abi-omnipotent
>.align   32
>gcm_init_avx:
>.cfi_startproc
>___
>if ($avx) {
>...
>$code.=<<___;
>   ret
>.cfi_endproc
>.size gcm_init_avx,.-gcm_init_avx
>___
>} else {
>$code.=<<___;
>   jmp   .L_init_clmul
>.size gcm_init_avx,.-gcm_init_avx
>___
>}
>----
>
>There's a .cfi_endproc in the here-doc on the if ($avx) branch, but
>none in the here-doc on the else branch. So if the assembler doesn't
>have AVX support, the script doesn't emit the .cfi_endproc directive.
>
>Same thing with gcm_ghash_avx.
>
>Is this a bug in ghash-x86_64.pl, or am I misinterpreting?
>
>I've just started looking at this, so apologies if it's a known issue
>that's already been fixed in a later release or on the master.
>
>Thanks,
>Michael Wojcik
>Distinguished Engineer, Micro Focus

--
Richard by mobile
Reply | Threaded
Open this post in threaded view
|

RE: bug in ghash-x86_64.pl for non-AVX assemblers?

Michael Wojcik
> From: openssl-users [mailto:[hidden email]] On Behalf Of
> Richard Levitte
> Sent: Friday, August 02, 2019 12:07
> To: [hidden email]
> Subject: Re: bug in ghash-x86_64.pl for non-AVX assemblers?
>
> This bug was found a couple of days after 1.1.1b was released. The fix us
> found in commit dbb1340314fcec37662d79720e6566fdd5a232e8
>
> The best thing you can do is to update your source to 1.1.1c, which has this
> fix and more.

Excellent. Many thanks. I think we hadn't picked up 1.1.1c yet because it was non-critical and we were close to a product release when it came out, but there's no reason not to move to it now.

--
Michael Wojcik
Distinguished Engineer, Micro Focus