Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] xen/misra: create exclusion file list
@ 2023-03-03  9:38 Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan Luca Fancellu
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Luca Fancellu @ 2023-03-03  9:38 UTC (permalink / raw
  To: xen-devel
  Cc: bertrand.marquis, wei.chen, Andrew Cooper, George Dunlap,
	Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu

This serie is introducing an exclusion list for the misra analysis, at the
moment only cppcheck can benefit from it because it's the tool where we
control every step and configuration.

Exclude a file from the analysis is the last step we should do, but sometime it
is unavoidable because we can't do direct changes to it to fix/deviate from
findings.
So we declare the file(s) here and we leave the burden of the misra compliance
to the final user.

Luca Fancellu (3):
  xen/cppcheck: add a way to exclude files from the scan
  xen/misra: add entries to exclude-list.json
  xen/cppcheck: globally suppress unusedStructMember

 docs/misra/exclude-list.json                  | 201 ++++++++++++++++++
 docs/misra/exclude-list.rst                   |  46 ++++
 xen/scripts/xen_analysis/cppcheck_analysis.py |  23 +-
 .../xen_analysis/exclusion_file_list.py       |  70 ++++++
 4 files changed, 338 insertions(+), 2 deletions(-)
 create mode 100644 docs/misra/exclude-list.json
 create mode 100644 docs/misra/exclude-list.rst
 create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py

-- 
2.34.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan
  2023-03-03  9:38 [PATCH v3 0/3] xen/misra: create exclusion file list Luca Fancellu
@ 2023-03-03  9:38 ` Luca Fancellu
  2023-03-03 20:54   ` Stefano Stabellini
  2023-03-03  9:38 ` [PATCH v3 2/3] xen/misra: add entries to exclude-list.json Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember Luca Fancellu
  2 siblings, 1 reply; 7+ messages in thread
From: Luca Fancellu @ 2023-03-03  9:38 UTC (permalink / raw
  To: xen-devel
  Cc: bertrand.marquis, wei.chen, Andrew Cooper, George Dunlap,
	Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu

Add a way to exclude files from the scan, in this way we can skip
some findings from the report on those files that Xen doesn't own.

To do that, introduce the exclude-list.json file under docs/misra,
this file will be populated with relative path to the files/folder
to be excluded.
Introduce a new module, exclusion_file_list.py, to deal with the
exclusion list file and use the new module in cppcheck_analysis.py
to take a list of excluded paths to update the suppression list of
cppcheck.
Modified --suppress flag for cppcheck tool to remove
unmatchedSuppression findings for those external file that are
listed but for example not built for the current architecture.

Add documentation for the file.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from v2:
 - enforce excluded folder path to end with '/*', so update docs
   and drop the code that handled (badly) that case from
   exclusion_file_list.py, __cppcheck_path_exclude_syntax(...)
   (Stefano)
Changes from v1:
 - Indentation fixed (Jan)
---
---
 docs/misra/exclude-list.json                  |  4 ++
 docs/misra/exclude-list.rst                   | 46 ++++++++++++
 xen/scripts/xen_analysis/cppcheck_analysis.py | 20 +++++-
 .../xen_analysis/exclusion_file_list.py       | 70 +++++++++++++++++++
 4 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 docs/misra/exclude-list.json
 create mode 100644 docs/misra/exclude-list.rst
 create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
new file mode 100644
index 000000000000..1fb0ac67747b
--- /dev/null
+++ b/docs/misra/exclude-list.json
@@ -0,0 +1,4 @@
+{
+    "version": "1.0",
+    "content": []
+}
diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
new file mode 100644
index 000000000000..c97431a86120
--- /dev/null
+++ b/docs/misra/exclude-list.rst
@@ -0,0 +1,46 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Exclude file list for xen-analysis script
+=========================================
+
+The code analysis is performed on the Xen codebase for both MISRA
+checkers and static analysis checkers, there are some files however that
+needs to be removed from the findings report for various reasons (e.g.
+they are imported from external sources, they generate too many false
+positive results, etc.).
+
+For this reason the file docs/misra/exclude-list.json is used to exclude every
+entry listed in that file from the final report.
+Currently only the cppcheck analysis will use this file.
+
+Here is an example of the exclude-list.json file::
+
+|{
+|    "version": "1.0",
+|    "content": [
+|        {
+|            "rel_path": "relative/path/from/xen/file",
+|            "comment": "This file is originated from ..."
+|        },
+|        {
+|            "rel_path": "relative/path/from/xen/folder/*",
+|            "comment": "This folder is a library"
+|        },
+|        {
+|            "rel_path": "relative/path/from/xen/mem*.c",
+|            "comment": "memcpy.c, memory.c and memcmp.c are from the outside"
+|        }
+|    ]
+|}
+
+Here is an explanation of the fields inside an object of the "content" array:
+ - rel_path: it is the relative path from the Xen folder to the file/folder that
+   needs to be excluded from the analysis report, it can contain a wildcard to
+   match more than one file/folder at the time. This field is mandatory.
+ - comment: an optional comment to explain why the file is removed from the
+   analysis.
+
+To ease the review and the modifications of the entries, they shall be listed in
+alphabetical order referring to the rel_path field.
+Excluded folder paths shall end with '/*' in order to match everything on that
+folder.
diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py
index cc1f403d315e..e385e2c8f54a 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -1,7 +1,8 @@
 #!/usr/bin/env python3
 
 import os, re, shutil
-from . import settings, utils, cppcheck_report_utils
+from . import settings, utils, cppcheck_report_utils, exclusion_file_list
+from .exclusion_file_list import ExclusionFileListError
 
 class GetMakeVarsPhaseError(Exception):
     pass
@@ -50,6 +51,21 @@ def __generate_suppression_list(out_file):
             # header for cppcheck
             supplist_file.write("*:*generated/compiler-def.h\n")
 
+            try:
+                exclusion_file = \
+                    "{}/docs/misra/exclude-list.json".format(settings.repo_dir)
+                exclusion_list = \
+                    exclusion_file_list.load_exclusion_file_list(exclusion_file)
+            except ExclusionFileListError as e:
+                raise CppcheckDepsPhaseError(
+                    "Issue with reading file {}: {}".format(exclusion_file, e)
+                )
+
+            # Append excluded files to the suppression list, using * as error id
+            # to suppress every findings on that file
+            for path in exclusion_list:
+                supplist_file.write("*:{}\n".format(path))
+
             for entry in headers:
                 filename = entry["file"]
                 try:
@@ -128,7 +144,7 @@ def generate_cppcheck_deps():
  --relative-paths={}
  --inline-suppr
  --suppressions-list={}/suppression-list.txt
- --suppress='unmatchedSuppression:*generated/compiler-def.h'
+ --suppress='unmatchedSuppression:*'
  --include={}/include/xen/config.h
  -DCPPCHECK
 """.format(settings.outdir, CPPCHECK_BUILD_DIR, settings.xen_dir,
diff --git a/xen/scripts/xen_analysis/exclusion_file_list.py b/xen/scripts/xen_analysis/exclusion_file_list.py
new file mode 100644
index 000000000000..871e480586bb
--- /dev/null
+++ b/xen/scripts/xen_analysis/exclusion_file_list.py
@@ -0,0 +1,70 @@
+#!/usr/bin/env python3
+
+import os, glob, json
+from . import settings
+
+class ExclusionFileListError(Exception):
+    pass
+
+
+def __cppcheck_path_exclude_syntax(path):
+    # Prepending * to the relative path to match every path where the Xen
+    # codebase could be
+    path = "*" + path
+
+    return path
+
+
+# Reads the exclusion file list and returns a list of relative path to be
+# excluded.
+def load_exclusion_file_list(input_file):
+    ret = []
+    try:
+        with open(input_file, "rt") as handle:
+            content = json.load(handle)
+            entries = content['content']
+    except json.JSONDecodeError as e:
+        raise ExclusionFileListError(
+                "JSON decoding error in file {}: {}".format(input_file, e)
+        )
+    except KeyError:
+        raise ExclusionFileListError(
+            "Malformed JSON file: content field not found!"
+        )
+    except Exception as e:
+        raise ExclusionFileListError(
+                "Can't open file {}: {}".format(input_file, e)
+        )
+
+    for entry in entries:
+        try:
+            path = entry['rel_path']
+        except KeyError:
+            raise ExclusionFileListError(
+                "Malformed JSON entry: rel_path field not found!"
+            )
+        abs_path = settings.xen_dir + "/" + path
+        check_path = [abs_path]
+
+        # If the path contains wildcards, solve them
+        if '*' in abs_path:
+            check_path = glob.glob(abs_path)
+
+        # Check that the path exists
+        for filepath_object in check_path:
+            if not os.path.exists(filepath_object):
+                raise ExclusionFileListError(
+                    "Malformed path: {} refers to {} that does not exists"
+                    .format(path, filepath_object)
+                )
+
+        if settings.analysis_tool == "cppcheck":
+            path = __cppcheck_path_exclude_syntax(path)
+        else:
+            raise ExclusionFileListError(
+                "Unimplemented for {}!".format(settings.analysis_tool)
+            )
+
+        ret.append(path)
+
+    return ret
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] xen/misra: add entries to exclude-list.json
  2023-03-03  9:38 [PATCH v3 0/3] xen/misra: create exclusion file list Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan Luca Fancellu
@ 2023-03-03  9:38 ` Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember Luca Fancellu
  2 siblings, 0 replies; 7+ messages in thread
From: Luca Fancellu @ 2023-03-03  9:38 UTC (permalink / raw
  To: xen-devel
  Cc: bertrand.marquis, wei.chen, Andrew Cooper, George Dunlap,
	Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu,
	Michal Orzel

Add entries to the exclude-list.json for those files that need to be
excluded from the analysis scan.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
This list is originated from Michal's work here:
https://patchwork.kernel.org/project/xen-devel/patch/20221116092032.4423-1-michal.orzel@amd.com/#25123099
and changed to adapt to this task.
Changes from v2:
 - Add Stefano's R-by
Changes from v1:
 - updated list with new files from Stefano
 - add comment field for every entry (Jan)
---
---
 docs/misra/exclude-list.json | 199 ++++++++++++++++++++++++++++++++++-
 1 file changed, 198 insertions(+), 1 deletion(-)

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 1fb0ac67747b..ca1e2dd678ff 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -1,4 +1,201 @@
 {
     "version": "1.0",
-    "content": []
+    "content": [
+        {
+            "rel_path": "arch/arm/arm64/cpufeature.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/arm/arm64/insn.c",
+            "comment": "Imported on Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/acpi/boot.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/acpi/cpu_idle.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/acpi/cpufreq/cpufreq.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/acpi/cpuidle_menu.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/acpi/lib.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/amd.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/centaur.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/common.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/hygon.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/intel.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/intel_cacheinfo.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/mcheck/non-fatal.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/mtrr/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/cpu/mwait-idle.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/delay.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/dmi_scan.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/mpparse.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/srat.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/time.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/bitmap.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/bunzip2.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/earlycpio.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/inflate.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/libfdt/*",
+            "comment": "External library"
+        },
+        {
+            "rel_path": "common/livepatch_elf.c",
+            "comment": "Not in scope initially as it generates many violations and it is not enabled in safety configurations"
+        },
+        {
+            "rel_path": "common/lzo.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/lz4/decompress.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/radix-tree.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/ubsan/ubsan.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/un*.c",
+            "comment": "unlz4.c implementation by Yann Collet, the others un* are from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/xz/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "common/zstd/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "crypto/*",
+            "comment": "Origin is external and documented in crypto/README.source"
+        },
+        {
+            "rel_path": "drivers/acpi/apei/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/hwregs.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/numa.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/osl.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/tables.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/tables/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/acpi/utilities/*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "drivers/video/font_*",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "lib/list-sort.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "lib/rbtree.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "lib/xxhash*.c",
+            "comment": "Imported from Linux, ignore for now"
+        },
+        {
+            "rel_path": "xsm/flask/*",
+            "comment": "Not in scope initially as it generates many violations and it is not enabled in safety configurations"
+        }
+    ]
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember
  2023-03-03  9:38 [PATCH v3 0/3] xen/misra: create exclusion file list Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan Luca Fancellu
  2023-03-03  9:38 ` [PATCH v3 2/3] xen/misra: add entries to exclude-list.json Luca Fancellu
@ 2023-03-03  9:38 ` Luca Fancellu
  2023-03-03 20:55   ` Stefano Stabellini
  2 siblings, 1 reply; 7+ messages in thread
From: Luca Fancellu @ 2023-03-03  9:38 UTC (permalink / raw
  To: xen-devel
  Cc: bertrand.marquis, wei.chen, Andrew Cooper, George Dunlap,
	Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu

unusedStructMember warnings from cppcheck are not reliable and
are causing a lot of false positives, suppress the checker
globally for now.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from v2:
 - New patch
---
 xen/scripts/xen_analysis/cppcheck_analysis.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py
index e385e2c8f54a..ab52ce38d502 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -133,6 +133,8 @@ def generate_cppcheck_deps():
     # - Explicitly suppress warnings on compiler-def.h because cppcheck throws
     #   an unmatchedSuppression due to the rule we put in suppression-list.txt
     #   to skip every finding in the file.
+    # - Explicitly suppress findings for unusedStructMember that is not very
+    #   reliable and causes lots of false positives.
     #
     # Compiler defines are in compiler-def.h which is included in config.h
     #
@@ -145,6 +147,7 @@ def generate_cppcheck_deps():
  --inline-suppr
  --suppressions-list={}/suppression-list.txt
  --suppress='unmatchedSuppression:*'
+ --suppress='unusedStructMember:*'
  --include={}/include/xen/config.h
  -DCPPCHECK
 """.format(settings.outdir, CPPCHECK_BUILD_DIR, settings.xen_dir,
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan
  2023-03-03  9:38 ` [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan Luca Fancellu
@ 2023-03-03 20:54   ` Stefano Stabellini
  0 siblings, 0 replies; 7+ messages in thread
From: Stefano Stabellini @ 2023-03-03 20:54 UTC (permalink / raw
  To: Luca Fancellu
  Cc: xen-devel, bertrand.marquis, wei.chen, Andrew Cooper,
	George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini,
	Wei Liu

On Fri, 3 Mar 2023, Luca Fancellu wrote:
> Add a way to exclude files from the scan, in this way we can skip
> some findings from the report on those files that Xen doesn't own.
> 
> To do that, introduce the exclude-list.json file under docs/misra,
> this file will be populated with relative path to the files/folder
> to be excluded.
> Introduce a new module, exclusion_file_list.py, to deal with the
> exclusion list file and use the new module in cppcheck_analysis.py
> to take a list of excluded paths to update the suppression list of
> cppcheck.
> Modified --suppress flag for cppcheck tool to remove
> unmatchedSuppression findings for those external file that are
> listed but for example not built for the current architecture.
> 
> Add documentation for the file.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes from v2:
>  - enforce excluded folder path to end with '/*', so update docs
>    and drop the code that handled (badly) that case from
>    exclusion_file_list.py, __cppcheck_path_exclude_syntax(...)
>    (Stefano)
> Changes from v1:
>  - Indentation fixed (Jan)
> ---
> ---
>  docs/misra/exclude-list.json                  |  4 ++
>  docs/misra/exclude-list.rst                   | 46 ++++++++++++
>  xen/scripts/xen_analysis/cppcheck_analysis.py | 20 +++++-
>  .../xen_analysis/exclusion_file_list.py       | 70 +++++++++++++++++++
>  4 files changed, 138 insertions(+), 2 deletions(-)
>  create mode 100644 docs/misra/exclude-list.json
>  create mode 100644 docs/misra/exclude-list.rst
>  create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py
> 
> diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
> new file mode 100644
> index 000000000000..1fb0ac67747b
> --- /dev/null
> +++ b/docs/misra/exclude-list.json
> @@ -0,0 +1,4 @@
> +{
> +    "version": "1.0",
> +    "content": []
> +}
> diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
> new file mode 100644
> index 000000000000..c97431a86120
> --- /dev/null
> +++ b/docs/misra/exclude-list.rst
> @@ -0,0 +1,46 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Exclude file list for xen-analysis script
> +=========================================
> +
> +The code analysis is performed on the Xen codebase for both MISRA
> +checkers and static analysis checkers, there are some files however that
> +needs to be removed from the findings report for various reasons (e.g.
> +they are imported from external sources, they generate too many false
> +positive results, etc.).
> +
> +For this reason the file docs/misra/exclude-list.json is used to exclude every
> +entry listed in that file from the final report.
> +Currently only the cppcheck analysis will use this file.
> +
> +Here is an example of the exclude-list.json file::
> +
> +|{
> +|    "version": "1.0",
> +|    "content": [
> +|        {
> +|            "rel_path": "relative/path/from/xen/file",
> +|            "comment": "This file is originated from ..."
> +|        },
> +|        {
> +|            "rel_path": "relative/path/from/xen/folder/*",
> +|            "comment": "This folder is a library"
> +|        },
> +|        {
> +|            "rel_path": "relative/path/from/xen/mem*.c",
> +|            "comment": "memcpy.c, memory.c and memcmp.c are from the outside"
> +|        }
> +|    ]
> +|}
> +
> +Here is an explanation of the fields inside an object of the "content" array:
> + - rel_path: it is the relative path from the Xen folder to the file/folder that
> +   needs to be excluded from the analysis report, it can contain a wildcard to
> +   match more than one file/folder at the time. This field is mandatory.
> + - comment: an optional comment to explain why the file is removed from the
> +   analysis.
> +
> +To ease the review and the modifications of the entries, they shall be listed in
> +alphabetical order referring to the rel_path field.
> +Excluded folder paths shall end with '/*' in order to match everything on that
> +folder.
> diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py
> index cc1f403d315e..e385e2c8f54a 100644
> --- a/xen/scripts/xen_analysis/cppcheck_analysis.py
> +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
> @@ -1,7 +1,8 @@
>  #!/usr/bin/env python3
>  
>  import os, re, shutil
> -from . import settings, utils, cppcheck_report_utils
> +from . import settings, utils, cppcheck_report_utils, exclusion_file_list
> +from .exclusion_file_list import ExclusionFileListError
>  
>  class GetMakeVarsPhaseError(Exception):
>      pass
> @@ -50,6 +51,21 @@ def __generate_suppression_list(out_file):
>              # header for cppcheck
>              supplist_file.write("*:*generated/compiler-def.h\n")
>  
> +            try:
> +                exclusion_file = \
> +                    "{}/docs/misra/exclude-list.json".format(settings.repo_dir)
> +                exclusion_list = \
> +                    exclusion_file_list.load_exclusion_file_list(exclusion_file)
> +            except ExclusionFileListError as e:
> +                raise CppcheckDepsPhaseError(
> +                    "Issue with reading file {}: {}".format(exclusion_file, e)
> +                )
> +
> +            # Append excluded files to the suppression list, using * as error id
> +            # to suppress every findings on that file
> +            for path in exclusion_list:
> +                supplist_file.write("*:{}\n".format(path))
> +
>              for entry in headers:
>                  filename = entry["file"]
>                  try:
> @@ -128,7 +144,7 @@ def generate_cppcheck_deps():
>   --relative-paths={}
>   --inline-suppr
>   --suppressions-list={}/suppression-list.txt
> - --suppress='unmatchedSuppression:*generated/compiler-def.h'
> + --suppress='unmatchedSuppression:*'
>   --include={}/include/xen/config.h
>   -DCPPCHECK
>  """.format(settings.outdir, CPPCHECK_BUILD_DIR, settings.xen_dir,
> diff --git a/xen/scripts/xen_analysis/exclusion_file_list.py b/xen/scripts/xen_analysis/exclusion_file_list.py
> new file mode 100644
> index 000000000000..871e480586bb
> --- /dev/null
> +++ b/xen/scripts/xen_analysis/exclusion_file_list.py
> @@ -0,0 +1,70 @@
> +#!/usr/bin/env python3
> +
> +import os, glob, json
> +from . import settings
> +
> +class ExclusionFileListError(Exception):
> +    pass
> +
> +
> +def __cppcheck_path_exclude_syntax(path):
> +    # Prepending * to the relative path to match every path where the Xen
> +    # codebase could be
> +    path = "*" + path
> +
> +    return path
> +
> +
> +# Reads the exclusion file list and returns a list of relative path to be
> +# excluded.
> +def load_exclusion_file_list(input_file):
> +    ret = []
> +    try:
> +        with open(input_file, "rt") as handle:
> +            content = json.load(handle)
> +            entries = content['content']
> +    except json.JSONDecodeError as e:
> +        raise ExclusionFileListError(
> +                "JSON decoding error in file {}: {}".format(input_file, e)
> +        )
> +    except KeyError:
> +        raise ExclusionFileListError(
> +            "Malformed JSON file: content field not found!"
> +        )
> +    except Exception as e:
> +        raise ExclusionFileListError(
> +                "Can't open file {}: {}".format(input_file, e)
> +        )
> +
> +    for entry in entries:
> +        try:
> +            path = entry['rel_path']
> +        except KeyError:
> +            raise ExclusionFileListError(
> +                "Malformed JSON entry: rel_path field not found!"
> +            )
> +        abs_path = settings.xen_dir + "/" + path
> +        check_path = [abs_path]
> +
> +        # If the path contains wildcards, solve them
> +        if '*' in abs_path:
> +            check_path = glob.glob(abs_path)
> +
> +        # Check that the path exists
> +        for filepath_object in check_path:
> +            if not os.path.exists(filepath_object):
> +                raise ExclusionFileListError(
> +                    "Malformed path: {} refers to {} that does not exists"
> +                    .format(path, filepath_object)
> +                )
> +
> +        if settings.analysis_tool == "cppcheck":
> +            path = __cppcheck_path_exclude_syntax(path)
> +        else:
> +            raise ExclusionFileListError(
> +                "Unimplemented for {}!".format(settings.analysis_tool)
> +            )
> +
> +        ret.append(path)
> +
> +    return ret
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember
  2023-03-03  9:38 ` [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember Luca Fancellu
@ 2023-03-03 20:55   ` Stefano Stabellini
  2023-03-04  7:44     ` Luca Fancellu
  0 siblings, 1 reply; 7+ messages in thread
From: Stefano Stabellini @ 2023-03-03 20:55 UTC (permalink / raw
  To: Luca Fancellu
  Cc: xen-devel, bertrand.marquis, wei.chen, Andrew Cooper,
	George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini,
	Wei Liu

On Fri, 3 Mar 2023, Luca Fancellu wrote:
> unusedStructMember warnings from cppcheck are not reliable and
> are causing a lot of false positives, suppress the checker
> globally for now.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

This is great! Results are much cleaner. With this series accepted, I
think we can start looking into how to "diff" cppcheck results to spot
regressions in new patches.


> ---
> Changes from v2:
>  - New patch
> ---
>  xen/scripts/xen_analysis/cppcheck_analysis.py | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py
> index e385e2c8f54a..ab52ce38d502 100644
> --- a/xen/scripts/xen_analysis/cppcheck_analysis.py
> +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
> @@ -133,6 +133,8 @@ def generate_cppcheck_deps():
>      # - Explicitly suppress warnings on compiler-def.h because cppcheck throws
>      #   an unmatchedSuppression due to the rule we put in suppression-list.txt
>      #   to skip every finding in the file.
> +    # - Explicitly suppress findings for unusedStructMember that is not very
> +    #   reliable and causes lots of false positives.
>      #
>      # Compiler defines are in compiler-def.h which is included in config.h
>      #
> @@ -145,6 +147,7 @@ def generate_cppcheck_deps():
>   --inline-suppr
>   --suppressions-list={}/suppression-list.txt
>   --suppress='unmatchedSuppression:*'
> + --suppress='unusedStructMember:*'
>   --include={}/include/xen/config.h
>   -DCPPCHECK
>  """.format(settings.outdir, CPPCHECK_BUILD_DIR, settings.xen_dir,
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember
  2023-03-03 20:55   ` Stefano Stabellini
@ 2023-03-04  7:44     ` Luca Fancellu
  0 siblings, 0 replies; 7+ messages in thread
From: Luca Fancellu @ 2023-03-04  7:44 UTC (permalink / raw
  To: Stefano Stabellini
  Cc: Xen-devel, Bertrand Marquis, Wei Chen, Andrew Cooper,
	George Dunlap, Jan Beulich, Julien Grall, Wei Liu



> On 3 Mar 2023, at 20:55, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Fri, 3 Mar 2023, Luca Fancellu wrote:
>> unusedStructMember warnings from cppcheck are not reliable and
>> are causing a lot of false positives, suppress the checker
>> globally for now.
>> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> This is great! Results are much cleaner. With this series accepted, I
> think we can start looking into how to "diff" cppcheck results to spot
> regressions in new patches.

Yes indeed! I have some ideas about the diff, I’ll present something at the
FuSa meeting

> 
> 
>> ---
>> Changes from v2:
>> - New patch
>> ---
>> xen/scripts/xen_analysis/cppcheck_analysis.py | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py
>> index e385e2c8f54a..ab52ce38d502 100644
>> --- a/xen/scripts/xen_analysis/cppcheck_analysis.py
>> +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
>> @@ -133,6 +133,8 @@ def generate_cppcheck_deps():
>>     # - Explicitly suppress warnings on compiler-def.h because cppcheck throws
>>     #   an unmatchedSuppression due to the rule we put in suppression-list.txt
>>     #   to skip every finding in the file.
>> +    # - Explicitly suppress findings for unusedStructMember that is not very
>> +    #   reliable and causes lots of false positives.
>>     #
>>     # Compiler defines are in compiler-def.h which is included in config.h
>>     #
>> @@ -145,6 +147,7 @@ def generate_cppcheck_deps():
>>  --inline-suppr
>>  --suppressions-list={}/suppression-list.txt
>>  --suppress='unmatchedSuppression:*'
>> + --suppress='unusedStructMember:*'
>>  --include={}/include/xen/config.h
>>  -DCPPCHECK
>> """.format(settings.outdir, CPPCHECK_BUILD_DIR, settings.xen_dir,
>> -- 
>> 2.34.1
>> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-04  7:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-03  9:38 [PATCH v3 0/3] xen/misra: create exclusion file list Luca Fancellu
2023-03-03  9:38 ` [PATCH v3 1/3] xen/cppcheck: add a way to exclude files from the scan Luca Fancellu
2023-03-03 20:54   ` Stefano Stabellini
2023-03-03  9:38 ` [PATCH v3 2/3] xen/misra: add entries to exclude-list.json Luca Fancellu
2023-03-03  9:38 ` [PATCH v3 3/3] xen/cppcheck: globally suppress unusedStructMember Luca Fancellu
2023-03-03 20:55   ` Stefano Stabellini
2023-03-04  7:44     ` Luca Fancellu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).