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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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?
>
> Jan
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.

> > 4. an use of any other macro with an octal constant in its expansion
> > won't be deviated,
> >     unless the configuration is suitably edited.
> >
> > Does this address your concern?
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)