Re: [XEN PATCH v2 1/3] EFI: address a violation of MISRA C Rule 13.6

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.10.2024 14:49, Roberto Bagnara wrote:
> On 2024-10-08 07:59, Jan Beulich wrote:
>> On 02.10.2024 08:54, Roberto Bagnara wrote:
>>> On 2024-10-02 08:09, Jan Beulich wrote:
>>>> On 01.10.2024 23:36, Stefano Stabellini wrote:
>>>>> On Tue, 1 Oct 2024, Jan Beulich wrote:
>>>>>> On 01.10.2024 07:25, Roberto Bagnara wrote:
>>>>>>> On 2024-09-30 15:07, Jan Beulich wrote:
>>>>>>>> On 30.09.2024 14:49, Federico Serafini wrote:
>>>>>>>>> guest_handle_ok()'s expansion contains a sizeof() involving its
>>>>>>>>> first argument which is guest_handle_cast().
>>>>>>>>> The expansion of the latter, in turn, contains a variable
>>>>>>>>> initialization.
>>>>>>>>>
>>>>>>>>> Since MISRA considers the initialization (even of a local variable)
>>>>>>>>> a side effect, the chain of expansions mentioned above violates
>>>>>>>>> MISRA C:2012 Rule 13.6 (The operand of the `sizeof' operator shall not
>>>>>>>>> contain any expression which has potential side effect).
>>>>>>>>
>>>>>>>> I'm afraid I need to ask for clarification of terminology and alike 
>>>>>>>> here.
>>>>>>>> While the Misra doc has a section on Persistent Side Effects in its
>>>>>>>> Glossary appendix, what constitutes a side effect from its pov isn't
>>>>>>>> really spelled out anywhere. Which in turn raises the question whether 
>>>>>>>> it
>>>>>>>> is indeed Misra (and not just Eclair) which deems initialization a side
>>>>>>>> effect. This is even more so relevant as 13.6 talks of only 
>>>>>>>> expressions,
>>>>>>>> yet initializers fall under declarations (in turn involving an 
>>>>>>>> expression
>>>>>>>> on the rhs of the equal sign).
>>>>>>>>
>>>>>>>> All the same of course affects patch 2 then, too.
>>>>>>>
>>>>>>> MISRA C leaves the definition of "side effect" to the C Standard.
>>>>>>> E.g., C18 5.1.2.3p2:
>>>>>>>
>>>>>>>      Accessing a volatile object, modifying an object, modifying a file,
>>>>>>>      or calling a function that does any of those operations are all
>>>>>>>      side effects,[omitted irrelevant footnote reference] which are
>>>>>>>      changes in the state of the execution environment.
>>>>>>>
>>>>>>> The MISRA C:2012/2023 Glossary entry for "Persistent side effect"
>>>>>>> indirectly confirms that initialization is always a side effect.
>>>>>>
>>>>>> Hmm, that's interesting: There's indeed an example with an initializer
>>>>>> there. Yet to me the text you quote from the C standard does not say
>>>>>> that initialization is a side effect - it would be "modifying an
>>>>>> object" aiui, yet ahead of initialization being complete the object
>>>>>> doesn't "exist" imo, and hence can be "modified" only afterwards.
>>>>>
>>>>> I feel it's becoming a bit too philosophical. Since there's some room
>>>>> for interpretation and only two violations left to address, I believe
>>>>> it's best to stick with the stricter interpretation of the definition.
>>>>> Therefore, I'd proceed with this series in its current form.
>>>>
>>>> Proceeding with the series in its current form may be okay (as you say,
>>>> you view the changes as readability improvements anyway), but imo the
>>>> interpretation needs settling on no matter what. In fact even for these
>>>> two patches it may affect what their descriptions ought to say (would
>>>> be nice imo to avoid permanently recording potentially misleading
>>>> information by committing as is). And of course clarity would help
>>>> dealing with future instances that might appear. I take it you realize
>>>> that if someone had submitted a patch adding code similar to the
>>>> original forms of what's being altered here, it would be relatively
>>>> unlikely for a reviewer to spot the issue. IOW here we're making
>>>> ourselves heavily dependent upon Eclair spotting (supposed) issues,
>>>> adding extra work and delays for such changes to go in.
>>>
>>> You can do two things to obtain a second opinion:
>>>
>>> 1) Use the MISRA forum (here is the link to the forum
>>>      section devoted to the side-effect rules of MISRA C:2012
>>>      and MISRA C:2023 (https://forum.misra.org.uk/forumdisplay.php?fid=168).
>>>      The MISRA C Working Group will, in due course, provide
>>>      you with an official answer to your questions about what,
>>>      for the interpretation of Rule 13.6, has to be considered
>>>      a side effect.
>>>
>>> 2) Reach out to your ISO National Body and try to obtain
>>>      an official answer from ISO/IEC JTC1/SC22/WG14 (the
>>>      international standardization working group for the
>>>      programming language C) to your questions about what the
>>>      C Standard considers to be side effects.
>>
>> I took the latter route, and to my (positive) surprise I got back an answer
>> the same day. There was a request for someone to confirm, but so far I didn't
>> see further replies. Since this is a German institution I raised the question
>> in German and got back an answer in German (attached); I've tried my best to
>> translate this without falsifying anything, but I've omitted non-technical
>> parts:
>>
>> "Initialization of an object is never a side effect of the initialization
>> by itself. Of course expressions used for initialization can themselves have
>> side effects on other objects.
>>
>> @Andreas: Do you agree? C after all has a far simpler object model than C++.
>> The (potential) change in object representation (i.e. the bytes underlying
>> the object) is not a side effect of object initialization, but its primary
>> purpose."
>>
>> Further for Misra she added a reference to a Swiss person, but I think with
>> Bugseng we have sufficient expertise there.
> 
> Unfortunately, the (translation of the) answer you received adds
> confusion to previous confusion: who answered has highlighted the
> "side" part of the term, which is indeed relevant in computer science,
> but not for the C standard.

I can't see any highlighting in the original reply I received.

>  To the point that the same argument could
> be used to deny that ++i has a side effect because the increment is
> the "primary" effect...

Well, if it's just "++i;" there's no side effect, just a primary one. In
"n = ++i + j--;" there are side effects (the increment and decrement).

> Part of the confusion is maybe in the the following paragraph Jan
> wrote earlier:
> 
>  > Hmm, that's interesting: There's indeed an example with an initializer
>  > there. Yet to me the text you quote from the C standard does not say
>  > that initialization is a side effect - it would be "modifying an
>  > object" aiui, yet ahead of initialization being complete the object
>  > doesn't "exist" imo, and hence can be "modified" only afterwards.
> 
> In C, it is not true that the object does not exist ahead of
> initialization.

I quoted "exist" for that reason. Of course the object's lifetime starts
with its declaration. It just has indeterminate value at that point.

>  Try the following:
> 
> extern int f(int* p);
> 
> int main() {
>    int i = f(&i);
> }

Which to me falls under "Of course expressions used for initialization
can themselves have side effects on other objects." Just that "other"
isn't quite right here.

Jan