| Message ID | 74b35aab0706fc977a3c5c3dfa20b52fda6f4698.1649855186.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen/build: Add cppcheck and cppcheck-html make rules | expand |
On Wed, 13 Apr 2022, Bertrand Marquis wrote: > cppcheck can be used to check Xen code quality. > > To create a report do "make cppcheck" on a built tree adding any options > you added during the process you used to build xen (like CROSS_COMPILE > or XEN_TARGET_ARCH). This will generate an xml report xen-cppcheck.xml. > > To create a html report do "make cppcheck-html" in the same way and a > full report to be seen in a browser will be generated in > cppcheck-htmlreport/index.html. > > For better results it is recommended to build your own cppcheck from the > latest sources that you can find at [1]. > Development and result analysis has been done with cppcheck 2.7. > > The Makefile rule is searching for all C files which have been compiled > (ie which have a generated .o file) and is running cppcheck on all of > them using the current configuration of xen so only the code actually > compiled is checked. > > A new tool is introduced to merge all cppcheck reports into one global > report including all findings and removing duplicates. > > Some extra variables can be used to customize the report: > - CPPCHECK can be used to give the full path to the cppcheck binary to > use (default is to use the one from the standard path). > - CPPCHECK_HTMLREPORT can be used to give the full path to > cppcheck-htmlreport (default is to use the one from the standard path). > > This has been tested on several arm configurations (x86 should work but > has not been tested). > > [1] https://cppcheck.sourceforge.io/ > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > Signed-off-by: Michal Orzel <michal.orzel@arm.com> This is better, thanks! I don't have a lot of comments as it looks pretty OK to me already. Only one comment on the python script. > --- > Changes in v2: > - catch parsing errors during merge and tell the user to update cppcheck > - fix alignments in Makefile > - use srctree and objtree instead of BASEDIR > - remove SCTLR invalid change > - add a check of cppcheck version and mandate version 2.7 or greater > - rework the merge script to make it faster and display progress > --- > .gitignore | 3 + > xen/Makefile | 84 +++++++++++++++++++++++++++- > xen/arch/arm/include/asm/processor.h | 2 + > xen/include/xen/config.h | 4 ++ > xen/include/xen/kconfig.h | 5 ++ > xen/tools/merge_cppcheck_reports.py | 83 +++++++++++++++++++++++++++ > 6 files changed, 179 insertions(+), 2 deletions(-) > create mode 100755 xen/tools/merge_cppcheck_reports.py > > diff --git a/.gitignore b/.gitignore > index d425be4bd9..0d2d60b8f1 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -7,6 +7,7 @@ > *.o > *.d > *.d2 > +*.c.cppcheck > *.opic > *.a > *.so > @@ -296,6 +297,7 @@ xen/.banner > xen/.config > xen/.config.old > xen/.xen.elf32 > +xen/xen-cppcheck.xml > xen/System.map > xen/arch/x86/boot/mkelf32 > xen/arch/x86/boot/cmdline.S > @@ -316,6 +318,7 @@ xen/arch/*/efi/runtime.c > xen/arch/*/include/asm/asm-offsets.h > xen/common/config_data.S > xen/common/config.gz > +xen/cppcheck-htmlreport > xen/include/headers*.chk > xen/include/compat/* > xen/include/config/ > diff --git a/xen/Makefile b/xen/Makefile > index ebc7854c08..734f9f2b4f 100644 > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -335,7 +335,7 @@ export CFLAGS_UBSAN > > endif # need-config > > -main-targets := build install uninstall clean distclean MAP > +main-targets := build install uninstall clean distclean MAP cppcheck cppcheck-html > .PHONY: $(main-targets) > ifneq ($(XEN_TARGET_ARCH),x86_32) > $(main-targets): %: _% ; > @@ -423,15 +423,17 @@ _clean: > $(Q)$(MAKE) $(clean)=tools/kconfig > find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \ > -o -name ".*.o.tmp" -o -name "*~" -o -name "core" \ > - -o -name '*.lex.c' -o -name '*.tab.[ch]' \ > + -o -name '*.lex.c' -o -name '*.tab.[ch]' -o -name '*.c.cppcheck' \ > -o -name "*.gcno" -o -name ".*.cmd" -o -name "lib.a" \) -exec rm -f {} \; > rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map > rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h > rm -f .banner .allconfig.tmp include/xen/compile.h > + rm -f xen-cppcheck.xml > > .PHONY: _distclean > _distclean: clean > rm -f tags TAGS cscope.files cscope.in.out cscope.out cscope.po.out GTAGS GPATH GRTAGS GSYMS .config > + rm -rf $(CPPCHECK_HTMLREPORT_OUTDIR) > > $(TARGET).gz: $(TARGET) > gzip -n -f -9 < $< > $@.new > @@ -505,6 +507,84 @@ cloc: > done; \ > done | cloc --list-file=- > > +# What cppcheck command to use. > +# To get proper results, it is recommended to build cppcheck manually from the > +# latest source and use CPPCHECK to give the full path to the built version. > +CPPCHECK ?= cppcheck > + > +# What cppcheck-htmlreport to use. > +# If you give the full path to a self compiled cppcheck, this should be set > +# to the full path to cppcheck-html in the htmlreport directory of cppcheck. > +# On recent distribution, this is available in the standard path. > +CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport > + > +# By default we generate the report in cppcheck-htmlreport directory in the > +# build directory. This can be changed by giving a directory in this variable. > +CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport > + > +# Compile flags to pass to cppcheck: > +# - include directories and defines Xen Makefile is passing (from CFLAGS) > +# - include config.h as this is passed directly to the compiler. > +# - define CPPCHECK as we use to disable or enable some specific part of the > +# code to solve some cppcheck issues. > +# - explicitely enable some cppcheck checks as we do not want to use "all" > +# which includes unusedFunction which gives wrong positives as we check file > +# per file. > +# > +# Compiler defines are in compiler-def.h which is included in config.h > +# > +CPPCHECKFLAGS := -DCPPCHECK --max-ctu-depth=10 \ > + --enable=style,information,missingInclude \ > + --include=$(srctree)/include/xen/config.h \ > + -I $(srctree)/xsm/flask/include \ > + -I $(srctree)/include/xen/libfdt \ > + $(filter -D% -I%,$(CFLAGS)) > + > +# We need to find all C files (as we are not checking assembly files) so > +# we find all generated .o files which have a .c corresponding file. > +CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \ > + $(filter-out $(objtree)/tools/%, \ > + $(shell find $(objtree) -name "*.o")))) > + > +quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(srctree)/%,%,$<) > +cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \ > + --output-file=$@ $< > + > +quiet_cmd_merge_cppcheck_reports = CPPCHECK-MERGE $@ > +cmd_merge_cppcheck_reports = $(srctree)/tools/merge_cppcheck_reports.py $^ $@ > + > +quiet_cmd_cppcheck_html = CPPCHECK-HTML $< > +cmd_cppcheck_html = $(CPPCHECK_HTMLREPORT) --file=$< --source-dir=$(srctree) \ > + --report-dir=$(CPPCHECK_HTMLREPORT_OUTDIR) --title=Xen > + > +PHONY += _cppcheck _cppcheck-html cppcheck-version > + > +_cppcheck-html: xen-cppcheck.xml > + $(call if_changed,cppcheck_html) > + > +_cppcheck: xen-cppcheck.xml > + > +xen-cppcheck.xml: $(patsubst $(srctree)/%.c,$(objtree)/%.c.cppcheck,$(CPPCHECKFILES)) > +ifeq ($(CPPCHECKFILES),) > + $(error Please build Xen before running cppcheck) > +endif > + $(call if_changed,merge_cppcheck_reports) > + > +$(objtree)/%.c.cppcheck: $(srctree)/%.c $(objtree)/include/generated/autoconf.h $(objtree)/include/generated/compiler-def.h | cppcheck-version > + $(call if_changed,cppcheck_xml) > + > +cppcheck-version: > +ifeq ($(shell which $(CPPCHECK)),) > + $(error Cannot find cppcheck executable: $(CPPCHECK)) > +endif > +ifeq ($(shell $(CPPCHECK) --version | awk '{print ($$2 < 2.7)}'),1) > + $(error Please upgrade your cppcheck to version 2.7 or greater) > +endif > + > +# Put this in generated headers this way it is cleaned by include/Makefile > +$(objtree)/include/generated/compiler-def.h: > + $(Q)$(CC) -dM -E -o $@ - < /dev/null > + > endif #config-build > > PHONY += FORCE > diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h > index 852b5f3c24..ef37cfa16f 100644 > --- a/xen/arch/arm/include/asm/processor.h > +++ b/xen/arch/arm/include/asm/processor.h > @@ -219,9 +219,11 @@ > SCTLR_Axx_ELx_A | SCTLR_Axx_ELx_C |\ > SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) > > +#ifndef CPPCHECK > #if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL > #error "Inconsistent SCTLR_EL2 set/clear bits" > #endif > +#endif > > #endif > > diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h > index 85c6f59be9..d888b2314d 100644 > --- a/xen/include/xen/config.h > +++ b/xen/include/xen/config.h > @@ -7,6 +7,10 @@ > #ifndef __XEN_CONFIG_H__ > #define __XEN_CONFIG_H__ > > +#ifdef CPPCHECK > +#include <generated/compiler-def.h> > +#endif > + > #include <xen/kconfig.h> > > #ifndef __ASSEMBLY__ > diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h > index 4d58c5bb3c..a717b0819c 100644 > --- a/xen/include/xen/kconfig.h > +++ b/xen/include/xen/kconfig.h > @@ -8,6 +8,10 @@ > * these only work with boolean option. > */ > > +/* cppcheck is failing to parse the macro so use a dummy one */ > +#ifdef CPPCHECK > +#define IS_ENABLED(option) option > +#else > /* > * Getting something that works in C and CPP for an arg that may or may > * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1" > @@ -27,5 +31,6 @@ > * otherwise. > */ > #define IS_ENABLED(option) config_enabled(option) > +#endif > > #endif /* __XEN_KCONFIG_H */ > diff --git a/xen/tools/merge_cppcheck_reports.py b/xen/tools/merge_cppcheck_reports.py > new file mode 100755 > index 0000000000..eabf558768 > --- /dev/null > +++ b/xen/tools/merge_cppcheck_reports.py > @@ -0,0 +1,83 @@ > +#!/usr/bin/env python > + > +""" > +This script acts as a tool to merge XML files created by cppcheck. > +Usage: > + merge_cppcheck_reports.py [FILES] [OUTPUT] > + > + FILES - list of XML files with extension .cppcheck > + OUTPUT - file to store results (with .xml extension). > + If not specified, the script will print results to stdout. > +""" > + > +import sys > +from xml.etree import ElementTree > + > +def elements_equal(el1, el2): > + if type(el1) != type(el2): return False > + > + el1_location = str(el1.find('location').attrib) > + el2_location = str(el2.find('location').attrib) I managed to repro another stacktrace: CPPCHECK-MERGE xen-cppcheck.xml 123 / 191Traceback (most recent call last): File "./tools/merge_cppcheck_reports.py", line 83, in <module> run() File "./tools/merge_cppcheck_reports.py", line 72, in run result = merge(files) File "./tools/merge_cppcheck_reports.py", line 53, in merge if not contain_element(xml_error_elem, curr_elem_list): File "./tools/merge_cppcheck_reports.py", line 28, in contain_element if elements_equal(new, elem): File "./tools/merge_cppcheck_reports.py", line 19, in elements_equal el1_location = str(el1.find('location').attrib) AttributeError: 'NoneType' object has no attribute 'attrib' make: *** [Makefile:685: xen-cppcheck.xml] Error 1 It must be a natural talent :-) Should we have a try/except also here as well? FYI I added: if el1.find('location') is None: return False if el2.find('location') is None: return False And that solved the issue for me > + if el1_location != el2_location: return False > + > + return True > + > +def contain_element(new, lst): > + for elem in lst: > + if elements_equal(new, elem): > + return True > + return False > + > +def merge(files): > + try: > + result_xml_root = ElementTree.parse(files[0]).getroot() > + except ParseError: > + print("Xml parsing error in %s\n" % (files[0])) > + print("Please upgrade your cppcheck to version 2.7 or greater") > + sys.exit(1) > + insert_point = result_xml_root.findall("errors")[0] > + curr = 1 > + total = len(files) > + numelem = len(insert_point) > + for xml_file in files[1:]: > + try: > + xml_root = ElementTree.parse(xml_file).getroot() > + except ParseError: > + print("Xml parsing error in %s\n" % (xml_file)) > + print("Please upgrade your cppcheck to version 2.7 or greater") > + sys.exit(1) > + curr_elem_list = list(insert_point) > + new_elem_list = list(xml_root.findall("errors")[0]) > + for xml_error_elem in new_elem_list: > + if not contain_element(xml_error_elem, curr_elem_list): > + insert_point.insert(1,xml_error_elem) > + numelem = numelem + 1 > + curr = curr + 1 > + sys.stdout.write('\r') > + sys.stdout.write(" %d / %d" % (curr,total)) > + sys.stdout.flush() > + > + sys.stdout.write('\r\n') > + print("Done: %d elements" % (numelem)) > + return result_xml_root > + > +def run(): > + files = [] > + output = None > + for i in sys.argv[1:]: > + output = i if '.xml' in i else None > + files.append(i) if '.cppcheck' in i else None > + > + result = merge(files) > + > + if result is None: > + return > + > + if output is not None: > + ElementTree.ElementTree(result).write(output) > + else: > + print(ElementTree.tostring(result).decode('utf-8')) > + > +if __name__ == '__main__': > + run() > -- > 2.25.1 >
Hi Stefano, > On 16 Apr 2022, at 00:42, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Wed, 13 Apr 2022, Bertrand Marquis wrote: >> cppcheck can be used to check Xen code quality. >> >> To create a report do "make cppcheck" on a built tree adding any options >> you added during the process you used to build xen (like CROSS_COMPILE >> or XEN_TARGET_ARCH). This will generate an xml report xen-cppcheck.xml. >> >> To create a html report do "make cppcheck-html" in the same way and a >> full report to be seen in a browser will be generated in >> cppcheck-htmlreport/index.html. >> >> For better results it is recommended to build your own cppcheck from the >> latest sources that you can find at [1]. >> Development and result analysis has been done with cppcheck 2.7. >> >> The Makefile rule is searching for all C files which have been compiled >> (ie which have a generated .o file) and is running cppcheck on all of >> them using the current configuration of xen so only the code actually >> compiled is checked. >> >> A new tool is introduced to merge all cppcheck reports into one global >> report including all findings and removing duplicates. >> >> Some extra variables can be used to customize the report: >> - CPPCHECK can be used to give the full path to the cppcheck binary to >> use (default is to use the one from the standard path). >> - CPPCHECK_HTMLREPORT can be used to give the full path to >> cppcheck-htmlreport (default is to use the one from the standard path). >> >> This has been tested on several arm configurations (x86 should work but >> has not been tested). >> >> [1] https://cppcheck.sourceforge.io/ >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> Signed-off-by: Michal Orzel <michal.orzel@arm.com> > > This is better, thanks! I don't have a lot of comments as it looks > pretty OK to me already. Only one comment on the python script. > > >> --- >> Changes in v2: >> - catch parsing errors during merge and tell the user to update cppcheck >> - fix alignments in Makefile >> - use srctree and objtree instead of BASEDIR >> - remove SCTLR invalid change >> - add a check of cppcheck version and mandate version 2.7 or greater >> - rework the merge script to make it faster and display progress >> --- >> .gitignore | 3 + >> xen/Makefile | 84 +++++++++++++++++++++++++++- >> xen/arch/arm/include/asm/processor.h | 2 + >> xen/include/xen/config.h | 4 ++ >> xen/include/xen/kconfig.h | 5 ++ >> xen/tools/merge_cppcheck_reports.py | 83 +++++++++++++++++++++++++++ >> 6 files changed, 179 insertions(+), 2 deletions(-) >> create mode 100755 xen/tools/merge_cppcheck_reports.py >> >> diff --git a/.gitignore b/.gitignore >> index d425be4bd9..0d2d60b8f1 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -7,6 +7,7 @@ >> *.o >> *.d >> *.d2 >> +*.c.cppcheck >> *.opic >> *.a >> *.so >> @@ -296,6 +297,7 @@ xen/.banner >> xen/.config >> xen/.config.old >> xen/.xen.elf32 >> +xen/xen-cppcheck.xml >> xen/System.map >> xen/arch/x86/boot/mkelf32 >> xen/arch/x86/boot/cmdline.S >> @@ -316,6 +318,7 @@ xen/arch/*/efi/runtime.c >> xen/arch/*/include/asm/asm-offsets.h >> xen/common/config_data.S >> xen/common/config.gz >> +xen/cppcheck-htmlreport >> xen/include/headers*.chk >> xen/include/compat/* >> xen/include/config/ >> diff --git a/xen/Makefile b/xen/Makefile >> index ebc7854c08..734f9f2b4f 100644 >> --- a/xen/Makefile >> +++ b/xen/Makefile >> @@ -335,7 +335,7 @@ export CFLAGS_UBSAN >> >> endif # need-config >> >> -main-targets := build install uninstall clean distclean MAP >> +main-targets := build install uninstall clean distclean MAP cppcheck cppcheck-html >> .PHONY: $(main-targets) >> ifneq ($(XEN_TARGET_ARCH),x86_32) >> $(main-targets): %: _% ; >> @@ -423,15 +423,17 @@ _clean: >> $(Q)$(MAKE) $(clean)=tools/kconfig >> find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \ >> -o -name ".*.o.tmp" -o -name "*~" -o -name "core" \ >> - -o -name '*.lex.c' -o -name '*.tab.[ch]' \ >> + -o -name '*.lex.c' -o -name '*.tab.[ch]' -o -name '*.c.cppcheck' \ >> -o -name "*.gcno" -o -name ".*.cmd" -o -name "lib.a" \) -exec rm -f {} \; >> rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map >> rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h >> rm -f .banner .allconfig.tmp include/xen/compile.h >> + rm -f xen-cppcheck.xml >> >> .PHONY: _distclean >> _distclean: clean >> rm -f tags TAGS cscope.files cscope.in.out cscope.out cscope.po.out GTAGS GPATH GRTAGS GSYMS .config >> + rm -rf $(CPPCHECK_HTMLREPORT_OUTDIR) >> >> $(TARGET).gz: $(TARGET) >> gzip -n -f -9 < $< > $@.new >> @@ -505,6 +507,84 @@ cloc: >> done; \ >> done | cloc --list-file=- >> >> +# What cppcheck command to use. >> +# To get proper results, it is recommended to build cppcheck manually from the >> +# latest source and use CPPCHECK to give the full path to the built version. >> +CPPCHECK ?= cppcheck >> + >> +# What cppcheck-htmlreport to use. >> +# If you give the full path to a self compiled cppcheck, this should be set >> +# to the full path to cppcheck-html in the htmlreport directory of cppcheck. >> +# On recent distribution, this is available in the standard path. >> +CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport >> + >> +# By default we generate the report in cppcheck-htmlreport directory in the >> +# build directory. This can be changed by giving a directory in this variable. >> +CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport >> + >> +# Compile flags to pass to cppcheck: >> +# - include directories and defines Xen Makefile is passing (from CFLAGS) >> +# - include config.h as this is passed directly to the compiler. >> +# - define CPPCHECK as we use to disable or enable some specific part of the >> +# code to solve some cppcheck issues. >> +# - explicitely enable some cppcheck checks as we do not want to use "all" >> +# which includes unusedFunction which gives wrong positives as we check file >> +# per file. >> +# >> +# Compiler defines are in compiler-def.h which is included in config.h >> +# >> +CPPCHECKFLAGS := -DCPPCHECK --max-ctu-depth=10 \ >> + --enable=style,information,missingInclude \ >> + --include=$(srctree)/include/xen/config.h \ >> + -I $(srctree)/xsm/flask/include \ >> + -I $(srctree)/include/xen/libfdt \ >> + $(filter -D% -I%,$(CFLAGS)) >> + >> +# We need to find all C files (as we are not checking assembly files) so >> +# we find all generated .o files which have a .c corresponding file. >> +CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \ >> + $(filter-out $(objtree)/tools/%, \ >> + $(shell find $(objtree) -name "*.o")))) >> + >> +quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(srctree)/%,%,$<) >> +cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \ >> + --output-file=$@ $< >> + >> +quiet_cmd_merge_cppcheck_reports = CPPCHECK-MERGE $@ >> +cmd_merge_cppcheck_reports = $(srctree)/tools/merge_cppcheck_reports.py $^ $@ >> + >> +quiet_cmd_cppcheck_html = CPPCHECK-HTML $< >> +cmd_cppcheck_html = $(CPPCHECK_HTMLREPORT) --file=$< --source-dir=$(srctree) \ >> + --report-dir=$(CPPCHECK_HTMLREPORT_OUTDIR) --title=Xen >> + >> +PHONY += _cppcheck _cppcheck-html cppcheck-version >> + >> +_cppcheck-html: xen-cppcheck.xml >> + $(call if_changed,cppcheck_html) >> + >> +_cppcheck: xen-cppcheck.xml >> + >> +xen-cppcheck.xml: $(patsubst $(srctree)/%.c,$(objtree)/%.c.cppcheck,$(CPPCHECKFILES)) >> +ifeq ($(CPPCHECKFILES),) >> + $(error Please build Xen before running cppcheck) >> +endif >> + $(call if_changed,merge_cppcheck_reports) >> + >> +$(objtree)/%.c.cppcheck: $(srctree)/%.c $(objtree)/include/generated/autoconf.h $(objtree)/include/generated/compiler-def.h | cppcheck-version >> + $(call if_changed,cppcheck_xml) >> + >> +cppcheck-version: >> +ifeq ($(shell which $(CPPCHECK)),) >> + $(error Cannot find cppcheck executable: $(CPPCHECK)) >> +endif >> +ifeq ($(shell $(CPPCHECK) --version | awk '{print ($$2 < 2.7)}'),1) >> + $(error Please upgrade your cppcheck to version 2.7 or greater) >> +endif >> + >> +# Put this in generated headers this way it is cleaned by include/Makefile >> +$(objtree)/include/generated/compiler-def.h: >> + $(Q)$(CC) -dM -E -o $@ - < /dev/null >> + >> endif #config-build >> >> PHONY += FORCE >> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h >> index 852b5f3c24..ef37cfa16f 100644 >> --- a/xen/arch/arm/include/asm/processor.h >> +++ b/xen/arch/arm/include/asm/processor.h >> @@ -219,9 +219,11 @@ >> SCTLR_Axx_ELx_A | SCTLR_Axx_ELx_C |\ >> SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) >> >> +#ifndef CPPCHECK >> #if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL >> #error "Inconsistent SCTLR_EL2 set/clear bits" >> #endif >> +#endif >> >> #endif >> >> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h >> index 85c6f59be9..d888b2314d 100644 >> --- a/xen/include/xen/config.h >> +++ b/xen/include/xen/config.h >> @@ -7,6 +7,10 @@ >> #ifndef __XEN_CONFIG_H__ >> #define __XEN_CONFIG_H__ >> >> +#ifdef CPPCHECK >> +#include <generated/compiler-def.h> >> +#endif >> + >> #include <xen/kconfig.h> >> >> #ifndef __ASSEMBLY__ >> diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h >> index 4d58c5bb3c..a717b0819c 100644 >> --- a/xen/include/xen/kconfig.h >> +++ b/xen/include/xen/kconfig.h >> @@ -8,6 +8,10 @@ >> * these only work with boolean option. >> */ >> >> +/* cppcheck is failing to parse the macro so use a dummy one */ >> +#ifdef CPPCHECK >> +#define IS_ENABLED(option) option >> +#else >> /* >> * Getting something that works in C and CPP for an arg that may or may >> * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1" >> @@ -27,5 +31,6 @@ >> * otherwise. >> */ >> #define IS_ENABLED(option) config_enabled(option) >> +#endif >> >> #endif /* __XEN_KCONFIG_H */ >> diff --git a/xen/tools/merge_cppcheck_reports.py b/xen/tools/merge_cppcheck_reports.py >> new file mode 100755 >> index 0000000000..eabf558768 >> --- /dev/null >> +++ b/xen/tools/merge_cppcheck_reports.py >> @@ -0,0 +1,83 @@ >> +#!/usr/bin/env python >> + >> +""" >> +This script acts as a tool to merge XML files created by cppcheck. >> +Usage: >> + merge_cppcheck_reports.py [FILES] [OUTPUT] >> + >> + FILES - list of XML files with extension .cppcheck >> + OUTPUT - file to store results (with .xml extension). >> + If not specified, the script will print results to stdout. >> +""" >> + >> +import sys >> +from xml.etree import ElementTree >> + >> +def elements_equal(el1, el2): >> + if type(el1) != type(el2): return False >> + >> + el1_location = str(el1.find('location').attrib) >> + el2_location = str(el2.find('location').attrib) > > I managed to repro another stacktrace: > > CPPCHECK-MERGE xen-cppcheck.xml > 123 / 191Traceback (most recent call last): > File "./tools/merge_cppcheck_reports.py", line 83, in <module> > run() > File "./tools/merge_cppcheck_reports.py", line 72, in run > result = merge(files) > File "./tools/merge_cppcheck_reports.py", line 53, in merge > if not contain_element(xml_error_elem, curr_elem_list): > File "./tools/merge_cppcheck_reports.py", line 28, in contain_element > if elements_equal(new, elem): > File "./tools/merge_cppcheck_reports.py", line 19, in elements_equal > el1_location = str(el1.find('location').attrib) > AttributeError: 'NoneType' object has no attribute 'attrib' > make: *** [Makefile:685: xen-cppcheck.xml] Error 1 > > It must be a natural talent :-) > > Should we have a try/except also here as well? > > FYI I added: > > if el1.find('location') is None: return False > if el2.find('location') is None: return False > > And that solved the issue for me You are very talented to trigger the errors (I do not have any on my side). Anyway I will include this and make the try/except to catch any kind of errors in v3. Thanks a lot for the deep testing :-) Cheers Bertrand
diff --git a/.gitignore b/.gitignore index d425be4bd9..0d2d60b8f1 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ *.o *.d *.d2 +*.c.cppcheck *.opic *.a *.so @@ -296,6 +297,7 @@ xen/.banner xen/.config xen/.config.old xen/.xen.elf32 +xen/xen-cppcheck.xml xen/System.map xen/arch/x86/boot/mkelf32 xen/arch/x86/boot/cmdline.S @@ -316,6 +318,7 @@ xen/arch/*/efi/runtime.c xen/arch/*/include/asm/asm-offsets.h xen/common/config_data.S xen/common/config.gz +xen/cppcheck-htmlreport xen/include/headers*.chk xen/include/compat/* xen/include/config/ diff --git a/xen/Makefile b/xen/Makefile index ebc7854c08..734f9f2b4f 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -335,7 +335,7 @@ export CFLAGS_UBSAN endif # need-config -main-targets := build install uninstall clean distclean MAP +main-targets := build install uninstall clean distclean MAP cppcheck cppcheck-html .PHONY: $(main-targets) ifneq ($(XEN_TARGET_ARCH),x86_32) $(main-targets): %: _% ; @@ -423,15 +423,17 @@ _clean: $(Q)$(MAKE) $(clean)=tools/kconfig find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \ -o -name ".*.o.tmp" -o -name "*~" -o -name "core" \ - -o -name '*.lex.c' -o -name '*.tab.[ch]' \ + -o -name '*.lex.c' -o -name '*.tab.[ch]' -o -name '*.c.cppcheck' \ -o -name "*.gcno" -o -name ".*.cmd" -o -name "lib.a" \) -exec rm -f {} \; rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h rm -f .banner .allconfig.tmp include/xen/compile.h + rm -f xen-cppcheck.xml .PHONY: _distclean _distclean: clean rm -f tags TAGS cscope.files cscope.in.out cscope.out cscope.po.out GTAGS GPATH GRTAGS GSYMS .config + rm -rf $(CPPCHECK_HTMLREPORT_OUTDIR) $(TARGET).gz: $(TARGET) gzip -n -f -9 < $< > $@.new @@ -505,6 +507,84 @@ cloc: done; \ done | cloc --list-file=- +# What cppcheck command to use. +# To get proper results, it is recommended to build cppcheck manually from the +# latest source and use CPPCHECK to give the full path to the built version. +CPPCHECK ?= cppcheck + +# What cppcheck-htmlreport to use. +# If you give the full path to a self compiled cppcheck, this should be set +# to the full path to cppcheck-html in the htmlreport directory of cppcheck. +# On recent distribution, this is available in the standard path. +CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport + +# By default we generate the report in cppcheck-htmlreport directory in the +# build directory. This can be changed by giving a directory in this variable. +CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport + +# Compile flags to pass to cppcheck: +# - include directories and defines Xen Makefile is passing (from CFLAGS) +# - include config.h as this is passed directly to the compiler. +# - define CPPCHECK as we use to disable or enable some specific part of the +# code to solve some cppcheck issues. +# - explicitely enable some cppcheck checks as we do not want to use "all" +# which includes unusedFunction which gives wrong positives as we check file +# per file. +# +# Compiler defines are in compiler-def.h which is included in config.h +# +CPPCHECKFLAGS := -DCPPCHECK --max-ctu-depth=10 \ + --enable=style,information,missingInclude \ + --include=$(srctree)/include/xen/config.h \ + -I $(srctree)/xsm/flask/include \ + -I $(srctree)/include/xen/libfdt \ + $(filter -D% -I%,$(CFLAGS)) + +# We need to find all C files (as we are not checking assembly files) so +# we find all generated .o files which have a .c corresponding file. +CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \ + $(filter-out $(objtree)/tools/%, \ + $(shell find $(objtree) -name "*.o")))) + +quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(srctree)/%,%,$<) +cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \ + --output-file=$@ $< + +quiet_cmd_merge_cppcheck_reports = CPPCHECK-MERGE $@ +cmd_merge_cppcheck_reports = $(srctree)/tools/merge_cppcheck_reports.py $^ $@ + +quiet_cmd_cppcheck_html = CPPCHECK-HTML $< +cmd_cppcheck_html = $(CPPCHECK_HTMLREPORT) --file=$< --source-dir=$(srctree) \ + --report-dir=$(CPPCHECK_HTMLREPORT_OUTDIR) --title=Xen + +PHONY += _cppcheck _cppcheck-html cppcheck-version + +_cppcheck-html: xen-cppcheck.xml + $(call if_changed,cppcheck_html) + +_cppcheck: xen-cppcheck.xml + +xen-cppcheck.xml: $(patsubst $(srctree)/%.c,$(objtree)/%.c.cppcheck,$(CPPCHECKFILES)) +ifeq ($(CPPCHECKFILES),) + $(error Please build Xen before running cppcheck) +endif + $(call if_changed,merge_cppcheck_reports) + +$(objtree)/%.c.cppcheck: $(srctree)/%.c $(objtree)/include/generated/autoconf.h $(objtree)/include/generated/compiler-def.h | cppcheck-version + $(call if_changed,cppcheck_xml) + +cppcheck-version: +ifeq ($(shell which $(CPPCHECK)),) + $(error Cannot find cppcheck executable: $(CPPCHECK)) +endif +ifeq ($(shell $(CPPCHECK) --version | awk '{print ($$2 < 2.7)}'),1) + $(error Please upgrade your cppcheck to version 2.7 or greater) +endif + +# Put this in generated headers this way it is cleaned by include/Makefile +$(objtree)/include/generated/compiler-def.h: + $(Q)$(CC) -dM -E -o $@ - < /dev/null + endif #config-build PHONY += FORCE diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index 852b5f3c24..ef37cfa16f 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -219,9 +219,11 @@ SCTLR_Axx_ELx_A | SCTLR_Axx_ELx_C |\ SCTLR_Axx_ELx_WXN | SCTLR_Axx_ELx_EE) +#ifndef CPPCHECK #if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL #error "Inconsistent SCTLR_EL2 set/clear bits" #endif +#endif #endif diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h index 85c6f59be9..d888b2314d 100644 --- a/xen/include/xen/config.h +++ b/xen/include/xen/config.h @@ -7,6 +7,10 @@ #ifndef __XEN_CONFIG_H__ #define __XEN_CONFIG_H__ +#ifdef CPPCHECK +#include <generated/compiler-def.h> +#endif + #include <xen/kconfig.h> #ifndef __ASSEMBLY__ diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h index 4d58c5bb3c..a717b0819c 100644 --- a/xen/include/xen/kconfig.h +++ b/xen/include/xen/kconfig.h @@ -8,6 +8,10 @@ * these only work with boolean option. */ +/* cppcheck is failing to parse the macro so use a dummy one */ +#ifdef CPPCHECK +#define IS_ENABLED(option) option +#else /* * Getting something that works in C and CPP for an arg that may or may * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1" @@ -27,5 +31,6 @@ * otherwise. */ #define IS_ENABLED(option) config_enabled(option) +#endif #endif /* __XEN_KCONFIG_H */ diff --git a/xen/tools/merge_cppcheck_reports.py b/xen/tools/merge_cppcheck_reports.py new file mode 100755 index 0000000000..eabf558768 --- /dev/null +++ b/xen/tools/merge_cppcheck_reports.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python + +""" +This script acts as a tool to merge XML files created by cppcheck. +Usage: + merge_cppcheck_reports.py [FILES] [OUTPUT] + + FILES - list of XML files with extension .cppcheck + OUTPUT - file to store results (with .xml extension). + If not specified, the script will print results to stdout. +""" + +import sys +from xml.etree import ElementTree + +def elements_equal(el1, el2): + if type(el1) != type(el2): return False + + el1_location = str(el1.find('location').attrib) + el2_location = str(el2.find('location').attrib) + + if el1_location != el2_location: return False + + return True + +def contain_element(new, lst): + for elem in lst: + if elements_equal(new, elem): + return True + return False + +def merge(files): + try: + result_xml_root = ElementTree.parse(files[0]).getroot() + except ParseError: + print("Xml parsing error in %s\n" % (files[0])) + print("Please upgrade your cppcheck to version 2.7 or greater") + sys.exit(1) + insert_point = result_xml_root.findall("errors")[0] + curr = 1 + total = len(files) + numelem = len(insert_point) + for xml_file in files[1:]: + try: + xml_root = ElementTree.parse(xml_file).getroot() + except ParseError: + print("Xml parsing error in %s\n" % (xml_file)) + print("Please upgrade your cppcheck to version 2.7 or greater") + sys.exit(1) + curr_elem_list = list(insert_point) + new_elem_list = list(xml_root.findall("errors")[0]) + for xml_error_elem in new_elem_list: + if not contain_element(xml_error_elem, curr_elem_list): + insert_point.insert(1,xml_error_elem) + numelem = numelem + 1 + curr = curr + 1 + sys.stdout.write('\r') + sys.stdout.write(" %d / %d" % (curr,total)) + sys.stdout.flush() + + sys.stdout.write('\r\n') + print("Done: %d elements" % (numelem)) + return result_xml_root + +def run(): + files = [] + output = None + for i in sys.argv[1:]: + output = i if '.xml' in i else None + files.append(i) if '.cppcheck' in i else None + + result = merge(files) + + if result is None: + return + + if output is not None: + ElementTree.ElementTree(result).write(output) + else: + print(ElementTree.tostring(result).decode('utf-8')) + +if __name__ == '__main__': + run()