Skip to contentSkip to conversation

Use "-Werror" for checking flags

PATCH Applied Bill Wendling 3 participants 7 messages Nov 7, 2019 – Nov 15, 2019

Subsystems (2)
KVM Kbuild
lore
Share & tools
lore

Bill Wendling

author
#0 lore raw

cover Use "-Werror" for checking flags

If "-Werror" isn't used then Clang doesn't fail if an unrecognized flag is used. This causes "cc-option" to pass when it shouldn't. Also add "cxx-option" for flags that aren't supported by the C++ compiler.

Bill Wendling (2):
Makefile: use "-Werror" in cc-option
Makefile: add "cxx-option" for C++ builds

 Makefile | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#1 lore raw

PATCH 1/2 Makefile: use "-Werror" in cc-option

The "cc-option" macro should use "-Werror" to determine if a flag is supported. Otherwise the test may not return a nonzero result. Also conditionalize some of the warning flags which aren't supported by clang.

Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)
Makefile +14 -6
index 32414dc..4c716da 100644
@@ -46,13 +46,13 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
 # cc-option
 # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
 
-cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
+cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
               > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
 
 COMMON_CFLAGS += -g $(autodepend-flags)
-COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
-COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
-COMMON_CFLAGS += -Werror
+COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
+COMMON_CFLAGS += -Wignored-qualifiers -Werror
+
 frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
 fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
 fnostack_protector := $(call cc-option, -fno-stack-protector, "")

@@ -60,16 +60,24 @@ fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
 wno_frame_address := $(call cc-option, -Wno-frame-address, "")
 fno_pic := $(call cc-option, -fno-pic, "")
 no_pie := $(call cc-option, -no-pie, "")
+wclobbered := $(call cc-option, -Wclobbered, "")
+wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
+wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
+
 COMMON_CFLAGS += $(fomit_frame_pointer)
 COMMON_CFLAGS += $(fno_stack_protector)
 COMMON_CFLAGS += $(fno_stack_protector_all)
 COMMON_CFLAGS += $(wno_frame_address)
 COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
 COMMON_CFLAGS += $(fno_pic) $(no_pie)
+COMMON_CFLAGS += $(wclobbered)
+COMMON_CFLAGS += $(wunused_but_set_parameter)
 
 CFLAGS += $(COMMON_CFLAGS)
-CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
-CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
+CFLAGS += $(wmissing_parameter_type)
+CFLAGS += $(wold_style_declaration)
+CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
 
 CXXFLAGS += $(COMMON_CFLAGS)
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
Reviewed-by on v1 1/2
#2 lore raw

Re: [kvm-unit-tests PATCH 1/2] Makefile: use "-Werror" in cc-option

Quoted 9 lines · Bill Wendling

The "cc-option" macro should use "-Werror" to determine if a flag is supported. Otherwise the test may not return a nonzero result. Also conditionalize some of the warning flags which aren't supported by clang.

Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
maintainer Applied
#3 lore raw

Re: [kvm-unit-tests PATCH 1/2] Makefile: use "-Werror" in cc-option

Quoted 6 lines · Thomas Huth
Quoted 5 lines · Bill Wendling

The "cc-option" macro should use "-Werror" to determine if a flag is supported. Otherwise the test may not return a nonzero result. Also conditionalize some of the warning flags which aren't supported by clang.

Quoted 7 lines
Quoted 4 lines
Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)
Reviewed-by Thomas Huth <[email protected]>

Queued, thanks.

(As for C++, the right thing to do would be to rewrite the tests for tools/testing/selftests/kvm/ and get rid of it...).

Paolo

author
#4 lore raw

PATCH 2/2 Makefile: add "cxx-option" for C++ builds

In reply to Bill Wendling

The C++ compiler may not support all of the same flags as the C compiler. Add a separate test for these flags.

Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
Makefile +11 -3
index 4c716da..9cb47e6 100644
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
 
 cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
               > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+              > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
 
 COMMON_CFLAGS += -g $(autodepend-flags)
 COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized

@@ -62,8 +64,6 @@ fno_pic := $(call cc-option, -fno-pic, "")
 no_pie := $(call cc-option, -no-pie, "")
 wclobbered := $(call cc-option, -Wclobbered, "")
 wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
-wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
-wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
 
 COMMON_CFLAGS += $(fomit_frame_pointer)
 COMMON_CFLAGS += $(fno_stack_protector)

@@ -75,11 +75,19 @@ COMMON_CFLAGS += $(wclobbered)
 COMMON_CFLAGS += $(wunused_but_set_parameter)
 
 CFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(COMMON_CFLAGS)
+
+wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
 CFLAGS += $(wmissing_parameter_type)
 CFLAGS += $(wold_style_declaration)
 CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
 
-CXXFLAGS += $(COMMON_CFLAGS)
+# Clang's C++ compiler doesn't support some of the flags its C compiler does.
+wmissing_parameter_type := $(call cxx-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cxx-option, -Wold-style-declaration, "")
+CXXFLAGS += $(wmissing_parameter_type)
+CXXFLAGS += $(wold_style_declaration)
 
 autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
#5 lore raw

Re: [kvm-unit-tests PATCH 2/2] Makefile: add "cxx-option" for C++ builds

Quoted 38 lines · Bill Wendling

The C++ compiler may not support all of the same flags as the C compiler. Add a separate test for these flags.

Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
Makefile +6 -2
index 4c716da..9cb47e6 100644
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
 
 cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
               > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+              > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
 
 COMMON_CFLAGS += -g $(autodepend-flags)
 COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized

@@ -62,8 +64,6 @@ fno_pic := $(call cc-option, -fno-pic, "")
 no_pie := $(call cc-option, -no-pie, "")
 wclobbered := $(call cc-option, -Wclobbered, "")
 wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
-wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
-wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
 
 COMMON_CFLAGS += $(fomit_frame_pointer)
 COMMON_CFLAGS += $(fno_stack_protector)

@@ -75,11 +75,19 @@ COMMON_CFLAGS += $(wclobbered)
 COMMON_CFLAGS += $(wunused_but_set_parameter)
 
 CFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(COMMON_CFLAGS)
+
+wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")

Makefile COMMON_CFLAGS += $(wclobbered) · line 90

Quoted 10 lines
Makefile +5 -1
 CFLAGS += $(wmissing_parameter_type)
 CFLAGS += $(wold_style_declaration)
 CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
 
-CXXFLAGS += $(COMMON_CFLAGS)
+# Clang's C++ compiler doesn't support some of the flags its C compiler does.
+wmissing_parameter_type := $(call cxx-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cxx-option, -Wold-style-declaration, "")
+CXXFLAGS += $(wmissing_parameter_type)
+CXXFLAGS += $(wold_style_declaration)

As mentioned in my mail to the previous version of this patch: I think both options are not valid with g++ as well, so I think this patch should simply be dropped.

Thomas

author
#6 lore raw

Re: [kvm-unit-tests PATCH 2/2] Makefile: add "cxx-option" for C++ builds

Quoted 44 lines · Thomas Huth
Quoted 42 lines · Bill Wendling

The C++ compiler may not support all of the same flags as the C compiler. Add a separate test for these flags.

Signed-off-by Bill Wendling <[email protected]>
---
 Makefile | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
Makefile +6 -2
index 4c716da..9cb47e6 100644
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
 
 cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
               > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+              > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
 
 COMMON_CFLAGS += -g $(autodepend-flags)
 COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized

@@ -62,8 +64,6 @@ fno_pic := $(call cc-option, -fno-pic, "")
 no_pie := $(call cc-option, -no-pie, "")
 wclobbered := $(call cc-option, -Wclobbered, "")
 wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
-wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
-wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
 
 COMMON_CFLAGS += $(fomit_frame_pointer)
 COMMON_CFLAGS += $(fno_stack_protector)

@@ -75,11 +75,19 @@ COMMON_CFLAGS += $(wclobbered)
 COMMON_CFLAGS += $(wunused_but_set_parameter)
 
 CFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(COMMON_CFLAGS)
+
+wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
 CFLAGS += $(wmissing_parameter_type)
 CFLAGS += $(wold_style_declaration)
 CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
Quoted 11 lines

Makefile COMMON_CFLAGS += $(wclobbered) · line 90

Quoted 6 lines
Makefile +5 -1
-CXXFLAGS += $(COMMON_CFLAGS)
+# Clang's C++ compiler doesn't support some of the flags its C compiler does.
+wmissing_parameter_type := $(call cxx-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cxx-option, -Wold-style-declaration, "")
+CXXFLAGS += $(wmissing_parameter_type)
+CXXFLAGS += $(wold_style_declaration)

As mentioned in my mail to the previous version of this patch: I think both options are not valid with g++ as well, so I think this patch should simply be dropped.

I agree. Thank you for the review!

-bw