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: Mon, 23 Oct 2023 10:17:07 +0200	[thread overview]
Message-ID: <23f2682f-acc4-20aa-21fc-644a3c1141b5@suse.com> (raw)
In-Reply-To: <800b2c809829942210323804b6778c40@bugseng.com>

On 23.10.2023 10:03, Nicola Vetrini wrote:
> On 23/10/2023 08:34, Jan Beulich wrote:
>> On 20.10.2023 16:58, Nicola Vetrini wrote:
>>> On 20/10/2023 15:24, Jan Beulich wrote:
>>>> On 20.10.2023 12:33, Nicola Vetrini wrote:
>>>>> On 20/10/2023 08:38, Jan Beulich wrote:
>>>>>> 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().
>>>>>>
>>>>>
>>>>> Why? What I'm deviating is reports due to octal constants used in
>>>>> expressions
>>>>> that contain MASK_EXTR in their expansion if and only if these are
>>>>> located in the
>>>>> file svm.h.
>>>>> No extra octal constant will match all these constraints.
>>>>
>>>> New MASK_EXTR() uses can appear at any time, without necessarily
>>>> matching the justification.
>>>>
>>>> Jan
>>>
>>> Sorry, but I don't understand what's your concern exactly. With the
>>> improvements I proposed
>>> (hence a new patch revision) I see the following possible future
>>> scenarios:
>>>
>>> 1. an use of MASK_EXTR() in a file other than x86/hvm/svm/emulate.c
>>> appears, with no
>>>     use of octal constants in the expansion. This won't be deviated;
>>> 2. an use of MASK_EXTR() in x86/hvm/svm/emulate.c appears, with no use
>>> of octal
>>>     constants in the expansion. This won't be deviated;
>>> 3. an use of MASK_EXTR() in x86/hvm/svm/emulate.c appears, with octal
>>>     constants in the expansion. This will be deviated;
>>
>> This is what I'm concerned of: How do you know up front whether such 
>> new
>> uses want deviating?
> 
> I understand you concern now. I can argue that all the macros in that 
> table have indeed
> an octal constant in their definition (0 is explicitly allowed by 
> MISRA).
> This is also specified in the comment above the INSTR_ENC macro 
> definition, therefore any
> new addition should have an octal the second argument to INSTR_ENC.

Right, and I previously indicated I agree as far as INSTR_ENC() goes.
What we appear to continue to disagree about is MASK_EXTR().

Jan