search

KhronosGroup / Vulkan-ValidationLayers

Vulkan Validation Layers (VVL)
https://vulkan.lunarg.com/doc/sdk/latest/linux/khronos_validation_layer.html
Other
757 stars 403 forks source link

build failure #3367

Closed zmike closed 3 years ago

zmike commented 3 years ago

When trying to use update_deps.py in a release build, I get the following error from trying to build spirv-tools:

/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp: In function ‘spv_result_t spvtools::{anonymous}::getWord(spv_text, spv_position, std::string*)’:
/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp:122:11: error: this statement may fall through [-Werror=implicit-fallthrough=]
  122 |           if (escaping || quoting) break;
      |           ^~
/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp:124:9: note: here
  124 |         case '\0': {  // NOTE: End of word found!
      |         ^~~~
cc1plus: all warnings being treated as errors
gmake[2]: *** [source/CMakeFiles/SPIRV-Tools-shared.dir/build.make:432: source/CMakeFiles/SPIRV-Tools-shared.dir/text_handler.cpp.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
[ 23%] Building CXX object source/CMakeFiles/SPIRV-Tools-static.dir/val/validate_atomics.cpp.o
[ 23%] Building CXX object source/CMakeFiles/SPIRV-Tools-static.dir/val/validate_barriers.cpp.o
/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp: In function ‘spv_result_t spvtools::{anonymous}::getWord(spv_text, spv_position, std::string*)’:
/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp:122:11: error: this statement may fall through [-Werror=implicit-fallthrough=]
  122 |           if (escaping || quoting) break;
      |           ^~
/home/zmike/src/Vulkan-ValidationLayers/build/SPIRV-Tools/source/text_handler.cpp:124:9: note: here
  124 |         case '\0': {  // NOTE: End of word found!
      |         ^~~~
cc1plus: all warnings being treated as errors

I guess this means either not using -Werror for this subproject or resolving the warnings?

ncesario-lunarg commented 3 years ago

@zmike I'm not seeing this with the following:

cmake -Bbuild -GNinja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=ON -DUPDATE_DEPS=ON
ninja -Cbuild

cmake version: 3.21.2 gcc/g++ version: 11.1.0.

Can you provide the build invocation you used?

ncesario-lunarg commented 3 years ago

NOTE: you will probably want to remove/clean your external directory when switching between debug and release builds.

zmike commented 3 years ago

cmake 3.20.5 gcc 11.2.1

I tested your cmake command with a completely clean checkout and got this:

$ cmake -Bbuild -GNinja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=ON -DUPDATE_DEPS=ON                           (~/src/Vulkan-ValidationLayers)
-- The C compiler identification is GNU 11.2.1
-- The CXX compiler identification is GNU 11.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/lib64/ccache/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib64/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PythonInterp: /usr/bin/python3 (found suitable version "3.9.6", minimum required is "3") 
CMake Warning at CMakeLists.txt:58 (message):
  CMAKE_GENERATOR_PLATFORM not set.  Using x64 as target architecture.

********************************************************************************
* NOTE: Adding target vvl_update_deps to run as needed for updating            *
*       dependencies.                                                          *
********************************************************************************
CMake Error: Error: generator : Ninja
Does not match the generator used previously: Unix Makefiles
Either remove the CMakeCache.txt file and CMakeFiles directory or choose a different binary directory.
Starting builds in /home/zmike/src/Vulkan-ValidationLayers/external
Checking out glslang in /home/zmike/src/Vulkan-ValidationLayers/external/glslang
b'HEAD detached at 2fb89a00\nnothing to commit, working tree clean\n'
Building glslang in /home/zmike/src/Vulkan-ValidationLayers/external/glslang
Build dir = /home/zmike/src/Vulkan-ValidationLayers/external/glslang/build
Install dir = /home/zmike/src/Vulkan-ValidationLayers/external/glslang/build/install

CMake Error at CMakeLists.txt:95 (message):
  Could not run update_deps.py which is necessary to download dependencies.

-- Configuring incomplete, errors occurred!
See also "/home/zmike/src/Vulkan-ValidationLayers/build/CMakeFiles/CMakeOutput.log".

If I try to run ../scripts/update_deps.py --config release from my build directory, I get the errors from the original post.

zmike commented 3 years ago

Argh, okay, git clean ignored external/, which explains that, at least...

ncesario-lunarg commented 3 years ago

Does not match the generator used previously: Unix Makefiles

Looks like external is not getting cleaned. I think you can safely delete the external directory before re-generating (and Ninja is not necessary, just copy/pasted).

zmike commented 3 years ago

I use ninja as well, I guess this is just different because update_deps uses make?

Anyway, with cmake -Bbuild -GNinja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=ON -DUPDATE_DEPS=ON I now get the same exact error as in the OP:

FAILED: source/CMakeFiles/SPIRV-Tools-shared.dir/text_handler.cpp.o 
/usr/lib64/ccache/c++ -DSPIRV_CHECK_CONTEXT -DSPIRV_COLOR_TERMINAL -DSPIRV_LINUX -DSPIRV_TIMER_ENABLED -DSPIRV_TOOLS_IMPLEMENTATION -DSPIRV_TOOLS_SHAREDLIB -DSPIRV_Tools_shared_EXPORTS -I../ -I../include -I. -I../../SPIRV-Headers/include -O3 -DNDEBUG -fPIC -fvisibility=hidden -Wall -Wextra -Wnon-virtual-dtor -Wno-missing-field-initializers -Werror -std=c++11 -fno-exceptions -Wno-long-long -Wshadow -Wundef -Wconversion -Wno-sign-conversion -std=gnu++11 -MD -MT source/CMakeFiles/SPIRV-Tools-shared.dir/text_handler.cpp.o -MF source/CMakeFiles/SPIRV-Tools-shared.dir/text_handler.cpp.o.d -o source/CMakeFiles/SPIRV-Tools-shared.dir/text_handler.cpp.o -c ../source/text_handler.cpp
../source/text_handler.cpp: In function ‘spv_result_t spvtools::{anonymous}::getWord(spv_text, spv_position, std::string*)’:
../source/text_handler.cpp:122:11: error: this statement may fall through [-Werror=implicit-fallthrough=]
  122 |           if (escaping || quoting) break;
      |           ^~
../source/text_handler.cpp:124:9: note: here
  124 |         case '\0': {  // NOTE: End of word found!
      |         ^~~~
cc1plus: all warnings being treated as errors
[46/319] Building CXX object source/CMakeFiles/SPIRV-Tools-shared.dir/val/validate_atomics.cpp.o
[47/319] Building CXX object source/CMakeFiles/SPIRV-Tools-shared.dir/val/validate_arithmetics.cpp.o
[48/319] Building CXX object source/CMakeFiles/SPIRV-Tools-shared.dir/val/validate_annotation.cpp.o
[49/319] Building CXX object source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o
FAILED: source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o 
/usr/lib64/ccache/c++ -DSPIRV_CHECK_CONTEXT -DSPIRV_COLOR_TERMINAL -DSPIRV_LINUX -DSPIRV_TIMER_ENABLED -I../ -I../include -I. -I../../SPIRV-Headers/include -O3 -DNDEBUG -fPIC -fno-rtti -Wall -Wextra -Wnon-virtual-dtor -Wno-missing-field-initializers -Werror -std=c++11 -fno-exceptions -Wno-long-long -Wshadow -Wundef -Wconversion -Wno-sign-conversion -std=gnu++11 -MD -MT source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o -MF source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o.d -o source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o -c ../source/text_handler.cpp
../source/text_handler.cpp: In function ‘spv_result_t spvtools::{anonymous}::getWord(spv_text, spv_position, std::string*)’:
../source/text_handler.cpp:122:11: error: this statement may fall through [-Werror=implicit-fallthrough=]
  122 |           if (escaping || quoting) break;
      |           ^~
../source/text_handler.cpp:124:9: note: here
  124 |         case '\0': {  // NOTE: End of word found!
      |         ^~~~
cc1plus: all warnings being treated as errors
dneto0 commented 3 years ago

There's already a comment there saying the fallthrough is intentional. See https://github.com/KhronosGroup/SPIRV-Tools/blob/ba4b390c367e09edaad36f0376a87aa03620fd4b/source/text_handler.cpp#L123 

        case '\r':
          if (escaping || quoting) break;
        // Fall through.
        case '\0': {  // NOTE: End of word found!

But it doesn't match the patterns GCC is looking for: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough

I can fix this upstream in SPIRV-Tools.
In the meantime, you can disable implicit fallthrough checking specifically.

ncesario-lunarg commented 3 years ago

I'm still confused as to why I can't reproduce 11.2.1 in the same environment as @zmike.

dneto0 commented 3 years ago

I'm still confused as to why I can't reproduce 11.2.1 in the same environment as @zmike.

Indeed. I'm trying to reproduce, and haven't yet.

I have GCC 11.2.0 (and cmake 3.18.4, but I hope that doesn't matter).

dneto0 commented 3 years ago

I've tried:

cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc-11 -DCMAKE_CXX_COMPILER=g++-11 ../shaderc -DCMAKE_CXX_FLAGS=-Wimplicit-fallthrough=3

-- The C compiler identification is GNU 11.2.0 -- The CXX compiler identification is GNU 11.2.0 -- Detecting C compiler ABI info ...

and I confirmed in the build.ninja file that the flags setting took effect:

build third_party/spirv-tools/source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o: CXX_COMPILER__SPIRV-Tools-static_Release /build/shaderc/shaderc/third_party/spirv-tools/source/text_handler.cpp || cmake_object_order_depends_target_SPIRV-Tools-static DEFINES = -DSPIRV_COLOR_TERMINAL -DSPIRV_LINUX -DSPIRV_TIMER_ENABLED DEP_FILE = third_party/spirv-tools/source/CMakeFiles/SPIRV-Tools-static.dir/text_handler.cpp.o.d FLAGS = -Wimplicit-fallthrough=3 -Wimplicit-fallthrough -O3 -DNDEBUG -fPIC -Wextra-semi -w -fno-rtti -Wall -Wextra -Wnon-virtual-dtor -Wno-missing-field-initializers -Werror -std=c++11 -fno-exceptions -Wno-long-long -Wshadow -Wundef -Wconversion -Wno-sign-conversion -std=gnu++11

ncesario-lunarg commented 3 years ago

I get similar results with (gcc 11.1.0, cmake 3.21.2) and (gcc 11.2.1, cmake 3.20.5).

In any case, thanks for the quick upstream fix @dneto0!

zmike commented 3 years ago

Sometimes compilers be like that. Seems like it's fixed upstream, so probably this can be closed once the VVL dependency version is bumped?

ncesario-lunarg commented 3 years ago

@zmike SPIRV-Tools should be updated with the compiler fix on master now.