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
