Programs using security/cryptopp crash with illegal instructions

[ftigeot]

While investigating net-p2p/amule crashes, I found out the security/cryptopp library tries to use illegal instructions.

This occurs in the CryptoPP::RandomNumberGenerator::GenerateWord32(unsigned int, unsigned int) method.

And is caused by the use of the shrx x86 instruction:

(gdb output) Program received signal SIGILL, Illegal instruction. 0x0000000000626b88 in CryptoPP::RandomNumberGenerator::GenerateWord32(unsigned int, unsigned int) () => 0x0000000000626b88 <_ZN8CryptoPP21RandomNumberGenerator14GenerateWord32Ejj+40>: c4 e2 73 f7 c3 shrx %ecx,%ebx,%eax

If I'm not mistaken, shrx is a new Haswell instruction, which means programs using the cryptopp library will crash on all machines not using this latest Intel processor generation.

This is most likely caused by a configure issue and is 100% reproductible so far. Locally built cryptopp binaries on an Ivy-Bridge CPU fail to run on this same CPU.

[ftigeot]

I was mistaken about the configure issue: cryptopp binaries built on a particular CPU can run on this CPU. Howewer, the cryptopp package only provides a static .a file and packages using it have to be rebuilt to use the new code.

[ftigeot]

The root of the issue most likely comes from the CXXFLAGS line in GNUmakefile. Changing it to use to only generate instructions for the base amd64 and/or i386 microarchitecture implementations should fix packages once and for all.

Pseudo-patch for amd64:

-CXXFLAGS += -march=native +CXXFLAGS += -march=k8 -mtune=generic

[jrmarino]

good investigation work. Shouldn't FreeBSD also suffer from this? I think we can change value of CXXFLAGS depending if PACKAGE_BUILDING is set or not. (e.g. march=native is used if built from source, generic if it's a bulk build)

[ftigeot]

Using -native by default is too dangerous to be selectively patched IMHO. It would be best to have the port respect CFLAGS; if special flags are to be used, this should only be done after a conscious choice by the user.

[ftigeot]

FreeBSD probably doesn't see any issue right now due to the old gcc version it still uses by default.

[jrmarino]

The only issue with conditional CXXFLAGS is if somebody is mixing binary packages with source builds. If it's all binary or all source, there should be no issue. We could always add a warning message that shows after the build...

(The assumption being native is a big deal performance wise...)

[ftigeot]

It isn't in this case, cryptopp routines autodetect cpu features at runtime (most performance sensitive software does the same thing). The Haswell instruction was generated from a C++ method prefix if I remember correctly.

[jrmarino]

That response leads me to think we're talking about slightly different things.

I said if "native" on Haswell is a big performance boost over forcing march to k8, then perhaps we should not set it to k8 unconditionally in every use case.

It doesn't matter about autodetect if the configuration says, "use k8 regardless".

So I was proprosing that the dports package are always built with k8, but building from source results in "native" instructions. In general I think that should be fine, but a little advisory might be in order though.

[ftigeot]

What if the user wants to copy the resulting binary on a different machine ? Make it available via nfs ? This will only lead to new bug reports and a bad reputation. There is also no speed to be gained here: the Haswell instruction was generated from a regular C++ statement due to the "-native" flag; performance sensitive routines in this port are already written in assembly language and selected at runtime.

[jrmarino]

What should the flag be for i386? What is the reason for explicitly defining march=-native?

[ftigeot]

Developers only thinking about their local machine ? It would not be the first time.

I'm starting to think we should not force the use of a particular CFLAGS value but reuse whatever the build system is using by default. ie, take the value from /etc/make.conf or whatever poudriere / the user sets it to.

[jrmarino]

Can you open a FreeBSD PR about the issue? I suspect the maintainer doesn't know about this issue and will agree to use a generic instruction set, but it will have to be defined per arch.

[ftigeot]

FreeBSD PR opened: http://www.freebsd.org/cgi/query-pr.cgi?pr=185826

[ftigeot]

Patch in FreeBSD PR committed.