Build fails on fedora30+ (CPU features miss-detected) #812

kwizart · 2019-02-26T17:12:18Z

Using Fedora 30, I cannot update to cryptopp 8.0.0/8.1.0
Seems like CPU features are miss-detected as I cannot see any ASM_FLAGS been used in "some" cases.

Build attempt for f31,f29, f28
https://koji.fedoraproject.org/koji/taskinfo?taskID=33066081
https://koji.fedoraproject.org/koji/taskinfo?taskID=33066857
https://koji.fedoraproject.org/koji/taskinfo?taskID=33067570

I'm using the default GNUMake files to build using:
%make_build -f GNUmakefile
CXXFLAGS="-DNDEBUG %{optflags} -fPIC -DPIC"
LDFLAGS="%{?__global_ldflags}"
shared cryptest.exe

noloader · 2019-02-26T21:35:58Z

Thanks @kwizart.

Interesting in a morbid sort of way... I use Fedora 29 as my primary workstation, and we specifically test using Fedora's build flags.

The symptom is the following taken from the task 33066120 build.log:

g++ -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC -DPIC -c aria_simd.cpp
BUILDSTDERR: In file included from aria_simd.cpp:14:
BUILDSTDERR: /usr/lib/gcc/x86_64-redhat-linux/9/include/tmmintrin.h: In function 'void CryptoPP::ARIA_ProcessAndXorBlock_SSSE3(const byte*, CryptoPP::byte*, const byte*, CryptoPP::word32*)':
BUILDSTDERR: /usr/lib/gcc/x86_64-redhat-linux/9/include/tmmintrin.h:136:1: error: inlining failed in call to always_inline '__m128i _mm_shuffle_epi8(__m128i, __m128i)': target specific option mismatch
BUILDSTDERR:   136 | _mm_shuffle_epi8 (__m128i __X, __m128i __Y)
BUILDSTDERR:       | ^~~~~~~~~~~~~~~~

The makefile should be adding -mssse3 for aria_simd.cpp and other cpu-specific flags for other *_simd.cpp files. For example rijndael_simd.cpp gets -msse4.1 -maes on x86 machines.

But I can't seem to reproduce it using Fedora flags:

skylake:cryptopp$ export CXXFLAGS=" -DNDEBUG -O2 -g -pipe -Wall -Werror=format-s
ecurity -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-p
rotector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened
-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchr
onous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC -DPIC"

skylake:cryptopp$ g++ $CXXFLAGS -mssse3 TestPrograms/test_x86_ssse3.cxx
skylake:cryptopp$

The g++ $CXXFLAGS -mssse3 TestPrograms/test_x86_ssse3.cxx is the manual version of what the makefile does. The GNUmakefile does it at line 265.

There's something unusual here, though. From the build log the order is a tad bit incorrect:

g++ -DNDEBUG -O2 -g ... -fPIC -DPIC -c algebra.cpp
g++ -DNDEBUG -O2 -g ... -fPIC -DPIC -c cpu.cpp
g++ -DNDEBUG -O2 -g ... -fPIC -DPIC -c adler32.cpp

The Crypto++ makefile builds in a specific order due to Static Initialization Order Fiasco:

cryptopp$ make
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c cryptlib.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c cpu.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c integer.cpp
<remainder in alphabetical order for deterministic build>
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c 3way.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c adler32.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c algebra.cpp
...

Where can I find the contents of the GNUmakefile you are using?

The makefile tries to pre-qualify NEON (for lack of a better term), and sets IS_NEON accordingly. If IS_NEON=1, then we go on to perform test compiles to see if -mfloat-abi=X -mfpu=neon (and friends) actually work. Effectively we are performing a test to see if we should perform another test. The IS_NEON flag predates our compile time feature tests. It was kind of helpful when we were trying to sort out if a platform and compiler options supported NEON without a compile test. That was an absolute mess and we quickly learned we needed a real compile time feature test (which we now have). Additionally, Debian and Fedora ARMEL builds are failing because we are misdetecting NEON availability. It looks like we fail to set IS_NEON properly, so we never get into the code paths that set either (1) -mfloat-abi=X -mfpu=neon or (2) -DCRYPTOPP_DISABLE_NEON or -DCRYPTOPP_DISABLE_ASM. Later, the makefile builds a *_simd.cpp and the result is an error that NEON needs to be activated (or disabled). This commit removes IS_NEON so we immediately move to compile time feature tests.

noloader · 2019-02-28T00:38:07Z

I believe an underlying ARMEL issue was cleared at Commit 84ab1f3c66a5.

But I am not sure if it will help Fedora becuase it appears a different makefile is being used.

kwizart · 2019-02-28T08:20:45Z

Seems most arches are now fixed but aarch64.
https://koji.fedoraproject.org/koji/taskinfo?taskID=33100404

About https://www.cryptopp.com/wiki/Static_Initialization_Order_Fiasco
Does it means I need to build with -j1 ?

noloader · 2019-02-28T09:18:32Z

Seems most arches are now fixed but aarch64.
https://koji.fedoraproject.org/koji/taskinfo?taskID=33100404

From the 33100407 build log, it looks like we have another mis-detection. The first is actually aria_simd.cpp again though it does not cause a compile failure:

...
g++ -DNDEBUG -O2 -g -pipe ... -fPIC -DPIC -c aria.cpp
g++ -DNDEBUG -O2 -g -pipe ... -fPIC -DPIC -c aria_simd.cpp
g++ -DNDEBUG -O2 -g -pipe ... -fPIC -DPIC -c ariatab.cpp

Here's what it looks like when working as expected. Notice the addition of -march=armv8-a:

...
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c aria.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -march=armv8-a -c aria_simd.cpp
g++ -DNDEBUG -g2 -O3 -fPIC -pthread -pipe -c ariatab.cpp

So it looks like we are not setting IS_ARMV8 correctly in the GNUmakefile for Fedora:

HOSTX := $(shell $(CXX) $(CXXFLAGS) -dumpmachine 2>/dev/null | cut -f 1 -d '-')
...
IS_ARMV8 := $(shell echo "$(HOSTX)" | $(GREP) -i -c -E 'aarch32|aarch64|arm64|armv8')

Later we use it to add the appropriate flags in GNUmakefile. Something like this:

ifneq ($(IS_ARM32)$(IS_ARMV8),00)
    ...
    ifneq ($(IS_ARMV8),0)
        # Do ARMv8 tests
        TPROG = TestPrograms/test_arm_asimd.cxx
        TOPT = -march=armv8-a
        HAVE_OPT = $(shell $(CXX) $(CXXFLAGS) $(ACLE_FLAG) $(ZOPT) $(TOPT) $(TPROG) -o $(TOUT) 2>&1 | tr ' ' '\n' | wc -l)
        ifeq ($(strip $(HAVE_OPT)),0)
            ASIMD_FLAG = -march=armv8-a
            ARIA_FLAG = -march=armv8-a
            ...
        endif
    endif
endif

And I cannot duplicate it on two dev-boards, gcc117 or gcc118 from the compile farm. But I don't believe any of the machines are running Fedora.

What does g++ -dumpmachine output on the Fedora build machine?

About https://www.cryptopp.com/wiki/Static_Initialization_Order_Fiasco
Does it means I need to build with -j1 ?

No. It means we (Crypto++) needs to build and link the objects in a specific order to avoid initialization problems.

kwizart · 2019-02-28T09:58:27Z

What does g++ -dumpmachine output on the Fedora build machine ?

$ c++ -dumpmachine
aarch64-redhat-linux

I'm testing with export CXXFLAGS instead of passing flags with make -f GNUMake CXXFLAGS="-DNDEBUG %{optflags}"

For aarch64 I now have -DCRYPTOPP_DISABLE_ASM set (so the build succeed). But for others arches, everything is built with the appropropriate ASM when relevant.

noloader · 2019-02-28T10:12:37Z

I'm testing with export CXXFLAGS instead of passing flags with make -f GNUmakefile CXXFLAGS="-DNDEBUG %{optflags}"

Yeah, you usually don't want this when running make:

make CXXFLAGS="..."

That says, don't use project CXXFLAGS; use our flags instead. It effectively makes CXXFLAGS read only for the makefile. When GNU make encounters it it will ignore things like:

CXXFLAGS += "<some project flag>"

Usually you want this:

 CXXFLAGS="..." make

That allows us to add to your CXXFLAGS, and this like this will work as expected:

CXXFLAGS += "<some project flag>"

So I think you should use:

CXXFLAGS="-DNDEBUG %{optflags}" make -f GNUmakefile

You actually tickled a bug in our Makefile. I've known about it for several years, but I never addressed it. The problem is, we don't build correctly when make is invoked like below because make overrides the flags we would add.

make CXXFLAGS="..."

Here's the reason it happens. It is our default recipe:

# We determine we need some flag, but CXXFLAGS is
#   read-only due to 'make CXXFLAGS="..."'
CXXFLAGS += <some flag>
...

%.o : %.cpp
    $(CXX) $(strip $(CXXFLAGS) -c) $<

This is what we should be doing. It is textbook Stallman from the GNU Make manual:

# We determine we need some flag
OUR_CXXFLAGS += <some flag>
...

%.o : %.cpp
    $(CXX) $(strip $(OUR_CXXFLAGS) $(CXXFLAGS) -c) $<

Now it does not matter how you invoke make. Things just work for you. And it follows Stallman's freedom philosophy. The user's CXXFLAGS always takes precedence over OUR_CXXFLAGS.

Since someone tickled it in the field I am going to get it fixed.

kwizart · 2019-02-28T10:23:46Z

Okay I think I got it:

c++ ${CXXFLAGS} -DCRYPTOPP_ARM_ACLE_AVAILABLE=1  -march=armv8-a -O0 -o TestPrograms/test_arm_asimd TestPrograms/test_arm_asimd.cxx
In file included from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdint.h:26,
                 from /usr/lib/gcc/aarch64-redhat-linux/8/include/stdint.h:9,
                 from /usr/lib/gcc/aarch64-redhat-linux/8/include/arm_neon.h:33,
                 from TestPrograms/test_arm_asimd.cxx:1:
/usr/include/features.h:381:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp]
 #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)

I'm testing with using ZOPT='' instead because it overrides our default CXXFLAGS in an incompatible way.
But is it relevant to keep it ?

noloader · 2019-02-28T10:41:00Z

I'm testing with using ZOPT='' instead because it overrides our default CXXFLAGS in an incompatible way.

But is it relevant to keep it ?

For testing features only, we remove -Wp,-D_FORTIFY_SOURCE=2 from CXXFLAGS. That's because of ZOPT=-O0 and the error you see. ZOPT=-O0 ensures the optimizer does not remove code during the feature test. We also remove some others during feature testing, like -Werror. You can see it here around line 115 in the Makefile:

# Strip out -Wall, -Wextra and friends for feature testing
ifeq ($(DETECT_FEATURES),1)
  TCXXFLAGS := $(filter-out -Wall -Wextra -Werror% -Wunused -Wconversion -Wp%, $(CXXFLAGS))
  ifneq ($(strip $(TCXXFLAGS)),)
    $(info Using testing flags: $(TCXXFLAGS))
  endif
  #TPROG = TestPrograms/test_cxx.cxx
  #$(info Testing compile... )
  #$(info $(shell $(CXX) $(TCXXFLAGS) $(ZOPT) $(TOPT) $(TPROG) -o $(TOUT) 1>/dev/null))
endif

We use TCXXFLAGS (test CXXFLAGS) to test for options like -march=armv8-a.

The reason we remove some options during feature testing is, we use a clean compile as the gate. If there is any noise then we fail the feature. This is needed for some compilers like SunCC on Solaris and IBM XL C/C++ on AIX. Those compilers don't return 1 or failure. Instead they report an error to stdout or stderr and return 0 or success (derp...).

When we build the library (as opposed to a feature test) we use your full CXXFLAGS with nothing removed.

As done with others tests. This will avoid a miss-detection of aarch64 features when using flags such as _FORTIFY_SOURCE that needs to be filtered for testing This fixes weidai11#812 Signed-off-by: Nicolas Chauvet <kwizart@gmail.com>

kwizart · 2019-02-28T11:21:45Z

Ok, using ZOPT='' cleared the issue, the build are optimized (even on aarch64).

Okay I've understood the idea about the test flags. I've found the root cause.
(PR in a few).

As done with others tests. This will avoid a miss-detection of aarch64 features when using flags such as _FORTIFY_SOURCE that needs to be filtered for testing This fixes weidai11#812 V2: Fix all cases Signed-off-by: Nicolas Chauvet <kwizart@gmail.com>

As done with others tests. This will avoid a miss-detection of aarch64 features when using flags such as _FORTIFY_SOURCE that needs to be filtered for testing This fixes #812 V2: Fix all cases Signed-off-by: Nicolas Chauvet <kwizart@gmail.com>

noloader · 2019-02-28T11:38:43Z

Thanks @kwizart.

Man, was I wrong about this:

We use TCXXFLAGS (test CXXFLAGS) to test for options like -march=armv8-a.

Sorry about that.

noloader · 2019-02-28T11:44:33Z

@kwizart,

Here is a related commit for the PR you just made: Commit 08b9e21e5a43. I don't believe you need it since it applies to GNUmakefile-cross, but I mention it for completeness.

noloader mentioned this issue Feb 28, 2019

GNUmakefile needs to use separate variable for CXXFLAGS #814

Closed

kwizart mentioned this issue Feb 28, 2019

Use TCXXFLAGS instead of CXXFLAGS #815

Merged

noloader closed this as completed in #815 Feb 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build fails on fedora30+ (CPU features miss-detected) #812

Build fails on fedora30+ (CPU features miss-detected) #812

kwizart commented Feb 26, 2019

noloader commented Feb 26, 2019 •

edited

Loading

noloader commented Feb 28, 2019

kwizart commented Feb 28, 2019

noloader commented Feb 28, 2019 •

edited

Loading

kwizart commented Feb 28, 2019 •

edited by noloader

Loading

noloader commented Feb 28, 2019 •

edited

Loading

kwizart commented Feb 28, 2019 •

edited

Loading

noloader commented Feb 28, 2019 •

edited

Loading

kwizart commented Feb 28, 2019

noloader commented Feb 28, 2019

noloader commented Feb 28, 2019 •

edited

Loading

Build fails on fedora30+ (CPU features miss-detected) #812

Build fails on fedora30+ (CPU features miss-detected) #812

Comments

kwizart commented Feb 26, 2019

noloader commented Feb 26, 2019 • edited Loading

noloader commented Feb 28, 2019

kwizart commented Feb 28, 2019

noloader commented Feb 28, 2019 • edited Loading

kwizart commented Feb 28, 2019 • edited by noloader Loading

noloader commented Feb 28, 2019 • edited Loading

kwizart commented Feb 28, 2019 • edited Loading

noloader commented Feb 28, 2019 • edited Loading

kwizart commented Feb 28, 2019

noloader commented Feb 28, 2019

noloader commented Feb 28, 2019 • edited Loading

noloader commented Feb 26, 2019 •

edited

Loading

noloader commented Feb 28, 2019 •

edited

Loading

kwizart commented Feb 28, 2019 •

edited by noloader

Loading

noloader commented Feb 28, 2019 •

edited

Loading

kwizart commented Feb 28, 2019 •

edited

Loading

noloader commented Feb 28, 2019 •

edited

Loading

noloader commented Feb 28, 2019 •

edited

Loading