Re: struct mctelem_cookie missing definition

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-02-18 22:37, Stefano Stabellini wrote:
> On Tue, 18 Feb 2025, Jan Beulich wrote:
> > On 18.02.2025 03:45, Stefano Stabellini wrote:
> > > On Mon, 17 Feb 2025, Jan Beulich wrote:
> > >> On 15.02.2025 09:59, Nicola Vetrini wrote:
> > >>> On 2025-02-15 00:04, Stefano Stabellini wrote:
> > >>>> On Fri, 14 Feb 2025, Jan Beulich wrote:
> > >>>>>> Would deviating macros "COOKIE2MCTE" and "MCTE2COOKIE" work?
> > >>>>>
> > >>>>> If it did, COOKIE2ID and ID2COOKIE would likely need including as
> > >>>>> well.
> > >>>>
> > >>>> I wrote this patch. Nicola, can you please check the changes to
> > >>>> deviations.ecl, this is the first time I try to write the deviation
> > >>>> myself :-)
> > >>>>
> > >>>> ---
> > >>>> misra: add 11.2 deviation for incomplete types
> > >>>>
> > >>>> struct mctelem_cookie* is used exactly because it is an incomplete type
> > >>>> so the pointer cannot be dereferenced. This is deliberate. So add a
> > >>>> deviation for it.
> > >>>>
> > >>>> In deviations.ecl, add the list of macros that do the conversions to
> > >>>> and
> > >>>> from struct mctelem_cookie*.
> > >>>>
> > >>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> > >>>>
> > >>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > >>>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > >>>> index a28eb0ae76..87bfd2160c 100644
> > >>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > >>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > >>>> @@ -366,6 +366,14 @@ constant expressions are required.\""
> > >>>>  }
> > >>>>  -doc_end
> > >>>>
> > >>>> +-doc_begin="Certain pointers point to incomplete types purposely so
> > >>>> that they are impossible to dereference."
> > >>>> +-config=MC3A2.R11.2,reports+={deliberate,
> > >>>> "any_area(any_loc(any_exp(macro(^COOKIE2MCTE$))))"}
> > >>>> +-config=MC3A2.R11.2,reports+={deliberate,
> > >>>> "any_area(any_loc(any_exp(macro(^MCTE2COOKIE$))))"}
> > >>>> +-config=MC3A2.R11.2,reports+={deliberate,
> > >>>> "any_area(any_loc(any_exp(macro(^COOKIE2ID$))))"}
> > >>>> +-config=MC3A2.R11.2,reports+={deliberate,
> > >>>> "any_area(any_loc(any_exp(macro(^ID2COOKIE$))))"}
> > >>>> +}
> > >>>
> > >>> -config=MC3A2.R11.2,reports+={deliberate,
> > >>> 
> > "any_area(any_loc(any_exp(macro(name(COOKIE2MCTE||MCTE2COOKIE||COOKIE2ID||ID2COOKIE)))))"}
> > >>>
> > >>> Note however that there is also this deviation in place
> > >>>
> > >>> -doc_begin="The conversion from a pointer to an incomplete type to
> > >>> unsigned long does not lose any information, provided that the target
> > >>> type has enough bits to store it."
> > >>> -config=MC3A2.R11.2,casts+={safe,
> > >>>    "from(type(any()))
> > >>>     &&to(type(canonical(builtin(unsigned long))))
> > >>>     &&relation(definitely_preserves_value)"
> > >>> }
> > >>> -doc_end
> > >>>
> > >>> I was a bit confused by Jan's remark, which seemed correct, but I
> > >>> couldn't see any violations in the report, so I dug a bit deeper.
> > >>> ID2COOKIE and COOKIE2ID, which operate to/from unsigned long are already
> > >>> excluded due to being safe. If you envision those macros to be used with
> > >>> other types, then your deviation should mention them, otherwise they are
> > >>> already handled.
> > >>
> > >> Yet then can't we leverage that deviation to also make the other two
> > >> covered:
> > >>
> > >> #define        COOKIE2MCTE(c)          ((struct mctelem_ent *)(unsigned 
> > long)(c))
> > >> #define        MCTE2COOKIE(tep)        ((mctelem_cookie_t)(unsigned 
> > long)(tep))
> > >
> > > Jan is asking why ID2COOKIE and COOKIE2ID are considered safe, while
> > > COOKIE2MCTE and MCTE2COOKIE are not. I think the reason is that
> > > ID2COOKIE and COOKIE2ID convert to/from unsigned long and that falls
> > > under the other deviation we already have:
> > >
> > > -doc_begin="The conversion from a pointer to an incomplete type to
> > > unsigned long does not lose any information, provided that the target
> > > type has enough bits to store it."
> > > -config=MC3A2.R11.2,casts+={safe,
> > >    "from(type(any()))
> > >     &&to(type(canonical(builtin(unsigned long))))
> > >     &&relation(definitely_preserves_value)"
> > >
> > > On the other hand COOKIE2MCTE and MCTE2COOKIE convert to/from another
> > > pointer type, so they don't fall under the same deviation.
> >
> > And then the adjusted forms suggested above ought to also be covered,
> > I would have thought.
> I understand your point. I tried it, but it does not work. I do not 
> know
> why. Someone with more knowledge of ECLAIR internals than I have might
> be able to explain.
>
> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/sstabellini/xen/ECLAIR_normal/my-eclair-11.2-4-1/X86_64/9176469474/PROJECT.ecd;/by_service/MC3A2.R11.2.html#{%22select%22:true,%22selection%22:{%22hiddenAreaKinds%22:[],%22hiddenSubareaKinds%22:[],%22show%22:false,%22selector%22:{%22enabled%22:true,%22negated%22:true,%22kind%22:0,%22domain%22:%22kind%22,%22inputs%22:[{%22enabled%22:true,%22text%22:%22violation%22}]}}}
The reason is quite simple: the deviation is for casts from any type to 
unsigned long. It would need a similar configuration 
from(type(canonical(builtin(unsigned long)))) to(any()) in order to 
catch those.
> I suggest we go with this patch instead.
>
> ---
> misra: add 11.2 deviation for incomplete types
>
> struct mctelem_cookie* is used exactly because it is an incomplete type
> so the pointer cannot be dereferenced. This is deliberate. So add a
> deviation for it.
>
> In deviations.ecl, add the list of macros that do the conversions to 
> and
> from struct mctelem_cookie*.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
>
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index a28eb0ae76..d33b777e6a 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -366,6 +366,10 @@ constant expressions are required.\""
>  }
>  -doc_end
>
> +-doc_begin="Certain pointers point to incomplete types purposely so 
> that they are impossible to dereference."
> +-config=MC3A2.R11.2,reports+={deliberate, 
> "any_area(any_loc(any_exp(macro(name(COOKIE2MCTE||MCTE2COOKIE||COOKIE2ID||ID2COOKIE)))))"}
> +-doc_end
> +
>  -doc_begin="Conversions to object pointers that have a pointee type 
> with a smaller (i.e., less strict) alignment requirement are safe."
>  -config=MC3A2.R11.3,casts+={safe,
>    "!relation(more_aligned_pointee)"
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index fe0b1e10a2..04ffc62f44 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -324,6 +324,13 @@ Deviations related to MISRA C:2012 Rules:
>         semantics that do not lead to unexpected behaviour.
>       - Tagged as `safe` for ECLAIR.
>
> +   * - R11.2
> +     - Certain pointers point to incomplete types purposely so that 
> they
> +       are impossible to dereference, since they cannot be 
> dereferenced,
> +       pointers alignments considerations do not apply.
> +     - Tagged as `deliberate` for ECLAIR. Such pointer is struct
> +       mctelem_cookie \*.
> +
>     * - R11.2
>       - The conversion from a pointer to an incomplete type to unsigned 
> long
>         does not lose any information, provided that the target type 
> has enough
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253