> On 26 Oct 2022, at 00:24, Stefano Stabellini <sstabell...@kernel.org> wrote: > > On Tue, 25 Oct 2022, Luca Fancellu wrote: >> Hi all, >> >> This is the V2 of the proposal for deviations tagging in the Xen codebase, >> this includes >> all the feedbacks from the FuSa session held at the Xen Summit 2022 and all >> the >> feedbacks received in the previous proposal sent on the mailing list. > > It would be good to commit this proposal (when acked) as a pandoc under > xen.git/docs/misra
Yes it will be part of my serie that I will push soon > > >> Here a link to the previous thread: >> https://lists.xenproject.org/archives/html/xen-devel/2022-10/msg00541.html >> >> 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 will include 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-<tool>: 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, the number after SAF- shall be incremental and unique, base ten >> notation and without leading zeros. >> >> 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. >> >> An entry can be reused in multiple places in the code to suppress a finding >> if >> and only if the justification holds for the same non-compliance to the coding >> standard. >> >> An orphan entry, that is an entry who was justifying a finding in the code, >> but later >> that code was removed and there is no other use of that entry in the code, >> can be >> reused as long as the justification for the finding holds. This is done to >> avoid the >> allocation of a new entry with exactly the same justification, that would >> lead to waste >> of space and maintenance issues of the database. >> >> The files where to store all the justifications are in xen/docs/misra/ and >> are >> named as safe.json and false-positive-<tool>.json, they have JSON format, >> entries >> of these files have independent ID numbering. >> >> Here is an example to add a new justification in safe.json:: >> >> |{ >> | "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": "" >> | } >> | ] >> |} >> >> Here is an example to add a new justification in >> false-positive-cppcheck.json:: >> >> |{ >> | "version": "1.0", >> | "content": [ >> | { >> | "id":"SAF-0-false-positive-cppcheck", >> | "analyser": { >> | "cppcheck": "misra-c2012-20.7" >> | }, >> | “tool-version”: “2.7", >> | "name": “R20.7 second operand of member-access operator", >> | "text": "The second operand of a member access operator shall >> be a name of a member of the type pointed to, so in this particular case it >> is wrong to use parentheses on the macro parameter." > > Any way we can make the text max 80 chars in lengths (without breaking > the json parser)? Unfortunately it is a limitation of json, but it’s so easy to integrate in python scripts that I couldn’t find a better alternatives in terms of flexibility and availability of parser. I guess in case of justifications that needs lot of text, graphs, images, we can commit a document and put the path to it as text content. > > Also, if we are going to commit this document in xen.git, please use > consistently " instead of “ Yes this is a copy-paste error I will fix > > >> | }, >> | { >> | "id":”SAF-1-false-positive-cppcheck", >> | "analyser": {}, >> | “tool-version”: “", >> | "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 [...] \*/ > > No need for the final \ My vscode extension that renders the .rst file is complaining if I don’t use the final \ . > > Everything else looks good to me. > > >> - 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 to suppress a >> finding for the Rule 8.6: >> >> Eclair reports it here: >> https://eclairit.com:3787/fs/var/lib/jenkins/jobs/XEN/configurations/axis-Target/ARM64/axis-agent/public/builds/549/archive/ECLAIR/out/PROJECT.ecd;/sources/xen/include/xen/kernel.h.html#R50743_1 >> >> Also coverity reports it, here an extract of the finding: >> >> xen/include/xen/kernel.h:68: >> 1. misra_c_2012_rule_8_6_violation: Function "_start" is declared but never >> defined. >> >> The analysers are complaining because we have this in >> xen/include/xen/kernel.h at line 68: >> >> extern char _start[], _end[], start[]; >> >> Those are symbols exported by the linker, hence we will need to have a >> proper deviation for this finding. >> >> We will prepare our entry in the database: >> >> |{ >> | "version": "1.0", >> | "content": [ >> | { >> | […] >> | }, >> | { >> | "id":”SAF-1-safe", >> | "analyser": { >> | “eclair": "MC3R1.R8.6", >> | "coverity": "misra_c_2012_rule_8_6_violation" >> | }, >> | "name": “Rule 8.6: linker defined symbols", >> | "text": “It is safe to declare this symbol because it is >> defined in the linker script." >> | }, >> | { >> | "id":”SAF-2-safe", >> | "analyser": {}, >> | "name": "Sentinel", >> | "text": "" >> | } >> | ] >> |} >> >> And we will use the proper tag above the violation line: >> >> /* SAF-1-safe [optional text] */ >> extern char _start[], _end[], start[]; >> >> This entry will fix also the violation on _end and start, because they are >> on the same line and the >> same “violation ID”. >> >> Also, the same tag can be used on other symbols from the linker that are >> declared in the codebase, >> because the justification holds for them too.
