[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH] xen/build: Add cppcheck and cppcheck-html make rules


  • To: Bertrand Marquis <bertrand.marquis@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Thu, 24 Mar 2022 13:06:11 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=YNUWIJkr9+cm0ciI1s2TusuUU/Pg8cJ/NaTj0XNs/8s=; b=Mu4MqGyT6vhEBwFOMgd9EjTNxAWiHodfNaI6JbpEQ6Ty+HDxCdsSO83PSzhpmLiuCQuD9WE6PkwInlq/MZd8k6d0ye23vUupUxs34loI58EbmySejk9ezuUCVLSfJTX8AW2WlQhjf2ESKrp6XWk6/8ceFFMj5s/PPDdeqE46WLutGz25v+KtzLbRwLu5FnMAP9CtKf4pGIZomSu/kBVsDGBdVXK15Nod/egeXGswy1nYVbioT7CFCcqLnZHvNSCelu5ou4EoDa0MIdA7QKn1+6GRqJfyWtTk3ZiD04K0xI0XNcnqn4aw3PXNEsr3inYAwtCrUez3PckbBdjcgc84Ig==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=n0V+PGVMW0nYv2AjYpPaeN3+nawqX+iVo+zoTusjp1sdznOxyBdSGfFPIOVe2fdP9P8a45k/n9YA3JXCfCnGrUeC3ICR314mHCOgLGmHvNlvBjWEv2PgLTgr6umG6Wi+ISZKyJdgrL3ay5X2ql3U4tt/5KaDGWb871JPk86Kyks3JcSEQGwBcWkFAbaNQ+3SN7kKJ9XFCv/BNcSEg8vY/Fn46KKlfPnwfW1ig8KxWAaUw0bTWgi4UUt8viSA1NdKFKwuU5OGKxlGfzMVqNplQpfF65BXu2iJ68tIkZ8NyHkgpaJvl2weIhOQfPY3sMHJFe31SEM4hCkLY9NBU7swXg==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: George Dunlap <George.Dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, "Michal Orzel" <michal.orzel@xxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>
  • Delivery-date: Thu, 24 Mar 2022 13:07:18 +0000
  • Ironport-data: A9a23:2Lr/ZKwnAyJjKgkZJPl6t+e5xirEfRIJ4+MujC+fZmUNrF6WrkUHm mIWXmDUaP7fYzf8fYolYI628U8PusXQmtY3SgJpryAxQypGp/SeCIXCJC8cHc8zwu4v7q5Dx 59DAjUVBJlsFhcwnj/0bv656yMUOZigHtIQMsadUsxKbVIiGX9JZS5LwbZj2NYz24ThWWthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ NpltLa3bkAXLo7wnt8tej9DEDhbfoJK0eqSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DFYUToHx/ixreCu4rW8vrSKTW/95Imjw3g6iiGN6AO JJGMGAwPHwsZTVjIWkRJcIVoN2xvVLzVC929A+Vr4Metj27IAtZj+G2bYu9lsaxbdpRtlaVo CTB5WuRKgoBKNWVxD6B83StruzChyX2XMQVDrLQ3vtri12awioUEg8bUXOyu/z/gUm7M/phL EgT9jsrvLIF3kWhRdngXDW1uHeB+BUbXrJ4A+A8rQ2A1KfQywKYHXQfCC5MbsQ8s807TiBs0 UWG9/vjCCZzqrSTRTSY/62NsDKpESEPKCkJYipsZQkY59jupqkjgxSJScxseIa3hNDoHTD7w xiRsTMzwb4UiKY2O76TpA6dxWj2/96QE1Bztl6/sn+ZAh1RZaSiQMuW+Xvn681wBaCCTUCk/ 1cIsp3LhAwRNq2lmCuISeQLObim4feZLTHR6WJS84kdGyeFoCD6I90JiN1qDAIwa5tfJ2e1C KPGkVkJjKK/KkdGekOej2iZL80xhZbtGt3+Phw/RoofO8MhHONrEcwHWKJx44wPuBV0+U3cE c3CGSpJMZr8If44pNZRb71BuYLHPghkmQvuqWnTlnxLK4a2an+PUqsiO1CTdO0/567siFyLr 4YHaZPakEsHDb2WjszrHWg7dwBiwZ8TX82eliCqXrTbfloO9J8JVZc9Po/Ni6Q6xv8Ix48kD 1m2W1NCyUqXuJE0AV7iV5yXU5u2BcwXhStiZUQEZA/0s1B+MdfHxPpOLPMfIOh4nNGPONYpF pHpje3bWa8RItkGkhxABaTAQHtKKEv6317fbnD5CNX9FrY5LzH0FhbfVlKH3AEFDzattNt4p Lul1wjBRoEESRgkB8HTAM9DBXvs4xDxRMoas5P0H+Ru
  • Ironport-hdrordr: A9a23:rmrgbqFzixPtdosvpLqFRpHXdLJyesId70hD6qkvc3Nom52j+/ xGws536fatskdtZJkh8erwXZVp2RvnhNFICPoqTMuftW7dySWVxeBZnMffKljbdREWmdQtrJ uIH5IOa+EYSGIK9/oSgzPIUurIouP3iJxA7N22pxwGLGFXguNbnnxE426gYxZLrWJ9dP4E/e +nl6x6Tk2bCBMqh6qAdxs4dtmGg+eOuIPtYBYACRJiwhKJlymU5LnzFAXd9gsCUhtUqI1SsV Ttokjc3OGOovu7whjT2yv49JJNgubszdNFGYilltUVEDPxkQylDb4RGIFq/QpF4t1H2mxa1O UkkC1QePibLEmhOF1dlCGdnjUIFgxeskMKh2Xo2UcL6vaJOw7SQ/Ax+76xNCGpsXbI9esMoJ 6ilQiixutqJAKFkyLn69fSURZ20kKyvHo5iOYWy2dSSI0EddZq3MciFW5uYd499RjBmcga+S hVfbXhzecTdUnfY2HSv2FpztDpVnMvHg2eSkxHvsCOyTBZkH1w0kNdnaUk7zo93YN4T4MB6/ XPM6xumr0LRsgKbbhlDONERcesEGTCTR/FLWrXK1X6E6MMPW7LtvfMkf8IzfDvfIZNwIo5mZ zHXl8dvWkue1j2AcnLx5FP+gClehTKYd0s8LAo23FUgMyPeFOwC1zxdLkHqbrUn8ki
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYP2703qMuP9Phv0+tczF2jTmG1qzOgP4A
  • Thread-topic: [RFC PATCH] xen/build: Add cppcheck and cppcheck-html make rules

On 24/03/2022 11:04, 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@xxxxxxx>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>

Does CPPCheck have configurable errors vs warnings?  Should we wire this
into CI so we can fail builds which introduce errors that we've already
managed to purge from the codebase?

Also, please include Anthony on future patches, given the extent of
makefile changes.

~Andrew

> ---
>  .gitignore                           |  3 ++
>  xen/Makefile                         | 75 +++++++++++++++++++++++++++-
>  xen/arch/arm/include/asm/processor.h |  4 +-
>  xen/include/xen/config.h             |  4 ++
>  xen/include/xen/kconfig.h            |  5 ++
>  xen/tools/merge_cppcheck_reports.py  | 73 +++++++++++++++++++++++++++
>  6 files changed, 161 insertions(+), 3 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 18a4f7e101..0280d65051 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -336,7 +336,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): %: _% ;
> @@ -424,15 +424,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
> @@ -511,6 +513,75 @@ 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=$(BASEDIR)/include/xen/config.h \
> +                              -I $(BASEDIR)/xsm/flask/include \
> +                              -I $(BASEDIR)/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 %.o,%.c, $(filter-out 
> $(BASEDIR)/tools/%,$(shell find $(BASEDIR) -name "*.o"))))
> +
> +quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(BASEDIR)/%,%,$<)
> +cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \
> +                                            --output-file=$@ $<
> +
> +quiet_cmd_merge_cppcheck_reports = CPPCHECK-MERGE $@
> +cmd_merge_cppcheck_reports = $(BASEDIR)/tools/merge_cppcheck_reports.py $^ $@
> +
> +quiet_cmd_cppcheck_html = CPPCHECK-HTML $<
> +cmd_cppcheck_html = $(CPPCHECK_HTMLREPORT) --file=$< --source-dir=$(BASEDIR) 
> \
> +                                                                             
>    --report-dir=$(CPPCHECK_HTMLREPORT_OUTDIR) \
> +                                                                             
>    --title=Xen
> +
> +PHONY += _cppcheck _cppcheck-html
> +
> +_cppcheck-html: xen-cppcheck.xml
> +     $(call if_changed,cppcheck_html)
> +
> +_cppcheck: xen-cppcheck.xml
> +
> +xen-cppcheck.xml: $(patsubst %.c,%.c.cppcheck,$(CPPCHECKFILES))
> +ifeq ($(CPPCHECKFILES),)
> +     $(error Please build Xen before running cppcheck)
> +endif
> +     $(call if_changed,merge_cppcheck_reports)
> +
> +%.c.cppcheck: %.c $(BASEDIR)/include/generated/autoconf.h 
> $(BASEDIR)/include/generated/compiler-def.h
> +     $(call if_changed,cppcheck_xml)
> +
> +# Put this in generated headers this way it is cleaned by include/Makefile
> +$(BASEDIR)/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..0b4ba73760 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)
>  
> -#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffUL
> +#ifndef CPPCHECK
> +#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffULL
>  #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 b76222ecf6..36e11e7133 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..ef055f6925
> --- /dev/null
> +++ b/xen/tools/merge_cppcheck_reports.py
> @@ -0,0 +1,73 @@
> +#!/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 remove_duplicates(xml_root_element):
> +    elems_to_remove = []
> +    index = 0
> +    elems_list = list(xml_root_element.findall("errors")[0])
> +    for elem1 in elems_list:
> +        index += 1
> +        for elem2 in elems_list[index:]:
> +            if elements_equal(elem1, elem2) and elem2 not in elems_to_remove:
> +                elems_to_remove.append(elem2)
> +                continue
> +
> +    for elem in elems_to_remove:
> +        xml_root_element.findall("errors")[0].remove(elem)
> +
> +def merge(files):
> +    result_xml_root = None
> +    for xml_file in files:
> +        xml_root = ElementTree.parse(xml_file).getroot()
> +        for xml_error_elem in xml_root.iter('errors'):
> +            if result_xml_root is None:
> +                result_xml_root = xml_root
> +                insert_point = result_xml_root.findall("errors")[0]
> +            else:
> +                insert_point.extend(xml_error_elem)
> +
> +    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
> +
> +    remove_duplicates(result)
> +
> +    if output is not None:
> +        ElementTree.ElementTree(result).write(output)
> +    else:
> +        print(ElementTree.tostring(result).decode('utf-8'))
> +
> +if __name__ == '__main__':
> +    run()


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.