1.. SPDX-License-Identifier: CC-BY-4.0
2
3Documenting violations
4======================
5
6Static analysers are used on the Xen codebase for both static analysis and MISRA
7compliance.
8There might be the need to suppress some findings instead of fixing them and
9many tools permit the usage of in-code comments that suppress findings so that
10they are not shown in the final report.
11
12Xen includes a tool capable of translating a specific comment used in its
13codebase to the right proprietary in-code comment understandable by the selected
14analyser that suppress its finding.
15
16In the Xen codebase, these tags will be used to document and suppress findings:
17
18 - SAF-X-safe: This tag means that the next line of code contains a finding, but
19   the non compliance to the checker is analysed and demonstrated to be safe.
20 - SAF-X-false-positive-<tool>: This tag means that the next line of code
21   contains a finding, but the finding is a bug of the tool.
22
23SAF stands for Static Analyser Finding, the X is a placeholder for a positive
24number that starts from zero, the number after SAF- shall be incremental and
25unique, base ten notation and without leading zeros.
26
27Entries in the database shall never be removed, even if they are not used
28anymore in the code (if a patch is removing or modifying the faulty line).
29This is to make sure that numbers are not reused which could lead to conflicts
30with old branches or misleading justifications.
31
32An entry can be reused in multiple places in the code to suppress a finding if
33and only if the justification holds for the same non-compliance to the coding
34standard.
35
36An orphan entry, that is an entry who was justifying a finding in the code, but
37later that code was removed and there is no other use of that entry in the code,
38can be reused as long as the justification for the finding holds. This is done
39to avoid the allocation of a new entry with exactly the same justification, that
40would lead to waste of space and maintenance issues of the database.
41
42The files where to store all the justifications are in xen/docs/misra/ and are
43named as safe.json and false-positive-<tool>.json, they have JSON format, each
44one has a different justification schema which shares some fields.
45
46Here is an example to add a new justification in safe.json::
47
48|{
49|    "version": "1.0",
50|    "content": [
51|        {
52|            "id": "SAF-0-safe",
53|            "analyser": {
54|                "cppcheck": "misra-c2012-20.7",
55|                "coverity": "misra_c_2012_rule_20_7_violation",
56|                "eclair": "MC3A2.R20.7"
57|            },
58|            "name": "R20.7 C macro parameters not used as expression",
59|            "text": "The macro parameters used in this [...]"
60|        },
61|        {
62|            "id": "SAF-1-safe",
63|            "analyser": {},
64|            "name": "Sentinel",
65|            "text": "Next ID to be used"
66|        }
67|    ]
68|}
69
70To document a finding in safe.json, just add another block {[...]} before the
71sentinel block, using the id contained in the sentinel block and increment by
72one the number contained in the id of the sentinel block.
73
74Here is an explanation of the fields inside an object of the "content" array:
75 - id: it is a unique string that is used to refer to the finding, many finding
76   can be tagged with the same id, if the justification holds for any applied
77   case.
78   It tells the tool to substitute a Xen in-code comment having this structure:
79   /* SAF-0-safe [...] \*/
80 - analyser: it is an object containing pair of key-value strings, the key is
81   the analyser, so it can be cppcheck, coverity or eclair, the value is the
82   proprietary id corresponding on the finding, for example when coverity is
83   used as analyser, the tool will translate the Xen in-code coment in this way:
84   /* SAF-0-safe [...] \*/ -> /* coverity[misra_c_2012_rule_20_7_violation] \*/
85   if the object doesn't have a key-value, then the corresponding in-code
86   comment won't be translated.
87 - name: a simple name for the finding
88 - text: a proper justification to turn off the finding.
89
90
91Here is an example to add a new justification in false-positive-<tool>.json::
92
93|{
94|    "version": "1.0",
95|    "content": [
96|        {
97|            "id": "SAF-0-false-positive-<tool>",
98|            "violation-id": "<proprietary-id>",
99|            "tool-version": "<version>",
100|            "name": "R20.7 [...]",
101|            "text": "[...]"
102|        },
103|        {
104|            "id": "SAF-1-false-positive-<tool>",
105|            "violation-id": "",
106|            "tool-version": "",
107|            "name": "Sentinel",
108|            "text": "Next ID to be used"
109|        }
110|    ]
111|}
112
113To document a finding in false-positive-<tool>.json, just add another block
114{[...]} before the sentinel block, using the id contained in the sentinel block
115and increment by one the number contained in the id of the sentinel block.
116
117Here is an explanation of the fields inside an object of the "content" array:
118 - id: it has the same meaning as in the "safe" justification schema.
119   It tells the tool to substitute a Xen in-code comment having this structure:
120   /* SAF-0-false-positive-<tool> [...] \*/
121 - violation-id: its value is a string containing the proprietary id
122   corresponding to the finding, for example when <tool> is coverity, the Xen
123   tool will translate the Xen in-code coment in this way:
124   /* SAF-0-false-positive-coverity [...] \*/ -> /* coverity[misra_c_2012_rule_20_7_violation] \*/
125   if the object doesn't have a value, then the corresponding in-code comment
126   won't be translated.
127 - tool-version: the version of the tool affected by the false positive, if it
128   is discovered in more than one version, this string can be a range
129   (eg. 2.7 - 3.0)
130 - name, text: they have the same meaning as in the "safe" justification schema.
131
132
133Justification example
134---------------------
135
136Here an example of the usage of the in-code comment tags to suppress a finding
137for the Rule 8.6:
138
139Eclair reports it in its web report, file xen/include/xen/kernel.h, line 68:
140
141| MC3A2.R8.6 for program 'xen/xen-syms', variable '_start' has no definition
142
143Also coverity reports it, here is an extract of the finding:
144
145| xen/include/xen/kernel.h:68:
146| 1. misra_c_2012_rule_8_6_violation: Function "_start" is declared but never
147 defined.
148
149The analysers are complaining because we have this in xen/include/xen/kernel.h
150at line 68::
151
152| extern char _start[], _end[], start[];
153
154Those are symbols exported by the linker, hence we will need to have a proper
155deviation for this finding.
156
157We will prepare our entry in the safe.json database::
158
159|{
160|    "version": "1.0",
161|    "content": [
162|        {
163|        [...]
164|        },
165|        {
166|            "id": "SAF-1-safe",
167|            "analyser": {
168|                "eclair": "MC3A2.R8.6",
169|                "coverity": "misra_c_2012_rule_8_6_violation"
170|            },
171|            "name": "Rule 8.6: linker script defined symbols",
172|            "text": "It is safe to declare this symbol because it is defined in the linker script."
173|        },
174|        {
175|            "id": "SAF-2-safe",
176|            "analyser": {},
177|            "name": "Sentinel",
178|            "text": "Next ID to be used"
179|        }
180|    ]
181|}
182
183And we will use the proper tag above the violation line::
184
185| /* SAF-1-safe R8.6 linker defined symbols */
186| extern char _start[], _end[], start[];
187
188This entry will fix also the violation on _end and start, because they are on
189the same line and the same "violation ID".
190
191Also, the same tag can be used on other symbols from the linker that are
192declared in the codebase, because the justification holds for them too.
193
194A possible violation found by Cppcheck can be handled in the same way, from the
195cppcheck text report it is possible to identify the violation id:
196
197| include/public/arch-arm.h(226,0):misra-c2012-20.7:style:Expressions resulting from the expansion of macro parameters shall be enclosed in parentheses (Misra rule 20.7)
198
199The violation id can be located also in the HTML report, opening index.html from
200the browser, the violations can be filtered by id in the left side panel, under
201the column "Defect ID". On the right there will be a list of files with the type
202of violation and the violation line number, for the same violation above, there
203will be an entry like the following and the violation id will be in the column
204"Id":
205
206| include/public/arch-arm.h
207| [...]
208| 226 misra-c2012-20.7  style Expressions resulting from the expansion of macro parameters shall be enclosed in parentheses (Misra rule 20.7)
209| [...]
210
211Given the violation id "misra-c2012-20.7", the procedure above can be followed
212to justify this finding.
213
214Another way to justify the above violation is to put the in-code comment tag
215at the end of the affected line::
216
217| extern char _start[], _end[], start[]; /* SAF-1-safe [...] */
218
219This way of deviating violations needs however to be used only when placing the
220tag above the line can't be done. This option suffers from some limitation on
221cppcheck and coverity tool that don't support natively the suppression comment
222at the end of the line.
223