Re: [XEN PATCH] misra: merge the two R14.3 invariant-condition deviations

["Alejandro Vallejo" <alejandro.garciavallejo@xxxxxxx>]

On Tue Oct 27, 2026 at 9:18 AM CET, Chen, Shuaidong wrote:
>
> On 10/26/2026 9:14 PM, Jan Beulich wrote:
> > On 26.10.2026 04:03, Shuaidong Chen wrote:
> >> MC3A2.R14.3 ("Controlling expressions shall not be invariant") is
> >> deviated by two separate entries.  One covers 'if' statements and
> >> conditional operators and places no restriction on where the invariant
> >> condition comes from.  The other covers 'switch' statements, but only
> >> when the controlling expression is a 'sizeof' operator.
> >>
> >> The rationale recorded for the two is the same: the condition is
> >> deliberate.  Nothing about a 'switch' makes an invariant condition less
> >> deliberate than the same condition in an 'if', so the 'sizeof'
> >> restriction reads as an artefact of that entry having been added for
> >> read_atomic() and friends in arch/x86/include/asm/atomic.h rather than
> >> as a considered limit.
> > Well,
> >
> >      if ( 1 )
> >
> > can make sense, especially when 1 is the expansion of e.g. IS_ENABLED().
> > Otoh
> >
> >      switch ( 1 )
> >
> > doesn't make a lot of sense, even when it's as a result of the expanding
> > IS_ENABLED(). I agree that limiting to just sizeof() is unnecessarily
> > strict; alignof(), for example, should about always be usable where
> > sizeof() is. Yet making if() and switch() equal is imo going too far. I
> > realize though that it may be difficult to find a suitable boundary.
> > Stefano - do you possibly still have notes from the discussions long ago?
> >
> >> Fold switch_stmt into the general entry and drop the 'sizeof'-restricted
> >> one, which the general predicate now subsumes.  Invariant 'switch'
> >> conditions that are not a 'sizeof' become deliberate as a result; that
> >> is the intent.
> >>
> >> Signed-off-by: Shuaidong Chen <shuaidong.chen@xxxxxxx>
> > What you don't say at all: Does this help with any remaining violations?
> >
> > Jan
> Yes, and let me give the full context, since the motivation comes from
> downstream safety work rather than from the upstream analysis.
>
> On safety branch one of the techniques we rely on there is to pin build-time
> configuration to compile-time constants, so that the code which the
> intended configuration can never reach is removed by the compiler
> rather than merely left untested.  That matters for us in two ways:
> code eliminated at compile time does not enter the coverage
> denominator, and - more importantly - it is not in the binary at all,
> so it does not have to be carried through the whole qualification
> chain.
>
> Two examples:
>
> https://gitlab.com/amd-xen-safety/xen/-/commit/160dac43f554733a68d83c38cc1d0e73b3b7bff0
> https://gitlab.com/amd-xen-safety/xen/-/commit/e0eccfa7184b7acee23916cd5cb2a3b5afa101f2
>
> A side effect of this is that some of those constants end up as the
> controlling expression of an switch statement, which then
> trips R14.3.
>
>  > Otoh switch ( 1 ) doesn't make a lot of sense
>
> Agreed in the abstract, and I would push back on anyone writing that
> by hand.  But that is not what happens here.  The sources still read
>
>    switch ( gt->gt_version )
>    switch ( opt_bootscrub )
>
> exactly as upstream wrote them, against what upstream intends as a
> runtime-variable value.  The constant only appears after a build
> configuration specialises that value.  So the construct is not a
> pointless switch on a literal; it is ordinary upstream code that a
> particular configuration has made invariant - which is precisely the
> situation the existing 'deliberate' deviation for if statements and
>
> conditional operators already covers.
>
> Of course the other alternative: rewrite those switches as
> if/else chains, which the existing deviation would already cover it, and 
> the violations gone. Technically that works.
>
> The reason I would rather not is that it is churn in common code for no 
> functional
> reason, carried indefinitely as a delta against upstream, and it makes
> every subsequent rebase of the safety branch more expensive.
>
> Having seen that a deviation for switch already exists - restricted to
> sizeof - extending it to switch in general seemed the smaller and more
> honest change: it states that an invariant switch condition is
> deliberate, just as the project already states for if and for the
> conditional operators.
>
> -Shuaidong

To build further on this. add_sized() is one such case (already covered
by the disappearing exception. There are existing cases in the upstream
tree where this is or will be the case, but due to their being inside
macros become invisible to Eclair.

Or my (still not resent) cpu_vendor() series, I rely on placing
X86_VENDOR_X constants as invariant expressions in switch statements.
Even if nothing does it today, MISRA would be prohibiting constification
of any global that happens to go in a switch for no particular benefit.

IOW: This isn't about hardcoded values, it's about IS_ENABLED() gating
and DCE.

On the specifics of the patch, it's not exclusive to switches. I'd much
rather see it extended to for loops as well.

    for ( unsigned int i = 0; i < NR_MAX_FOOS; i++ )
        ...

Where having NR_MAX_FOOS be zero is effectively a guard on the whole
loop. Today we need explicit early IS_ENABLED()-gated exits to keep
Eclair happy on some such statements, and that's just strictly worse.

Shuaidong had to recently fix these violations in our tree because the
spirit of the existing exception is needlessly exclusive to "if"
statements.

Cheers,
Alejandro