package/opencv3: fix CMAKE_CXX_FLAGS

The commit 4904c4c "package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC" overrides CMAKE_CXX_FLAGS with the single -latomic, losing all ABI CFLAGS that are passed there by default. This breaks build on xtensa where ABI CFLAGS contain important code generation options.

Append $(TARGET_CXXFLAGS) to CMAKE_CXX_FLAGS along with -latomic.

Fixes:
http://autobuild.buildroot.net/results/7f1c96abd8fbb5b358a07100ab623316e9bb9dcd
http://autobuild.buildroot.net/results/e0c93d0f6d1da0d62d4dbba211a275bfe75e9645
http://autobuild.buildroot.net/results/53e7e4b4b6a7b48b8012799d7507f7594dbf01b2

Signed-off-by Max Filippov <[email protected]>

---
 package/opencv3/opencv3.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/opencv3/opencv3.mk +0 -0

index 2529de9..10660a9 100644

Adding Samuel in Cc: there is a CMake issue below.

Quoted 22 lines · Max Filippov

The commit 4904c4c "package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC" overrides CMAKE_CXX_FLAGS with the single -latomic, losing all ABI CFLAGS that are passed there by default. This breaks build on xtensa where ABI CFLAGS contain important code generation options.

Append $(TARGET_CXXFLAGS) to CMAKE_CXX_FLAGS along with -latomic.

Fixes:
http://autobuild.buildroot.net/results/7f1c96abd8fbb5b358a07100ab623316e9bb9dcd
http://autobuild.buildroot.net/results/e0c93d0f6d1da0d62d4dbba211a275bfe75e9645
http://autobuild.buildroot.net/results/53e7e4b4b6a7b48b8012799d7507f7594dbf01b2

Signed-off-by Max Filippov <[email protected]>

---
 package/opencv3/opencv3.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/opencv3/opencv3.mk +0 -0

index 2529de9..10660a9 100644

package/opencv3/opencv3.mk OPENCV3_LICENSE_FILES = LICENSE · line 15

Quoted 5 lines

package/opencv3/opencv3.mk +1 -1

@@ -12,7 +12,7 @@ OPENCV3_LICENSE_FILES = LICENSE
 
 # Uses __atomic_fetch_add_4
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
-OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic"
+OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"

There is something fishy going on here. Indeed,
support/misc/toolchainfile.cmake.in contains:

  set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ?

Quoted 9 lines

There is something fishy going on here. Indeed,
support/misc/toolchainfile.cmake.in contains:

  set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ?

Yes, it does.

opencv2 which has similar cmake build system but doesn't pass -DCMAKE_CXX_FLAGS builds correctly.

Quoted 10 lines · Max Filippov

Quoted 9 lines

There is something fishy going on here. Indeed,
support/misc/toolchainfile.cmake.in contains:

  set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ?

Quoted 1 line

Yes, it does.

So if it does, why is your patch necessary? I'm not following here. Do you mean that the Xtensa-specific flags are present in the toolchain.cmake file, but are in the end not used during the build ?

Quoted 11 lines · Thomas Petazzoni

Quoted 10 lines · Max Filippov

Quoted 9 lines

There is something fishy going on here. Indeed,
support/misc/toolchainfile.cmake.in contains:

  set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ?

Quoted 5 lines

Quoted 1 line

Yes, it does.

So if it does, why is your patch necessary? I'm not following here. Do you mean that the Xtensa-specific flags are present in the toolchain.cmake file, but are in the end not used during the build ?

Right. The file
host/usr/share/buildroot/toolchainfile.cmake
exists and contains the following (correct) lines:

set(CMAKE_C_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS") set(CMAKE_CXX_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

But the file
build/opencv3-3.1.0/CMakeCache.txt
that governs opencv3 build contains the following lines:

//Buildroot CXXFLAGS
CMAKE_CXX_FLAGS:STRING=-latomic
//Buildroot CFLAGS CMAKE_C_FLAGS:STRING='-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os '

Below is the output that I see during the failing opencv3 build:

-- C++ flags (Release): -latomic -fsigned-char -W -Wall

(quoted diff) +0 -15

-Werror=return-type -Werror=non-virtual-dtor -Werror=address
-Werror=sequence-point -Wformat -Werror=format-security
-Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow
-Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor
-fdiagnostics-show-option -pthread -fomit-frame-pointer
-ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -DNDEBUG  -DNDEBUG
--     C++ flags (Debug):           -latomic   -fsigned-char -W -Wall
-Werror=return-type -Werror=non-virtual-dtor -Werror=address
-Werror=sequence-point -Wformat -Werror=format-security
-Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow
-Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor
-fdiagnostics-show-option -pthread -fomit-frame-pointer
-ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -g
 -O0 -DDEBUG -D_DEBUG
--     C Compiler:

/home/jcmvbkbc/tmp/br/build-20160928-reproduce-opencv/host/usr/bin/xtensa-linux-gcc
-- C flags (Release): -D_LARGEFILE_SOURCE

(quoted diff) +0 -17

-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls
-mauto-litpools -Os    -fsigned-char -W -Wall -Werror=return-type
-Werror=non-virtual-dtor -Werror=address -Werror=sequence-point
-Wformat -Werror=format-security -Wmissing-declarations
-Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self
-Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option
-pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden
-O3 -DNDEBUG  -DNDEBUG
--     C flags (Debug):             -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls
-mauto-litpools -Os    -fsigned-char -W -Wall -Werror=return-type
-Werror=non-virtual-dtor -Werror=address -Werror=sequence-point
-Wformat -Werror=format-security -Wmissing-declarations
-Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self
-Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option
-pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden
-g  -O0 -DDEBUG -D_DEBUG

Notice the difference between the C and the C++ flags.

Hi all,

Quoted 79 lines · Max Filippov

Quoted 11 lines · Thomas Petazzoni

Quoted 10 lines · Max Filippov

Quoted 9 lines

There is something fishy going on here. Indeed,
support/misc/toolchainfile.cmake.in contains:

  set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ?

Quoted 5 lines

Quoted 1 line

Yes, it does.

So if it does, why is your patch necessary? I'm not following here. Do you mean that the Xtensa-specific flags are present in the toolchain.cmake file, but are in the end not used during the build ?

Right. The file
host/usr/share/buildroot/toolchainfile.cmake
exists and contains the following (correct) lines:

set(CMAKE_C_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS") set(CMAKE_CXX_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")

But the file
build/opencv3-3.1.0/CMakeCache.txt
that governs opencv3 build contains the following lines:

//Buildroot CXXFLAGS
CMAKE_CXX_FLAGS:STRING=-latomic
//Buildroot CFLAGS CMAKE_C_FLAGS:STRING='-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os '

Below is the output that I see during the failing opencv3 build:

-- C++ flags (Release): -latomic -fsigned-char -W -Wall

(quoted diff) +0 -15

-Werror=return-type -Werror=non-virtual-dtor -Werror=address
-Werror=sequence-point -Wformat -Werror=format-security
-Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow
-Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor
-fdiagnostics-show-option -pthread -fomit-frame-pointer
-ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -DNDEBUG  -DNDEBUG
--     C++ flags (Debug):           -latomic   -fsigned-char -W -Wall
-Werror=return-type -Werror=non-virtual-dtor -Werror=address
-Werror=sequence-point -Wformat -Werror=format-security
-Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow
-Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor
-fdiagnostics-show-option -pthread -fomit-frame-pointer
-ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -g
 -O0 -DDEBUG -D_DEBUG
--     C Compiler:

/home/jcmvbkbc/tmp/br/build-20160928-reproduce-opencv/host/usr/bin/xtensa-linux-gcc
-- C flags (Release): -D_LARGEFILE_SOURCE

(quoted diff) +0 -17

-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls
-mauto-litpools -Os    -fsigned-char -W -Wall -Werror=return-type
-Werror=non-virtual-dtor -Werror=address -Werror=sequence-point
-Wformat -Werror=format-security -Wmissing-declarations
-Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self
-Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option
-pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden
-O3 -DNDEBUG  -DNDEBUG
--     C flags (Debug):             -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls
-mauto-litpools -Os    -fsigned-char -W -Wall -Werror=return-type
-Werror=non-virtual-dtor -Werror=address -Werror=sequence-point
-Wformat -Werror=format-security -Wmissing-declarations
-Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self
-Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option
-pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden
-g  -O0 -DDEBUG -D_DEBUG

Quoted 1 line

Notice the difference between the C and the C++ flags.

Indeed, there is a bug in the way we are currently setting/expending {C,CXX}FLAGS in the CMake toolchain file.
I'll post a patch fixing it quickly.

Regards,