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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.10.2026 09:18, 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

Just to mention - both pages give 404, presumably to anyone outside
of AMD.

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

Plus replacing switch() by if/else chains impacts code readability.

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

We may need to find a middle ground here.

Jan