Re: [XEN PATCH][for-4.19 v2] xen: Add SAF deviations for MISRA C:2012 Rule 7.1

From: Jan Beulich <jbeulich@suse.com>
To: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: sstabellini@kernel.org, michal.orzel@amd.com,
	xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com,
	consulting@bugseng.com, andrew.cooper3@citrix.com,
	roger.pau@citrix.com,
	Simone Ballarin <simone.ballarin@bugseng.com>,
	Doug Goldstein <cardoe@cardoe.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH][for-4.19 v2] xen: Add SAF deviations for MISRA C:2012 Rule 7.1
Date: Fri, 20 Oct 2023 08:38:24 +0200	[thread overview]
Message-ID: <dd58e1cb-8f17-986d-f4f9-f39e998e05a0@suse.com> (raw)
In-Reply-To: <d879b8e4689e51131f081edcb3b2abe6@bugseng.com>

On 19.10.2023 18:34, Nicola Vetrini wrote:
> On 19/10/2023 17:57, Jan Beulich wrote:
>> On 19.10.2023 13:04, Nicola Vetrini wrote:
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -85,10 +85,10 @@ conform to the directive."
>>>  # Series 7.
>>>  #
>>>
>>> --doc_begin="Usage of the following constants is safe, since they are 
>>> given as-is
>>> -in the inflate algorithm specification and there is therefore no risk 
>>> of them
>>> -being interpreted as decimal constants."
>>> --config=MC3R1.R7.1,literals={safe, 
>>> "^0(007|37|070|213|236|300|321|330|331|332|333|334|335|337|371)$"}
>>> +-doc_begin="Octal constants used as arguments to macro INSTR_ENC or 
>>> MASK_EXTR
>>> +can be used, because they appear as is in specifications, manuals, 
>>> and
>>> +algorithm descriptions."
>>> +-config=MC3R1.R7.1,reports+={safe, 
>>> "any_area(any_loc(any_exp(macro(^(INSTR_ENC|MASK_EXTR)$))))"}
>>
>> INSTR_ENC() is a local macro in x86'es AMD SVM code. A macro of the 
>> same
>> name could imo be introduced without issues in, say, Arm code. The 
>> above
>> would then needlessly suppress findings there, aiui.
>>
>> MASK_EXTR() otoh is a global macro which ise used for various purposes.
>> Excluding checking there is imo going too far, too.
> 
> I should have thought about it; I can simply enforce the deviation to 
> additionally match
> only a specific file for each of the macros.

That'll work for INSTR_ENC(), but not for MASK_EXTR().

>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -90,6 +90,12 @@ Deviations related to MISRA C:2012 Rules:
>>>           - __emulate_2op and __emulate_2op_nobyte
>>>           - read_debugreg and write_debugreg
>>>
>>> +   * - R7.1
>>> +     - It is safe to use certain octal constants the way they are 
>>> defined in
>>> +       specifications, manuals, and algorithm descriptions as 
>>> arguments to
>>> +       macros 'INSTR_ENC' and 'MASK_EXTR'.
>>> +     - Tagged as `safe` for ECLAIR.
>>
>> Similarly this wording is imo inappropriate, while ...
>>
> 
> I tried to be a bit more specific about what is actually being deviated, 
> on the assumption
> that the maintainers and contributors would find it more useful than 
> parsing ecl files, but
> if you prefer it to be more general, no problem.

Just dropping everything after the last comma would deal with my concern.

Jan