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.stabell...@amd.com>
> >>
> >> 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.
