Re: [PATCH v2] block-sha1: remove use of assembly

> On Wed, Mar 09, 2022 at 10:10:33PM +0000, brian m. carlson wrote:
>> On 2022-03-08 at 13:38:06, Ævar Arnfjörð Bjarmason wrote:
>> >
>> > On Tue, Mar 08 2022, brian m. carlson wrote:
>> >
>> > I think the $subject of the patch needs updating. It's not removing all
>> > the assemply from the file, after this patch we still have the
>> > ARM-specific assembly.
>> >
>> > I don't have a box to test that on, but I wonder if that also triggers
>> > the pedantic mode?
>> >
>> > Perhaps:
>> >
>> >     block-sha1: remove superfluous i386 and x86-64 assembly
>>
>> I suspect it has the same problem.  My inclination is to just remove it,
>> because my guess is that the compiler has gotten smarter between 2009
>> and now.
>
> Almost certainly. I don't have a machine to test it on, either, but I
> would be shocked if `make BLK_SHA=YesPlease DEVELOPER=1` worked on
> master today on an arm machine.

>> I honestly intend to just remove this code in a future version because
>> everyone not using SHA1DC has a security problem and we shouldn't offer
>> insecure options.
>>
>> However, I think for now, I'm just going to reroll this with the new
>> title and then I can remove it in a future version unless somebody with
>> an ARM system can relatively quickly tell me whether it's necessary.
>
> I wonder if a good stop-gap for arm systems might be to do something
> like:
>
> --- 8< ---
>
> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
> index 1bb6e7c069..7402d02875 100644
> --- a/block-sha1/sha1.c
> +++ b/block-sha1/sha1.c
> @@ -57,7 +57,7 @@
>  #if defined(__i386__) || defined(__x86_64__)
>    #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
>  #elif defined(__GNUC__) && defined(__arm__)
> -  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
> +  #define setW(x, val) do { W(x) = (val); __extension__ __asm__("":::"memory"); } while (0)
>  #else
>    #define setW(x, val) (W(x) = (val))
>  #endif
>
> --- >8 ---