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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.10.2026 06:30, Chen, Shuaidong wrote:
> 
> On 10/27/2026 5:14 PM, Alejandro Vallejo wrote:
>> 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
> 
> Yes, as Alejandro described, some DCE techniques have been adopted in 
> safety to meet the coverage requirements for safety certification. 
> However, this inevitably introduces code or macro patterns that can 
> trigger violations of this rule in some cases.
> 
> The current deviation for this rule is unnecessarily narrow. If 
> extending it too broadly would have an excessive impact, we could also 
> expand it incrementally to cover specific cases as they arise.
> 
> For example, this patch could initially extend the deviation to switch 
> statements only. That would already be a reasonable improvement over the 
> current situation, where only if statements are covered.

That is what the proposed patch does, isn't it? Personally I keep thinking
that this is what is going too far. Extending to while() and maybe for(),
otoh, would be okay with me, I think. Albeit infinite loops and do/while
already have "built-in" exceptions.

Jan