intel / linux-sgx

Intel SGX for Linux*
https://www.intel.com/content/www/us/en/developer/tools/software-guard-extensions/linux-overview.html
Other
1.33k stars 546 forks source link

Don't build with -Werror #382

Open thiagomacieira opened 5 years ago

thiagomacieira commented 5 years ago

Unless you're testing with ALL compiler versions, including the unreleased ones. You're not testing the GCC version released one year ago.

gcc -g -O3 -feliminate-unused-debug-types  -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=32 -Wformat -Wformat-security -Wl,--copy-dt-needed-entries -m64  -fasynchronous-unwind-tables -Wp,-D_REENTRANT -ftree-loop-distribute-patterns -Wl,-z -Wl,now -Wl,-z -Wl,relro -fno-semantic-interposition -ffat-lto-objects  -fno-signed-zeros -fno-trapping-math  -fassociative-math -Wl,-sort-common -Wl,--enable-new-dtags -mtune=skylake -Wjump-misses-init -Wstrict-prototypes -Wunsuffixed-float-constants -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -UDEBUG -DNDEBUG -ffunction-sections -fdata-sections -Wall -Wextra -Winit-self -Wpointer-arith -Wreturn-type -Waddress -Wsequence-point -Wformat-security -Wmissing-include-dirs -Wfloat-equal -Wundef -Wshadow -Wcast-align -Wconversion -Wredundant-decls -DITT_ARCH_IA64 -Wjump-misses-init -Wstrict-prototypes -Wunsuffixed-float-constants -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -UDEBUG -DNDEBUG -ffunction-sections -fdata-sections -Wall -Wextra -Winit-self -Wpointer-arith -Wreturn-type -Waddress -Wsequence-point -Wformat-security -Wmissing-include-dirs -Wfloat-equal -Wundef -Wshadow -Wcast-align -Wconversion -Wredundant-decls -DITT_ARCH_IA64 -Wjump-misses-init -Wstrict-prototypes -Wunsuffixed-float-constants -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -UDEBUG -DNDEBUG -ffunction-sections -fdata-sections -Wall -Wextra -Winit-self -Wpointer-arith -Wreturn-type -Waddress -Wsequence-point -Wformat-security -Wmissing-include-dirs -Wfloat-equal -Wundef -Wshadow -Wcast-align -Wconversion -Wredundant-decls -DITT_ARCH_IA64 -Wjump-misses-init -Wstrict-prototypes -Wunsuffixed-float-constants -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -UDEBUG -DNDEBUG -ffunction-sections -fdata-sections -Wall -Wextra -Winit-self -Wpointer-arith -Wreturn-type -Waddress -Wsequence-point -Wformat-security -Wmissing-include-dirs -Wfloat-equal -Wundef -Wshadow -Wcast-align -Wconversion -Wredundant-decls -DITT_ARCH_IA64 -Wjump-misses-init -Wstrict-prototypes -Wunsuffixed-float-constants -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -UDEBUG -DNDEBUG -ffunction-sections -fdata-sections -Wall -Wextra -Winit-self -Wpointer-arith -Wreturn-type -Waddress -Wsequence-point -Wformat-security -Wmissing-include-dirs -Wfloat-equal -Wundef -Wshadow -Wcast-align -Wconversion -Wredundant-decls -DITT_ARCH_IA64 -DDISABLE_TRACE -fPIC -DSE_SIM -Werror -g -I ../../../include -fPIC -Wno-strict-prototypes   -c -o ittnotify_static.o ittnotify_static.c
ittnotify_static.c: In function ‘__itt_init_ittlib’:
ittnotify_static.c:978:36: error: this statement may fall through [-Werror=implicit-fallthrough=]
                             groups = __itt_group_legacy;
                             ~~~~~~~^~~~~~~~~~~~~~~~~~~~
ittnotify_static.c:979:25: note: here
                         case 1:
                         ^~~~
cc1: all warnings being treated as errors
lzha101 commented 5 years ago

Seems your environment sets CFLAGS as a global environment and it would take all history setting to build an object. But I think we can fix this failure by a slight modification in Makefiles.

thiagomacieira commented 5 years ago

Yes, I have a few CFLAGS more set. But enabling -Werror for regular users of a project is inadviseable for exactly these reasons: your users will upgrade compilers, use different compilers and enable more optimisation options, ones you did not check. Just think of it this way: you can't test today with a compiler that will only exist in 2021.

It's fine for developers of a given project to enable -Werror during development. Recommended, even.

But that option should not be enabled by default for users of that project. Please remove it from the releases.

andyzyb commented 5 years ago

SGX is a security centric project and we'd like to have high criteria for compiler options to avoid potential coding issues. -Werror option has been tested on all the OS version we currently support. For other distros we don't support currently and there may be issue in compiling with -Werror, it would be better to fix the compiling issue instead of disabling -Werror flag.

thiagomacieira commented 5 years ago

Let me be clear of what I am asking: you should have -Wall -Wextra -Werror for your own developers, to increase code quality. I am just saying you shouldn't have -Werror for your users, the fact that your code is security-sensitive nothwithstanding.

Unless you want to say that your software should never be used in any Linux distribution version that you didn't specifically test, including future versions of the versions you have tested.

If this is the position of the maintainers, I'll just stop trying to make Clear Linux support this SDK.