Proposal for deviations in static analyser findings

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi all,

Here is a proposal to create a deviation system for all the static analyser 
finding from
both generic static code checkers and MISRA compliance, as said before, there 
might
be some case where we won’t fix a part of the code because the way it is 
written is
safer than the proposed fix from the tool or coding guidance.

The proposed tags will be translated by a tool during the analysis, using the 
database (JSON file) reported below,
the analysis steps will be:
1) translate the “Xen codebase in-code comment tags” to the proprietary syntax 
of the chosen tool in the source code.
2) perform the analysis and produce a report
3) revert back the source code to the original.

The proposal includes also feedbacks from the design session about FuSa held at 
the Xen Summit 2022.


Documenting violations
======================

Static analysers are used on the Xen codebase for both static analysis and MISRA
compliance.
There might be the need to suppress some findings instead of fixing them and
many tools permit the usage of in-code comments that suppress findings so that
they are not shown in the final report.

Xen includes a tool capable of translating a specific comment used in its
codebase to the right proprietary in-code comment understandable by the selected
analyser that suppress its finding.

In the Xen codebase, these tags will be used to document and suppress findings:

- SAF-X-safe: This tag means that the next line of code contains a finding, but
  the non compliance to the checker is analysed and demonstrated to be safe.
- SAF-X-false-positive: This tag means that the next line of code contains a
  finding, but the finding is a bug of the tool.

SAF stands for Static Analyser Finding, the X is a placeholder for a positive
number that starts from zero, the number after SAF- shall be incremental and
unique.

Entries in the database should never be removed, even if they are not used
anymore in the code (if a patch is removing or modifying the faulty line).
This is to make sure that numbers are not reused which could lead to conflicts
with old branches or misleading justifications.

The files where to store all the justifications are in xen/docs/misra/ and are
named as safe.json and false-positive.json, they have JSON format.

Here is an example to add a new justification::

|{
|    "version": "1.0",
|    "content": [
|        {
|            "id":"SAF-0-safe",
|            "analyser": {
|                "cppcheck": "misra-c2012-20.7",
|                "coverity": "misra_c_2012_rule_20_7_violation",
|                "eclair": "MC3R1.R20.7"
|            },
|            "name": “R20.7 C macro parameters not used as expression",
|            "text": "The macro parameters used in this […]"
|        },
|        {
|            "id":”SAF-1-safe",
|            "analyser": {
|                "cppcheck": "unreadVariable",
|                "coverity": "UNUSED_VALUE"
|            },
|            "name": “Variable set but not used",
|            "text": “It is safe because […]"
|        },
|        {
|            "id":”SAF-2-safe",
|            "analyser": {},
|            "name": "Sentinel",
|            "text": ""
|        }
|    ]
|}

To document a finding, just add another block {[...]} before the sentinel block,
using the id contained in the sentinel block and increment by one the number
contained in the id of the sentinel block.

Here a brief explanation of the field inside an object of the "content" array:
- id: it is a unique string that is used to refer to the finding, many finding
  can be tagged with the same id, if the justification holds for any applied
  case.
  It tells the tool to substitute a Xen in-code comment having this structure:
  /* SAF-0-safe [...] \*/
- analyser: it is an object containing pair of key-value strings, the key is
  the analyser, so it can be cppcheck, coverity or eclair. The value is the
  proprietary id corresponding on the finding, for example when coverity is
  used as analyser, the tool will translate the Xen in-code coment in this way:
  /* SAF-0-safe [...] \*/ -> /* coverity[coverity-id] \*/
  if the object doesn't have a key-value, then the corresponding in-code
  comment won't be translated.
- name: a simple name for the finding
- text: a proper justification to turn off the finding.



Here an example of the usage of the in-code comment tags:

/* SAF-0-safe [eventual developer message that shall not exceeds line char max 
count, don’t break the line!] */
#define string_param(_name, _var) \
    __setup_str __setup_str_##_var[] = _name; \
    __kparam __setup_##_var = \
        { .name = __setup_str_##_var, \
          .type = OPT_STR, \
          .len = sizeof(_var), \
          .par.var = &_var }

In the example above, the tool finding for this macro is suppressed. When there 
are multiple findings for
the same line, multiple in-code comments needs to be inserted, every one on a 
different line.

Cheers,
Luca