Re: [XEN PATCH v2 2/3] x86/uaccess: replace __{get,put}_user_bad() with STATIC_ASSERT_UNREACHABLE()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.02.2024 14:51, Federico Serafini wrote:
> On 07/02/24 08:38, Jan Beulich wrote:
>> On 07.02.2024 02:08, Stefano Stabellini wrote:
>>> On Tue, 6 Feb 2024, Jan Beulich wrote:
>>>> On 26.01.2024 11:05, Federico Serafini wrote:
>>>>> @@ -208,7 +205,7 @@ do {                                                  
>>>>>                      \
>>>>>       case 8:                                                             
>>>>>    \
>>>>>           put_unsafe_asm(x, ptr, grd, retval, "q",  "", "ir", errret);    
>>>>>    \
>>>>>           break;                                                          
>>>>>    \
>>>>> -    default: __put_user_bad();                                           
>>>>>   \
>>>>> +    default: STATIC_ASSERT_UNREACHABLE();                                
>>>>>   \
>>>>>       }                                                                   
>>>>>    \
>>>>>       clac();                                                             
>>>>>    \
>>>>>   } while ( false )
>>>>> @@ -227,7 +224,7 @@ do {                                                  
>>>>>                      \
>>>>>       case 2: get_unsafe_asm(x, ptr, grd, retval, "w", "=r", errret); 
>>>>> break; \
>>>>>       case 4: get_unsafe_asm(x, ptr, grd, retval, "k", "=r", errret); 
>>>>> break; \
>>>>>       case 8: get_unsafe_asm(x, ptr, grd, retval,  "", "=r", errret); 
>>>>> break; \
>>>>> -    default: __get_user_bad();                                           
>>>>>   \
>>>>> +    default: STATIC_ASSERT_UNREACHABLE();                                
>>>>>   \
>>>>>       }                                                                   
>>>>>    \
>>>>>       clac();                                                             
>>>>>    \
>>>>>   } while ( false )
>>>>
>>>> Related to my remark on patch 1 - how is one to know the macro this was
>>>> invoked from, when seeing the resulting diagnostic?
>>>
>>> I am not sure what do you mean here... we do get an error like the
>>> following (I added a STATIC_ASSERT_UNREACHABLE for case 4):
>>>
>>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: 
>>> unreachable
>>
>> Right - and how do I know what _user_ of the macro actually triggered
>> it? ISTR suggesting to use one or more of __FILE__ / __LINE__ /
>> __FUNCTION__ here, for that specific purpose ...
> 
> To test the macro and its diagnostics,
> I modified the first "git grep" occurrence of ASSERT_UNREACHABLE()
> on the x86 code with STATIC_ASSERT_UNREACHABLE(),
> that is in file arch/x86/alternative.c, line 312,
> function _apply_alternatives().
> 
> What I got is the following build error:
> 
> ...
> arch/x86/alternative.c: Assembler messages:
> arch/x86/alternative.c:312: Error: static assertion failed: unreachable
>    CC      arch/x86/copy_page.o
> make[2]: *** [Rules.mk:247: arch/x86/alternative.o] Error 1

But that's not what my request was about. Here sufficient context is
given, even if it would be nice if the function was also visible right
away. But that's not the same as the case above, where the new macro
is used inside another macro.

> If I understood your requests correctly,
> the only thing missing is the function name but I didn't find a way
> to make __FUNCTION__ or __func__ work with the .error directive.
> Do you know any tricks to make it work?

I didn't think any tricks would be required:

asm ( ".error " __FILE__ ":" __LINE__ ": in function " __FUNCTION__ );

Yet it looks like I was under the wrong impression that __FUNCTION__
differed from __func__ and would be like __FILE__ / __LINE__. I have
to admit I have no good idea then how to achieve helpful diagnostics.

Jan