[PATCH] xen: Add MISRA support to cppcheck make rule

Bertrand Marquis posted 1 patch 1 year, 11 months ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/5467d017fa1c6721436e21f8cc10c5d74adeb5bf.1654002661.git.bertrand.marquis@arm.com
There is a newer version of this series
.gitignore                     |   2 +
xen/Makefile                   |  32 +++++++-
xen/tools/convert_misra_doc.py | 139 +++++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+), 2 deletions(-)
create mode 100755 xen/tools/convert_misra_doc.py
[PATCH] xen: Add MISRA support to cppcheck make rule
Posted by Bertrand Marquis 1 year, 11 months ago
cppcheck MISRA addon can be used to check for non compliance to some of
the MISRA standard rules.

Add a CPPCHECK_MISRA variable that can be set to "y" using make command
line to generate a cppcheck report including cppcheck misra checks.

When MISRA checking is enabled, a file with a text description suitable
for cppcheck misra addon is generated out of Xen documentation file
which lists the rules followed by Xen (docs/misra/rules.rst).

By default MISRA checking is turned off.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
This change depends on the patch from Stefano Stabellini adding the list
of misra rules in the documentation (docs/misra: introduce rules.rst).
---
 .gitignore                     |   2 +
 xen/Makefile                   |  32 +++++++-
 xen/tools/convert_misra_doc.py | 139 +++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+), 2 deletions(-)
 create mode 100755 xen/tools/convert_misra_doc.py

diff --git a/.gitignore b/.gitignore
index 18ef56a780..65a58ba7e4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -297,6 +297,8 @@ xen/.banner
 xen/.config
 xen/.config.old
 xen/.xen.elf32
+xen/cppcheck-misra.txt
+xen/cppcheck-misra.json
 xen/xen-cppcheck.xml
 xen/System.map
 xen/arch/x86/boot/mkelf32
diff --git a/xen/Makefile b/xen/Makefile
index 82f5310b12..aee6660556 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -548,7 +548,7 @@ _clean:
 	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
+	rm -f cppcheck-misra.json cppcheck-misra.txt xen-cppcheck.xml
 
 .PHONY: _distclean
 _distclean: clean
@@ -642,6 +642,10 @@ CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport
 # build directory. This can be changed by giving a directory in this variable.
 CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport
 
+# By default we do not check misra rules, to enable pass "CPPCHECK_MISRA=y" to
+# make command line.
+CPPCHECK_MISRA ?= n
+
 # 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.
@@ -666,6 +670,15 @@ CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \
                  $(filter-out $(objtree)/tools/%, \
                  $(shell find $(objtree) -name "*.o"))))
 
+# Headers and files required to run cppcheck on a file
+CPPCHECKDEPS := $(objtree)/include/generated/autoconf.h \
+                $(objtree)/include/generated/compiler-def.h
+
+ifeq ($(CPPCHECK_MISRA),y)
+    CPPCHECKFLAGS += --addon=cppcheck-misra.json
+    CPPCHECKDEPS += cppcheck-misra.json
+endif
+
 quiet_cmd_cppcheck_xml = CPPCHECK $(patsubst $(srctree)/%,%,$<)
 cmd_cppcheck_xml = $(CPPCHECK) -v -q --xml $(CPPCHECKFLAGS) \
                    --output-file=$@ $<
@@ -690,7 +703,7 @@ ifeq ($(CPPCHECKFILES),)
 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
+$(objtree)/%.c.cppcheck: $(srctree)/%.c $(CPPCHECKDEPS) | cppcheck-version
 	$(call if_changed,cppcheck_xml)
 
 cppcheck-version:
@@ -703,6 +716,21 @@ cppcheck-version:
 		exit 1; \
 	fi
 
+# List of Misra rules to respect is written inside a doc.
+# In order to have some helpful text in the cppcheck output, generate a text
+# file containing the rules identifier, classification and text from the Xen
+# documentation file. Also generate a json file with the right arguments for
+# cppcheck in json format including the list of rules to ignore
+#
+# Replace current by goal in the dependency to generate an analysis for all
+# rules we would like to respect.
+cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
+	$(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
+		-j cppcheck-misra.json
+
+# Prevent parallel make issues as script is generating both files
+cppcheck-misra.json: cppcheck-misra.txt
+
 # 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
diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py
new file mode 100755
index 0000000000..47133a33a6
--- /dev/null
+++ b/xen/tools/convert_misra_doc.py
@@ -0,0 +1,139 @@
+#!/usr/bin/env python
+
+"""
+This script is converting the misra documentation RST file into a text file
+that can be used as text-rules for cppcheck.
+Usage:
+    convert_misr_doc.py -i INPUT [-o OUTPUT] [-j JSON]
+
+    INPUT  - RST file containing the list of misra rules.
+    OUTPUT - file to store the text output to be used by cppcheck.
+             If not specified, the result will be printed to stdout.
+    JSON   - cppcheck json file to be created (optional).
+"""
+
+import sys, getopt, re
+
+def main(argv):
+    infile = ''
+    outfile = ''
+    outstr = sys.stdout
+    jsonfile = ''
+
+    try:
+        opts, args = getopt.getopt(argv,"hi:o:j:",["input=","output=","json="])
+    except getopt.GetoptError:
+        print('convert-misra.py -i <input> [-o <output>] [-j <json>')
+        sys.exit(2)
+    for opt, arg in opts:
+        if opt == '-h':
+            print('convert-misra.py -i <input> [-o <output>] [-j <json>')
+            print('  If output is not specified, print to stdout')
+            sys.exit(1)
+        elif opt in ("-i", "--input"):
+            infile = arg
+        elif opt in ("-o", "--output"):
+            outfile = arg
+        elif opt in ("-j", "--json"):
+            jsonfile = arg
+
+    try:
+        file_stream = open(infile, 'rt')
+    except:
+        print('Error opening ' + infile)
+        sys.exit(1)
+
+    if outfile:
+        try:
+            outstr = open(outfile, "w")
+        except:
+            print('Error creating ' + outfile)
+            sys.exit(1)
+
+    # Each rule start with '- Rule: Dir'
+    pattern_dir = re.compile(r'^- Rule: Dir ([0-9]+.[0-9]+).*$')
+    pattern_rule = re.compile(r'^- Rule: Rule ([0-9]+.[0-9]+).*$')
+    pattern_severity = re.compile(r'^  - Severity:  (.*)$')
+    pattern_text = re.compile(r'^  - Summary:  (.*)$')
+
+    rule_number = ''
+    rule_state  = 0
+    rule_list = []
+
+    # Start search by cppcheck misra
+    outstr.write('Appendix A Summary of guidelines\n')
+
+    for line in file_stream:
+
+        line = line.replace('\r', '').replace('\n', '')
+
+        if len(line) == 0:
+            continue
+
+        # New rule
+        if rule_state == 0:
+            res = pattern_rule.match(line)
+            if res:
+                rule_state = 1
+                rule_number = res.group(1)
+                rule_list.append(rule_number)
+                continue
+            res = pattern_dir.match(line)
+            if res:
+                rule_state = 1
+                rule_number = res.group(1)
+                rule_list.append(rule_number)
+            continue
+
+        # Severity
+        elif rule_state == 1:
+            res = pattern_severity.match(line)
+            if res:
+                outstr.write('Rule ' + rule_number + ' ' + res.group(1) + '\n')
+                rule_state = 2
+            continue
+
+        # Summary
+        elif rule_state == 2:
+            res = pattern_text.match(line)
+            if res:
+                outstr.write(res.group(1) + ' (Misra rule ' + rule_number
+                             + ')\n')
+                rule_state = 0
+                rule_number = ''
+            continue
+        else:
+            print('Error impossible case !!!')
+
+    skip_list = []
+
+    # Search for missing rules and add a dummy text with the rule number
+    for i in list(range(1,22)):
+        for j in list(range(1,22)):
+            if str(i) + '.' + str(j) not in rule_list:
+                outstr.write('Rule ' + str(i) + '.' + str(j) + '\n')
+                outstr.write('No description for rule ' + str(i) + '.' + str(j)
+                             + '\n')
+                skip_list.append(str(i) + '.' + str(j))
+
+    # Make cppcheck happy by starting the appendix
+    outstr.write('Appendix B\n')
+    outstr.write('\n')
+    if outfile:
+        outstr.close()
+
+    if jsonfile:
+        with open(jsonfile, "w") as f:
+            f.write('{\n')
+            f.write('    "script": "misra.py",\n')
+            f.write('    "args": [\n')
+            if outfile:
+                f.write('      "--rule-texts=' + outfile + '",\n')
+
+            f.write('      "--suppress-rules=' + ",".join(skip_list) + '"\n')
+            f.write('    ]\n')
+            f.write('}\n')
+        f.close()
+
+if __name__ == "__main__":
+   main(sys.argv[1:])
-- 
2.25.1
Re: [PATCH] xen: Add MISRA support to cppcheck make rule
Posted by Jan Beulich 1 year, 11 months ago
On 31.05.2022 15:30, Bertrand Marquis wrote:
> --- a/.gitignore
> +++ b/.gitignore
> @@ -297,6 +297,8 @@ xen/.banner
>  xen/.config
>  xen/.config.old
>  xen/.xen.elf32
> +xen/cppcheck-misra.txt
> +xen/cppcheck-misra.json
>  xen/xen-cppcheck.xml
>  xen/System.map
>  xen/arch/x86/boot/mkelf32

Please can you obey to sorting here, inserting next to
xen/cppcheck-htmlreport? Seeing that xen/xen-cppcheck.xml was added
here by you at the same time as xen/cppcheck-htmlreport, may I further
ask that you move that line to where it belongs as well?

Additionally I wonder if you couldn't use just one line, specifying
xen/cppcheck-misra.* ; this could then also hold for the _clean rule
addition further down in the patch.

> @@ -703,6 +716,21 @@ cppcheck-version:
>  		exit 1; \
>  	fi
>  
> +# List of Misra rules to respect is written inside a doc.
> +# In order to have some helpful text in the cppcheck output, generate a text
> +# file containing the rules identifier, classification and text from the Xen
> +# documentation file. Also generate a json file with the right arguments for
> +# cppcheck in json format including the list of rules to ignore

Nit: Missing full stop at the end.

> +# Replace current by goal in the dependency to generate an analysis for all
> +# rules we would like to respect.
> +cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
> +	$(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
> +		-j cppcheck-misra.json
> +
> +# Prevent parallel make issues as script is generating both files
> +cppcheck-misra.json: cppcheck-misra.txt

With this dependency the earlier rule should not list multiple targets
(and it generally should not, for not being a pattern rule). If I'm not
mistaken the way you have it the Python script would be invoked twice,
and all you prevent is that it is invoked twice in parallel. And then
please use $@ inside the rule. Additionally, with the script being an
in-tree one, the rule should also have a dependency on that script
(such that the targets would be rebuilt if the script alone changes).

Jan
Re: [PATCH] xen: Add MISRA support to cppcheck make rule
Posted by Bertrand Marquis 1 year, 11 months ago
Hi Jan,

> On 31 May 2022, at 14:50, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 31.05.2022 15:30, Bertrand Marquis wrote:
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -297,6 +297,8 @@ xen/.banner
>> xen/.config
>> xen/.config.old
>> xen/.xen.elf32
>> +xen/cppcheck-misra.txt
>> +xen/cppcheck-misra.json
>> xen/xen-cppcheck.xml
>> xen/System.map
>> xen/arch/x86/boot/mkelf32
> 
> Please can you obey to sorting here, inserting next to
> xen/cppcheck-htmlreport? Seeing that xen/xen-cppcheck.xml was added
> here by you at the same time as xen/cppcheck-htmlreport, may I further
> ask that you move that line to where it belongs as well?

Sure will do.

> 
> Additionally I wonder if you couldn't use just one line, specifying
> xen/cppcheck-misra.* ; this could then also hold for the _clean rule
> addition further down in the patch.

Ok.

> 
>> @@ -703,6 +716,21 @@ cppcheck-version:
>> 		exit 1; \
>> 	fi
>> 
>> +# List of Misra rules to respect is written inside a doc.
>> +# In order to have some helpful text in the cppcheck output, generate a text
>> +# file containing the rules identifier, classification and text from the Xen
>> +# documentation file. Also generate a json file with the right arguments for
>> +# cppcheck in json format including the list of rules to ignore
> 
> Nit: Missing full stop at the end.

Will fix

> 
>> +# Replace current by goal in the dependency to generate an analysis for all
>> +# rules we would like to respect.
>> +cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
>> +	$(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
>> +		-j cppcheck-misra.json
>> +
>> +# Prevent parallel make issues as script is generating both files
>> +cppcheck-misra.json: cppcheck-misra.txt
> 
> With this dependency the earlier rule should not list multiple targets
> (and it generally should not, for not being a pattern rule). If I'm not
> mistaken the way you have it the Python script would be invoked twice,
> and all you prevent is that it is invoked twice in parallel. And then
> please use $@ inside the rule. Additionally, with the script being an
> in-tree one, the rule should also have a dependency on that script
> (such that the targets would be rebuilt if the script alone changes).

I am a bit lost on the $@, previous patch adding cppcheck was changed
to use $(Q) instead. So can you justify this request ?

For the rest, thanks for the finding, I agree and will fix.

Cheers
Bertrand

> 
> Jan
> 
Re: [PATCH] xen: Add MISRA support to cppcheck make rule
Posted by Jan Beulich 1 year, 11 months ago
On 31.05.2022 16:14, Bertrand Marquis wrote:
>> On 31 May 2022, at 14:50, Jan Beulich <jbeulich@suse.com> wrote:
>> On 31.05.2022 15:30, Bertrand Marquis wrote:
>>> +# Replace current by goal in the dependency to generate an analysis for all
>>> +# rules we would like to respect.
>>> +cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
>>> +	$(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
>>> +		-j cppcheck-misra.json
>>> +
>>> +# Prevent parallel make issues as script is generating both files
>>> +cppcheck-misra.json: cppcheck-misra.txt
>>
>> With this dependency the earlier rule should not list multiple targets
>> (and it generally should not, for not being a pattern rule). If I'm not
>> mistaken the way you have it the Python script would be invoked twice,
>> and all you prevent is that it is invoked twice in parallel. And then
>> please use $@ inside the rule. Additionally, with the script being an
>> in-tree one, the rule should also have a dependency on that script
>> (such that the targets would be rebuilt if the script alone changes).
> 
> I am a bit lost on the $@, previous patch adding cppcheck was changed
> to use $(Q) instead. So can you justify this request ?

I'm talking of $@ (a macro expansion), not @ as a prefix to suppress
command echoing.

Jan
Re: [PATCH] xen: Add MISRA support to cppcheck make rule
Posted by Bertrand Marquis 1 year, 11 months ago
Hi,

> On 31 May 2022, at 15:28, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 31.05.2022 16:14, Bertrand Marquis wrote:
>>> On 31 May 2022, at 14:50, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 31.05.2022 15:30, Bertrand Marquis wrote:
>>>> +# Replace current by goal in the dependency to generate an analysis for all
>>>> +# rules we would like to respect.
>>>> +cppcheck-misra.json cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst
>>>> +	$(Q)$(srctree)/tools/convert_misra_doc.py -i $< -o cppcheck-misra.txt \
>>>> +		-j cppcheck-misra.json
>>>> +
>>>> +# Prevent parallel make issues as script is generating both files
>>>> +cppcheck-misra.json: cppcheck-misra.txt
>>> 
>>> With this dependency the earlier rule should not list multiple targets
>>> (and it generally should not, for not being a pattern rule). If I'm not
>>> mistaken the way you have it the Python script would be invoked twice,
>>> and all you prevent is that it is invoked twice in parallel. And then
>>> please use $@ inside the rule. Additionally, with the script being an
>>> in-tree one, the rule should also have a dependency on that script
>>> (such that the targets would be rebuilt if the script alone changes).
>> 
>> I am a bit lost on the $@, previous patch adding cppcheck was changed
>> to use $(Q) instead. So can you justify this request ?
> 
> I'm talking of $@ (a macro expansion), not @ as a prefix to suppress
> command echoing.
> 

That make a lot more sense.

Thanks
Bertrand

> Jan
> 
>