[openssl-dev] [openssl.org #3650] sha1-586.asm broken in 1.0.2-stable for Windows builds

[Andy Polyakov via RT]

>> I am also having a issue this issue.  It is a 32 bit build issue
>> only.  The 64 bit build completes using the same development
>> environment.  The offending instruction is "movd".  Unfortunately
>> I am not a x86 assembler expert.
>>
>> Mark
>>
>>         perl crypto\sha\asm\sha1-586.pl win32 /MD /Ox /O2 /Ob2 -DOPENSSL_THREADS
>>   -DDSO_WIN32 -W3 -Gs0 -GF -Gy -nologo -DOPENSSL_SYSNAME_WIN32 -DWIN32_LEAN_AND_
>> MEAN -DL_ENDIAN -D_CRT_SECURE_NO_DEPRECATE -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL
>> _IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -
>> DSHA512_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH
>> _ASM -DOPENSSL_USE_APPLINK -I. -DOPENSSL_NO_RC5 -DOPENSSL_NO_MD2 -DOPENSSL_NO_KR
>> B5 -DOPENSSL_NO_JPAKE -DOPENSSL_NO_DYNAMIC_ENGINE >tmp32dll\sha1-586.asm
>>         ml /nologo /Cp /coff /c /Cx /Zi /Fotmp32dll\sha1-586.obj tmp32dll\sha1-5
>> 86.asm
>>  Assembling: tmp32dll\sha1-586.asm
>> tmp32dll\sha1-586.asm(1432) : error A2070:invalid instruction operands
>> tmp32dll\sha1-586.asm(1576) : error A2070:invalid instruction operands
>> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 11.0
>> \VC\BIN\ml.EXE"' : return code '0x1'
>> Stop.
>>
>> $Lshaext_shortcut::
>> 	mov	edi,DWORD PTR 20[esp]
>> 	mov	ebx,esp
>> 	mov	esi,DWORD PTR 24[esp]
>> 	mov	ecx,DWORD PTR 28[esp]
>> 	sub	esp,32
>> 	movdqu	xmm0,XMMWORD PTR [edi]
>> 	movd	xmm1,XMMWORD PTR 16[edi]		* 1432
> 
> Ah! It should be DWORD, not XMMWORD... Devja vu! There was similar case
> with movq elsewhere. See if attached fixes the problem.

It was brought to my attention that proposed patch creates other
problems. Attached one addresses those, as well as incorporates part of
solution suggested by Steve Kneizys. However! Even if x86masm.pl is
confirmed to work and is committed to repository, it will not make masm
supported. It will be available on don't-ask-in-case-of-doubt-use-nasm
basis. There are too many variables with masm while upgrading it is not
an option (because it's not available as separate packet). I mean since
we can't suggest to upgrade it, we would have to keep track of versions
and capabilities (instruction set and safeseh support), and it's not
exactly productive. One of concerns risen was that one would have to
answer question "why does one need to use something besides visual
studio". Well, you already use something besides visual studio, perl, so
how nasm is different? As for suggested workarounds, how using nasm is
different from writing custom program that pretends to be nasm but calls
masm, or providing extra script? I mean aren't they as "bad" as just
using nasm?

On side note. nasm is preferred option even in 64-bit context. Even
though we attempt to keep track of ml64 versions, using recent nasm
guarantees that you use all processor extensions, even when visual
studio doesn't.

-------------- next part --------------
diff --git a/crypto/perlasm/x86masm.pl b/crypto/perlasm/x86masm.pl
index 1741342..917d0f8 100644
--- a/crypto/perlasm/x86masm.pl
+++ b/crypto/perlasm/x86masm.pl
@@ -18,10 +18,10 @@ sub ::generic
 
     if ($opcode =~ /lea/ && @arg[1] =~ s/.*PTR\s+(\(.*\))$/OFFSET $1/)	# no []
     {	$opcode="mov";	}
-    elsif ($opcode !~ /movq/)
+    elsif ($opcode !~ /mov[dq]$/)
     {	# fix xmm references
-	$arg[0] =~ s/\b[A-Z]+WORD\s+PTR/XMMWORD PTR/i if ($arg[1]=~/\bxmm[0-7]\b/i);
-	$arg[1] =~ s/\b[A-Z]+WORD\s+PTR/XMMWORD PTR/i if ($arg[0]=~/\bxmm[0-7]\b/i);
+	$arg[0] =~ s/\b[A-Z]+WORD\s+PTR/XMMWORD PTR/i if ($arg[-1]=~/\bxmm[0-7]\b/i);
+	$arg[-1] =~ s/\b[A-Z]+WORD\s+PTR/XMMWORD PTR/i if ($arg[0]=~/\bxmm[0-7]\b/i);
     }
 
     &::emit($opcode, at arg);
@@ -160,13 +160,13 @@ sub ::public_label
 {   push(@out,"PUBLIC\t".&::LABEL($_[0],$nmdecor.$_[0])."\n");   }
 
 sub ::data_byte
-{   push(@out,("DB\t").join(',', at _)."\n");	}
+{   push(@out,("DB\t").join(',',splice(@_,0,16))."\n") while(@_);	}
 
 sub ::data_short
-{   push(@out,("DW\t").join(',', at _)."\n");	}
+{   push(@out,("DW\t").join(',',splice(@_,0,8))."\n") while(@_);	}
 
 sub ::data_word
-{   push(@out,("DD\t").join(',', at _)."\n");	}
+{   push(@out,("DD\t").join(',',splice(@_,0,4))."\n") while(@_);	}
 
 sub ::align
 {   push(@out,"ALIGN\t$_[0]\n");	}
diff --git a/crypto/sha/asm/sha1-586.pl b/crypto/sha/asm/sha1-586.pl
index 8377299..4895eb3 100644
--- a/crypto/sha/asm/sha1-586.pl
+++ b/crypto/sha/asm/sha1-586.pl
@@ -450,7 +450,7 @@ sub sha1msg2	{ sha1op38(0xca, at _); }
 	&sub	("esp",32);
 
 	&movdqu	($ABCD,&QWP(0,$ctx));
-	&movd	($E,&QWP(16,$ctx));
+	&movd	($E,&DWP(16,$ctx));
 	&and	("esp",-32);
 	&movdqa	($BSWAP,&QWP(0x50,$tmp1));	# byte-n-word swap