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

[Roberto Bagnara <roberto.bagnara@xxxxxxxxxxx>]

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.

Kind regards,

   Roberto

Roberto Bagnara, Ph.D.

Software Verification Expert and Evangelist, BUGSENG (http://bugseng.com)
Professor of Computer Science, University of Parma
Member, ISO/IEC JTC1/SC22/WG14 - C Standardization Working Group
Member, MISRA C Working Group