Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make-based cpu feature tests and language #741

Closed
noloader opened this issue Nov 18, 2018 · 8 comments
Closed

Make-based cpu feature tests and language #741

noloader opened this issue Nov 18, 2018 · 8 comments
Labels

Comments

@noloader
Copy link
Collaborator

noloader commented Nov 18, 2018

While working Issue 656 it suddenly occurred to me... This "bad result" is not internationalized (from GNUmake, around line 100):

BAD_RESULT="fatal|error|unknown|unrecognized|illegal|ignored|incorrect|not found|not exist|
  cannot find|not supported|not compatible|no such instruction|invalid mnemonic"

At this time we seem to have been running between the cracks. It seems we have been using English testers and modern Linux systems that have all the feature support. I think we need to fix the gap before it becomes a widespread problem.

We have several choices:

  1. Revert to toolchain version numbers

  2. Require folks to perform an English compile

  3. Add international language support

  4. Use output artifact as the gate

  5. Use a "clean compile" as the gate

All of the choices are pretty shitty except (4) and (5). Item (4) may be a good choice as it proves the toolchain can consume the options and produce an artifact like a.out.

Item (5) may be a good choice long as we stay warning-free, which is what we do anyways. For example, here is the SSE2 test under normal conditions. Notice there are no messages, and we can deduce the option was accepted:

skylake:cryptopp$ g++ -DNDEBUG -g2 -O0 -msse2 TestPrograms/test_x86_sse2.cxx
skylake:cryptopp$

And here's what happens when the compiler cannot handle an option. Notice the presence of output when things go sideways.

$ g++ -DNDEBUG -g2 -O0 -mXXX TestPrograms/test_x86_sse2.cxx
g++: error: unrecognized command line option ‘-mXXX’

We may be able to combine options (4) and (5).

The caveat I am aware for option (5) is, some OS X compilers generate a warning stating -pthreads have no effect, passing to the linker (or similar). In this case we would reject a valid option.

We'll keep this report open to solicit ideas and gather feedback.

@noloader
Copy link
Collaborator Author

noloader commented Nov 19, 2018

We started testing Method (5), "clean compile as the gate", at Commit 1ac7207545f3 . 1ac7207 is limited to PowerPC at the moment. PowerPC is not all that limited. For us, it covers (1) AIX, Linux and OS X operating systems; (2) Apple, Clang, IBM, and GCC compilers; and (3) Altivec, POWER4, POWER7, POWER8 and POWER9. It gets a lot of exercise.

Method (5) is testing better than the English-language only version we are using from Master because there are no language dependencies. Method (5) outperforms the past version that relied on toolchain versions.

The new feature test version is also very robust. It only accepts clean compiles, and it always rejects problems. It does not matter what combinations of toolchain and hardware we try to use. And it requires less work that the current version in Master because BAD_RESULT required us to prime (and maintain) the system with strings:

BAD_RESULT="fatal|error|unknown|unrecognized|unexpected|illegal|ignored|incorrect|
  not found|not exist|cannot find|not supported|not compatible|no such instruction|invalid mnemonic"

@ernsteiswuerfel
Copy link

ernsteiswuerfel commented Nov 19, 2018

Re-built cryptopp on my G5 from master which went fine. Now ./cryptest.exe errors out at:

SIMON validation suite running...

Testing SymmetricCipher algorithm SIMON-64/ECB.
Ungültiger Maschinenbefehl (Speicherabzug geschrieben)

@noloader
Copy link
Collaborator Author

noloader commented Nov 20, 2018

./cryptest.exe errors out at:

SIMON validation suite running...

Testing SymmetricCipher algorithm SIMON-64/ECB.
Ungültiger Maschinenbefehl (Speicherabzug geschrieben)

I've seen this before on Power7/Power8 but I don't have a fix for it. The problem is this...

void SIMON64_Encrypt(...)
{
#if CRYPTOPP_POWER7_AVAILABLE
    if (HasPower7())
    {
        SIMON64_Encrypt_POWER7(...);
        return;
    }
#endif
    SIMON64_Encrypt_CXX(...);
}

The compiler will optimize things a bit and a POWER7 instruction will percolate up into SIMON64_Encrypt near the HasPower7() call. When I observed it on POWER7, the frame's prologue had a POWER8 mtfprwz, which was being used to preserve callee registers.

The only workaround I know of is to manually add -DCRYPTOPP_DISABLE_POWER7 to your CXXFLAGS. So maybe something like:

$ make distclean
$ CXXFLAGS="-DNDEBUG -DCRYPTOPP_DISABLE_POWER7 -g2 -O3" make -j 4
...
$ ./cryptest.exe v
...

noloader added a commit that referenced this issue Nov 21, 2018
This will allow users to specify agreesive warning flags without accidentally failing a feature test. The feature tests are minimal but the system headers could be noisy under elevated warnings
@noloader
Copy link
Collaborator Author

@mouse07410,

I think we are ready to cut this over for i686/x86_64 in Master and ask for testers. Are you OK with it?

@mouse07410
Copy link
Collaborator

mouse07410 commented Nov 22, 2018

Could you perhaps put it on a separate branch for now? I'd like to pull that branch and try it on MacOS and a couple of Linux systems.

Also, I'd like you to take a look at GNUmakefile in my fork: I found it necessary for Macports GCC to add all the assembly flags, and disable the unavailable instructions via defines like -DCRYPTOPP_DISABLE_SSE2 and such. The current master was not detecting features properly for me.

@ernsteiswuerfel
Copy link

Built git-master from today with the CXXFLAGS you suggested.

Now the tests error out at:
Testing MessageDigest algorithm SM3.
..............................................................
Tests complete. Total tests = 62. Failed tests = 0.

BLAKE2s validation suite running...

passed algorithm name
Ungültiger Maschinenbefehl (Speicherabzug geschrieben)

noloader added a commit to noloader/cryptopp-cmake that referenced this issue Nov 28, 2018
@noloader
Copy link
Collaborator Author

noloader commented Dec 1, 2018

@ernsteiswuerfel,

We changed the Altivec flag to use only -maltivec, instead of -mcpu=power4 -maltivec. If that's not it, then I am out of ideas. We will need to see a disassembly around where the SIGILL occurs. We will also need to see the processor specs.

@noloader
Copy link
Collaborator Author

noloader commented Dec 1, 2018

The Makefile has been cut-over. Closing this issue.

@noloader noloader closed this as completed Dec 1, 2018
noloader added a commit to noloader/cryptopp-autotools that referenced this issue Jan 18, 2019
These tests suffered the same language problems as weidai11/cryptopp#741
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants