Re: [PATCH 05/12] x86/crash: address Misra 2.1 rule violation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.09.2026 20:53, Nicola Vetrini wrote:
> On 2026-09-10 14:29, Roger Pau Monné wrote:
>> On Thu, Sep 10, 2026 at 11:52:10AM +0200, Jan Beulich wrote:
>>> On 10.09.2026 11:30, Roger Pau Monné wrote:
>>> > On Thu, Sep 10, 2026 at 10:38:34AM +0200, Jan Beulich wrote:
>>> >> On 10.09.2026 09:49, Roger Pau Monné wrote:
>>> >>> On Fri, Aug 28, 2026 at 09:02:18AM +0200, Jan Beulich wrote:
>>> >>>> The use of unreachable(), when unreachability is visible to Eclair (and
>>> >>>> compilers), is deemed a violation. Drop the redundant statement.
>>> >>>
>>> >>> Urg, isn't that something that should be fixed in Eclair then?
>>> >>> Otherwise all the unreachable() calls in our codebase are likely to be
>>> >>> found by Eclair sooner or later, and will need to be removed.
>>> >>
>>> >> No, aiui most are covered by deviations. In particular ones in BUG() and
>>> >> ASSERT_UNREACHABLE().
>>> >
>>> > Shouldn't this be a deviation then also?
>>>
>>> Maybe, just that I had no good idea how to express such a deviation 
>>> (preferably
>>> without a SAF comment).
>>>
>>> >  Maybe it would be helpful if
>>> > the commit message states why this is handled differently from other
>>> > unreachable() instances then.
>>>
>>> I've added "..., , and the one here isn't covered by a deviation" to the 
>>> first
>>> sentence. Will that suffice?
>>
>> TBH, the handling of unreachable() feels inconsistent to me.  I don't
>> blame you for this, I know you are just trying to fix the remaining
>> issues.
>>
>> I guess I will defer the change to someone more familiar with MISRA
>> and why some unreachable() usages are covered by deviations while
>> others aren't.
>>
>> I think the point of adding something to the commit message is to
>> justify why this is removed vs a deviation being added.
> 
> Actually this should be done with a deviation, and I thought it was already 
> taken care of by
> 
> -config=MC3A2.R2.1,statements+={deliberate, 
> "call(decl(name(__builtin_unreachable||panic||do_unexpected_trap||machine_halt||machine_restart||reboot_or_halt)))"}
> 
> namely because unreachable() expands to a call to __builtin_unreachable(). It 
> might be worth checking why that is not the case. Probably the configuration 
> needs a slight tweaking.

Question (on my side at least) is: How would such "checking" look like?
As said elsewhere, the syntax used in and the semantics of all these
deviations are close to impossible to fully understand (which is very
certainly different for you). Yet without fully understanding what's
written at present, how would one "check" that this is actually what is
wanted/needed?

Jan