Re: [XEN PATCH 1/3] xen: introduce static_assert_unreachable()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.01.2024 09:20, Federico Serafini wrote:
> On 22/01/24 15:02, Jan Beulich wrote:
>> On 22.01.2024 14:48, Federico Serafini wrote:
>>> --- a/xen/include/xen/compiler.h
>>> +++ b/xen/include/xen/compiler.h
>>> @@ -64,6 +64,14 @@
>>>   # define fallthrough        do {} while (0)  /* fallthrough */
>>>   #endif
>>>   
>>> +/*
>>> + * Add the following macro to check that a program point is considered
>>> + * unreachable by the static analysis performed by the compiler,
>>> + * even at optimization level -O0.
>>> + */
>>> +#define static_assert_unreachable() \
>>> +    asm(".error \"unreachable program point reached\"");
>>
>> Did you check the diagnostic that results when this check actually
>> triggers? I expect it will be not really obvious from the message
>> you introduce where the issue actually is. I expect we will want
>> to use some of __FILE__ / __LINE__ / __FUNCTION__ to actually
>> supply such context.
> 
> The assembler error comes with file and line information, for example:
> 
> ./arch/x86/include/asm/uaccess.h: Assembler messages:
> ./arch/x86/include/asm/uaccess.h:377: Error: unreachable program point 
> reached
> 
> At line 377 there is an use of get_unsafe_size() where I passed a wrong
> size as argument. Is that clear enough?

Hmm, yes, looks like it might be sufficient. Mentioning __FUNCTION__ may
still add value, though. But I see now that __FILE__ / __LINE__ are
already covered for.

> What do you think about modifying the message as follows:
> "unreachability static assert failed."

I'm okay-ish with the original text, and I like it slightly better than
this new suggestion. If we want "static assert" in the output, then maybe
"static assertion failed: unreachable".

>> Also: Stray semicolon and (nit) missing blanks.
> 
> It is not clear to me where are the missing blanks.

Just like for other keywords:

    asm ( ".error \"unreachable program point reached\"" )

Jan