Re: struct mctelem_cookie missing definition

[Jan Beulich <jbeulich@xxxxxxxx>]

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

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

... undermining this rationale of the rule then, though.

Jan