Closed Bug 370780 Opened 17 years ago Closed 16 years ago

inline assembler breaks Windows x64 build

Categories

(NSS :: Libraries, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 227049

People

(Reporter: mmoy, Unassigned)

Details

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1) Gecko/20061025 BonEcho/2.0 (mmoy CE K8C-X01)
Build Identifier: Firefox 2.0.0.1

/cygdrive/f/Mozilla/mozilla/build/cygwin-wrapper cl -Fof:/Mozilla/mozilla/nss/freebl/WIN95_SINGLE_SHLIB/prng_fips18 61.obj -c -O2 -Ox   -MD -W3 -nologo -DXP_PC -DSHLIB_SUFFIX=\"dll\" -DSHLIB_PREFIX=\"\" -DSHLIB_VERSION=\"3\" -DSOFT OKEN_SHLIB_VERSION=\"3\" -DNSS_ENABLE_ECC -DRIJNDAEL_INCLUDE_TABLES -UDEBUG -U_DEBUG -DNDEBUG -DWIN32 -D_WINDOWS -D_X86_ -DWIN95 -DNSS_ENABLE_ECC -DNSS_USE_64 -DMP_NO_MP_WORD -DMP_USE_UINT_DIGIT -DMP_API_COMPATIBLE -If:/Mozilla/mozilla/dist/include/nspr -If:/Mozilla/mozilla/dist/include  -If:/Mozilla/mozilla/dist/public/nss -If:/Mozilla/mozilla/dist/private/nss -If:/Mozilla/mozilla/dist/include -If:/Mozilla/mozilla/dist/include/dbm -Impi -Iecl  f:/Mozilla/mozilla/security/nss/lib/freebl/prng_fips1861.c prng_fips1861.c
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(74) : error C4235: nonstandard extension used : '__asm' keyword not supported on this architecture
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(75) : error C2065: 'mov' : undeclared identifier
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(75) : error C2146: syntax error : missing ';' before identifier 'eax'
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(75) : error C2065: 'eax' : undeclared identifier
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(76) : error C2146: syntax error : missing ';' before identifier 'bswap'
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(76) : error C2065: 'bswap' : undeclared identifier
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(76) : error C2146: syntax error : missing ';' before identifier 'eax'
f:\mozilla\mozilla\security\nss\lib\freebl\sha_fast.h(77) : error C2143: syntax error : missing ';' before '}'

Here's the code that it's choking on:

#if defined(_MSC_VER) && defined(_X86_)
#if defined(IS_LITTLE_ENDIAN) 
#ifndef FORCEINLINE
#if (_MSC_VER >= 1200)
#define FORCEINLINE __forceinline
#else
#define FORCEINLINE __inline
#endif /* _MSC_VER */
#endif /* !defined FORCEINLINE */
#define FASTCALL __fastcall

static FORCEINLINE PRUint32 FASTCALL 
swap4b(PRUint32 dwd) 
{
    __asm {
    	mov   eax,dwd
	bswap eax
    }
}

There should be protection for Win x64 though offhand I don't see a define on the command line that indicates 64-bits. It appears that some of the more common  x64 defines aren't propagated down to this code.

An alternative to using inline assembler here would be to use the _byteswap_ulong intrinsic described at http://msdn2.microsoft.com/en-us/library/a3140177(VS.80).aspx which would cover Win32 and Win x64 (I've verified Win x64 but the documentation on this doesn't indicate x64-only; I don't have VS2005 Win32 installed on my current machine at the moment). I'm not sure which version of Visual C/C++ this was added in so some research or testing would be required there.

The optimization switch -Oi or -O2 would be needed to generate inline code as opposed to a called function. The generated code with either switch should be more efficient than inline assembler as one or more stack moves, common when
referencing variables in inline assembler can be optimized out.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



I haven't checked the trunk code to see if this problem exists there.
The whole problem is:  
   error C4235: '__asm' keyword not supported on this architecture

That's going to be a problem for several NSS source files that use inline asm.
See http://lxr.mozilla.org/security/search?string=__asm for the other places
that use it.

A few months ago, there was a big push to get rid of .asm files and use inline
asm everywhere instead, because MSVC 2005 Express didn't include MASM, but did
compile MASM syntax as inline assembler.  

Now maybe we're going to have to reverse that for MSVC x86_64.  

I wonder, does __asm__ work on this architecture?
I see NSS uses __asm, __asm__, and _asm for intel assembler.  
See http://lxr.mozilla.org/security/search?string=_asm

Maybe one of those works for 64-bit cl ?
Summary: sha_fast.h inline assembler breaks Windows x64 build → inline assembler breaks Windows x64 build
I think that this code change will work in this case. At least it compiles.
I'm running into problems and fixing them in trying to get a working Win64
build and the problems are varied.

ml64 is included with the x64 Platform SDK. I assume that ml.exe is included
with the Win32 Platform SDK though I don't have a system handy to check on
that.

I don't have access to the Intel compiler and can't test it but I am unaware of anyone attempting to build Firefox x64 Windows on the Intel compiler. If someone has access to it, then they could test __asm{} on the 64-bit side. But if Microsoft's not supporting it, then Intel shouldn't support it for compatibility.

Microsoft appears to be pushing intrinsics to get assembler performance where the language doesn't provide support for specific instructions. The advantage to this is that you don't have the extra stack moves seen with inline assembler. The disadvantage is that you lose some control. This can be a problem for SIMD intrinsics but usually isn't a problem for non-SIMD intrinsics as the instruction sequences are short.

Is there a way to automatically get added to bugs involving __asm{} or intrinsics on Windows?

#define swap4b(dwd) (_byteswap_ulong(dwd))

/*
static FORCEINLINE PRUint32 FASTCALL 
swap4b(PRUint32 dwd) 
{
    __asm {
    	mov   eax,dwd
	bswap eax
    }
}
*/
The code has to build for MSVC 6, MSVC 8 (a.k.a 2005, express), and now cl64.
The code presently also builds with MSVC 5, and I'd like to preserve that 
if possible.  I'm pretty sure that some of the intrinsics you're thinking of
are new, not in the older MSVCs.  So, what we need are preprocessor 
expressions (such as defined(_MSC_VER) and (_MSC_VER >= 1200)) that cause 
the right code to be built for each platform.  

Can you suggest a preprocessor expression that tells us when we're compiling
with cl64?
I'm using Makoto's stuff for building Win64 and I see that _AMD64, _M_AMD64 and _WIN64 are defined on the command line for some modules but these don't propagate to the security tree. I'll look around to see if Microsoft has something defined that indicates a 64-bit compiler.

sha512.c also fails on __asm{} code. It's the same code that's in sha_fast.h.
You'll find _asm in all these files:

freebl/ecl/ecl_gf.c
freebl/ecl/ecp_192.c
freebl/mpi/mp_comba.c
freebl/mpi/mpcpucache.c
freebl/mpi/mpi_x86_asm.c
freebl/sha512.c
freebl/sha_fast.h
freebl/win_rand.c
I got all the way to shlibsign (which crashed when running the exe) so I suspect that there is some kind of protection on the rest of those files. A few are GCC
inline assembler and I'm not sure why the others compile. Could be Makoto's code has them taken care of.

My last Win64 build was 2.0.0.0 so the only problems that I should have run into with this 2.0.0.1 build would be things that changed in this maintenance release.

winnt.h implies that _WIN64 is defined when running the 64-bit compiler and not
defined when running the 32-bit compiler. I ran a few tests and confirmed that
this does work. There's also _AMD64_. _WIN64 should be good enough unless there's the possibility of an Windows Itanium build.
I decided to run the build on my old laptop and the build went smoothly except for one problem. The difference between the old system and the new system is that the new system has the R2 PSDK while the old one has the SP1 PSDK. I ran a few tests and the old PSDK didn't define _X86_ but the new one does and that's why the compiler tried to pick up the inline assembler code.

I am replacing the new PSDK with the old one on my desktop and will just build with that for now as I don't have the time to debug the issues with the new PSDK.

On this particular issue I think that some conditional code for VC8 and later should use the intrinsic and the -Oi optimization switch for better performance than inline assembler which would also be picked up by Windows x64 builds.
What is "Makoto's stuff"?  Never heard of it before this.

The _asm work went into NSS ~2 years ago, so unless FF 2.0.0.,0 was built 
with 2+ year oldl NSS, that NSS code hasn't changed since 2.000. 

Yes, HP builds NSS for iTanium, so we shouldn't ignore that, but I don't
think they build for Windows iTanium.  

If you can get a stack trace for the shlibsign crash, let me know.
should you be dup of bug 227049?  I attached x64 asm code in bug 227049.
Yes, this is a dupe.
Status: UNCONFIRMED → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.