Re: struct mctelem_cookie missing definition

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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))
> 
> Arguable that's ...

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.


> >> --- a/docs/misra/deviations.rst
> >> +++ b/docs/misra/deviations.rst
> >> @@ -324,6 +324,12 @@ 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.
> >> +     - Tagged as `deliberate` for ECLAIR. Such pointer is struct
> >> +       mctelem_cookie \*.
> >> +
> > 
> > I think here (or above in the deviation text) the concern raised by the 
> > rationale of the rule should be addressed; namely, the rule states: 
> > "Conversions shall not be performed between a pointer to an incomplete 
> > type and any other type" because the resulting pointer might be 
> > misaligned, therefore there should be an argument as to why such thing 
> > is not possible.

I think the explanation would be that it is OK to have misaligned pointers
because they cannot be dereferenced by design.