| Message ID | e28bb23a12fae3c8630f943b469137d367f20022.1706259490.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce and use STATIC_ASSERT_UNREACHABLE() | expand |
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? Jan
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
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 ... Jan
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 make[2]: *** Waiting for unfinished jobs.... ... 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?
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
On 07/02/24 15:16, Jan Beulich wrote: > 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. An example of that is the get_unsafe_size() macro, whose body uses STATIC_ASSERT_UNREACHABLE(). A wrong use of get_unsafe_size() at line n leads to a build error pointing to the line n, isn't this the desired behavior?
On 07.02.2024 16:08, Federico Serafini wrote: > On 07/02/24 15:16, Jan Beulich wrote: >> 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. > > An example of that is the get_unsafe_size() macro, > whose body uses STATIC_ASSERT_UNREACHABLE(). > A wrong use of get_unsafe_size() at line n > leads to a build error pointing to the line n, > isn't this the desired behavior? Aiui this would point to the line in the header file, when what you need to spot the bad use of the macro is the line in the source file actually using the macro. Quoting from an earlier mail of yours: ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable Compare this with what we have today, where the linker will point out the function it found the bad use in. Of course this could also be solved by better assembler diagnostics, but I'm not sure compiler output would actually lend itself to that. Specifically we'd then rely on the .type directive always preceding the actual function. Plus while it may be reasonably possible to change gas, I'm not as sure about Clang's integrated assembler. Plus changing gas and then getting it into use by people will take quite a bit of time. Jan
On 07/02/24 16:24, Jan Beulich wrote: > On 07.02.2024 16:08, Federico Serafini wrote: >> On 07/02/24 15:16, Jan Beulich wrote: >>> 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. >> >> An example of that is the get_unsafe_size() macro, >> whose body uses STATIC_ASSERT_UNREACHABLE(). >> A wrong use of get_unsafe_size() at line n >> leads to a build error pointing to the line n, >> isn't this the desired behavior? > > Aiui this would point to the line in the header file, when what you need > to spot the bad use of the macro is the line in the source file actually > using the macro. Quoting from an earlier mail of yours: > > ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable It points to the header file uaccess.h because at line 262 there is an intentional wrong use of put_guest_size(), within the body of __copy_to_guest_pv() function. This example can be misleading because {get,put}_unsafe_size() are defined in the same file but the diagnostics is doing the right thing.
On 07.02.2024 16:58, Federico Serafini wrote: > On 07/02/24 16:24, Jan Beulich wrote: >> On 07.02.2024 16:08, Federico Serafini wrote: >>> On 07/02/24 15:16, Jan Beulich wrote: >>>> 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. >>> >>> An example of that is the get_unsafe_size() macro, >>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>> A wrong use of get_unsafe_size() at line n >>> leads to a build error pointing to the line n, >>> isn't this the desired behavior? >> >> Aiui this would point to the line in the header file, when what you need >> to spot the bad use of the macro is the line in the source file actually >> using the macro. Quoting from an earlier mail of yours: >> >> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable > > It points to the header file uaccess.h because at line 262 there is > an intentional wrong use of put_guest_size(), within the body of > __copy_to_guest_pv() function. Yet that's again only a helper function being inlined into the ultimate caller. That ultimate caller is what wants identifying in the diag. Not the least because of ... > This example can be misleading because {get,put}_unsafe_size() are > defined in the same file but the diagnostics is doing the > right thing. ... this. And really __copy_to_guest_pv() is the wrong place to put a wrong put_guest_size() in, to try out how diagnostics would look like in reality: That function falls back to copy_to_guest_ll() for all cases it can't handle directly. You want to place a bogus put_guest() somewhere in a .c file to see what results. Jan
On 07/02/24 17:19, Jan Beulich wrote: > On 07.02.2024 16:58, Federico Serafini wrote: >> On 07/02/24 16:24, Jan Beulich wrote: >>> On 07.02.2024 16:08, Federico Serafini wrote: >>>> On 07/02/24 15:16, Jan Beulich wrote: >>>>> 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. >>>> >>>> An example of that is the get_unsafe_size() macro, >>>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>>> A wrong use of get_unsafe_size() at line n >>>> leads to a build error pointing to the line n, >>>> isn't this the desired behavior? >>> >>> Aiui this would point to the line in the header file, when what you need >>> to spot the bad use of the macro is the line in the source file actually >>> using the macro. Quoting from an earlier mail of yours: >>> >>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable >> >> It points to the header file uaccess.h because at line 262 there is >> an intentional wrong use of put_guest_size(), within the body of >> __copy_to_guest_pv() function. > > Yet that's again only a helper function being inlined into the ultimate > caller. That ultimate caller is what wants identifying in the diag. Not > the least because of ... > >> This example can be misleading because {get,put}_unsafe_size() are >> defined in the same file but the diagnostics is doing the >> right thing. > > ... this. And really __copy_to_guest_pv() is the wrong place to put a > wrong put_guest_size() in, to try out how diagnostics would look like > in reality: That function falls back to copy_to_guest_ll() for all > cases it can't handle directly. You want to place a bogus put_guest() > somewhere in a .c file to see what results. I added a bogus call to put_guest() at line 387 of file xen/arch/x86/mm.c, inside function page_is_ram_type(). Assuming I did not choose another wrong place, the diagnostic seems appropriate: arch/x86/mm.c: Assembler messages: arch/x86/mm.c:387: Error: static assertion failed: unreachable
On 08.02.2024 11:45, Federico Serafini wrote: > On 07/02/24 17:19, Jan Beulich wrote: >> On 07.02.2024 16:58, Federico Serafini wrote: >>> On 07/02/24 16:24, Jan Beulich wrote: >>>> On 07.02.2024 16:08, Federico Serafini wrote: >>>>> On 07/02/24 15:16, Jan Beulich wrote: >>>>>> 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. >>>>> >>>>> An example of that is the get_unsafe_size() macro, >>>>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>>>> A wrong use of get_unsafe_size() at line n >>>>> leads to a build error pointing to the line n, >>>>> isn't this the desired behavior? >>>> >>>> Aiui this would point to the line in the header file, when what you need >>>> to spot the bad use of the macro is the line in the source file actually >>>> using the macro. Quoting from an earlier mail of yours: >>>> >>>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable >>> >>> It points to the header file uaccess.h because at line 262 there is >>> an intentional wrong use of put_guest_size(), within the body of >>> __copy_to_guest_pv() function. >> >> Yet that's again only a helper function being inlined into the ultimate >> caller. That ultimate caller is what wants identifying in the diag. Not >> the least because of ... >> >>> This example can be misleading because {get,put}_unsafe_size() are >>> defined in the same file but the diagnostics is doing the >>> right thing. >> >> ... this. And really __copy_to_guest_pv() is the wrong place to put a >> wrong put_guest_size() in, to try out how diagnostics would look like >> in reality: That function falls back to copy_to_guest_ll() for all >> cases it can't handle directly. You want to place a bogus put_guest() >> somewhere in a .c file to see what results. > > I added a bogus call to put_guest() at line 387 of > file xen/arch/x86/mm.c, inside function page_is_ram_type(). > Assuming I did not choose another wrong place, > the diagnostic seems appropriate: > > arch/x86/mm.c: Assembler messages: > arch/x86/mm.c:387: Error: static assertion failed: unreachable Oh, okay, this looks appropriate then as to identifying where the source construct is. However, we then still don't know where the assertion in question is (there could be multiple in what the original construct expands to). So I'm still inclined to ask that __FILE__ / __LINE__ and/or the name of the invoking construct (macro or function) be made visible in the diagnostic. Jan
On 08/02/24 12:14, Jan Beulich wrote: > On 08.02.2024 11:45, Federico Serafini wrote: >> On 07/02/24 17:19, Jan Beulich wrote: >>> On 07.02.2024 16:58, Federico Serafini wrote: >>>> On 07/02/24 16:24, Jan Beulich wrote: >>>>> On 07.02.2024 16:08, Federico Serafini wrote: >>>>>> On 07/02/24 15:16, Jan Beulich wrote: >>>>>>> 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. >>>>>> >>>>>> An example of that is the get_unsafe_size() macro, >>>>>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>>>>> A wrong use of get_unsafe_size() at line n >>>>>> leads to a build error pointing to the line n, >>>>>> isn't this the desired behavior? >>>>> >>>>> Aiui this would point to the line in the header file, when what you need >>>>> to spot the bad use of the macro is the line in the source file actually >>>>> using the macro. Quoting from an earlier mail of yours: >>>>> >>>>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable >>>> >>>> It points to the header file uaccess.h because at line 262 there is >>>> an intentional wrong use of put_guest_size(), within the body of >>>> __copy_to_guest_pv() function. >>> >>> Yet that's again only a helper function being inlined into the ultimate >>> caller. That ultimate caller is what wants identifying in the diag. Not >>> the least because of ... >>> >>>> This example can be misleading because {get,put}_unsafe_size() are >>>> defined in the same file but the diagnostics is doing the >>>> right thing. >>> >>> ... this. And really __copy_to_guest_pv() is the wrong place to put a >>> wrong put_guest_size() in, to try out how diagnostics would look like >>> in reality: That function falls back to copy_to_guest_ll() for all >>> cases it can't handle directly. You want to place a bogus put_guest() >>> somewhere in a .c file to see what results. >> >> I added a bogus call to put_guest() at line 387 of >> file xen/arch/x86/mm.c, inside function page_is_ram_type(). >> Assuming I did not choose another wrong place, >> the diagnostic seems appropriate: >> >> arch/x86/mm.c: Assembler messages: >> arch/x86/mm.c:387: Error: static assertion failed: unreachable > > Oh, okay, this looks appropriate then as to identifying where the > source construct is. However, we then still don't know where the > assertion in question is (there could be multiple in what the > original construct expands to). So I'm still inclined to ask that > __FILE__ / __LINE__ and/or the name of the invoking construct > (macro or function) be made visible in the diagnostic. Any use of __FILE__ and __LINE__ results in obtaining the same information already reported by the assembler error message. We could add an argument to the new macro to manually add some context at every use of the macro, but I think this would be annoying.
On 09.02.2024 10:50, Federico Serafini wrote: > On 08/02/24 12:14, Jan Beulich wrote: >> On 08.02.2024 11:45, Federico Serafini wrote: >>> On 07/02/24 17:19, Jan Beulich wrote: >>>> On 07.02.2024 16:58, Federico Serafini wrote: >>>>> On 07/02/24 16:24, Jan Beulich wrote: >>>>>> On 07.02.2024 16:08, Federico Serafini wrote: >>>>>>> On 07/02/24 15:16, Jan Beulich wrote: >>>>>>>> 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. >>>>>>> >>>>>>> An example of that is the get_unsafe_size() macro, >>>>>>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>>>>>> A wrong use of get_unsafe_size() at line n >>>>>>> leads to a build error pointing to the line n, >>>>>>> isn't this the desired behavior? >>>>>> >>>>>> Aiui this would point to the line in the header file, when what you need >>>>>> to spot the bad use of the macro is the line in the source file actually >>>>>> using the macro. Quoting from an earlier mail of yours: >>>>>> >>>>>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable >>>>> >>>>> It points to the header file uaccess.h because at line 262 there is >>>>> an intentional wrong use of put_guest_size(), within the body of >>>>> __copy_to_guest_pv() function. >>>> >>>> Yet that's again only a helper function being inlined into the ultimate >>>> caller. That ultimate caller is what wants identifying in the diag. Not >>>> the least because of ... >>>> >>>>> This example can be misleading because {get,put}_unsafe_size() are >>>>> defined in the same file but the diagnostics is doing the >>>>> right thing. >>>> >>>> ... this. And really __copy_to_guest_pv() is the wrong place to put a >>>> wrong put_guest_size() in, to try out how diagnostics would look like >>>> in reality: That function falls back to copy_to_guest_ll() for all >>>> cases it can't handle directly. You want to place a bogus put_guest() >>>> somewhere in a .c file to see what results. >>> >>> I added a bogus call to put_guest() at line 387 of >>> file xen/arch/x86/mm.c, inside function page_is_ram_type(). >>> Assuming I did not choose another wrong place, >>> the diagnostic seems appropriate: >>> >>> arch/x86/mm.c: Assembler messages: >>> arch/x86/mm.c:387: Error: static assertion failed: unreachable >> >> Oh, okay, this looks appropriate then as to identifying where the >> source construct is. However, we then still don't know where the >> assertion in question is (there could be multiple in what the >> original construct expands to). So I'm still inclined to ask that >> __FILE__ / __LINE__ and/or the name of the invoking construct >> (macro or function) be made visible in the diagnostic. > > Any use of __FILE__ and __LINE__ results in obtaining > the same information already reported by the assembler error message. Hmm, yes, since put_guest() is itself a macro. > We could add an argument to the new macro to manually add some context > at every use of the macro, but I think this would be annoying. That's a last resort. An alternative would be to see about converting from macros to inline functions, where this would make a difference here. Jan
On 12/02/24 09:43, Jan Beulich wrote: > On 09.02.2024 10:50, Federico Serafini wrote: >> On 08/02/24 12:14, Jan Beulich wrote: >>> On 08.02.2024 11:45, Federico Serafini wrote: >>>> On 07/02/24 17:19, Jan Beulich wrote: >>>>> On 07.02.2024 16:58, Federico Serafini wrote: >>>>>> On 07/02/24 16:24, Jan Beulich wrote: >>>>>>> On 07.02.2024 16:08, Federico Serafini wrote: >>>>>>>> On 07/02/24 15:16, Jan Beulich wrote: >>>>>>>>> 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. >>>>>>>> >>>>>>>> An example of that is the get_unsafe_size() macro, >>>>>>>> whose body uses STATIC_ASSERT_UNREACHABLE(). >>>>>>>> A wrong use of get_unsafe_size() at line n >>>>>>>> leads to a build error pointing to the line n, >>>>>>>> isn't this the desired behavior? >>>>>>> >>>>>>> Aiui this would point to the line in the header file, when what you need >>>>>>> to spot the bad use of the macro is the line in the source file actually >>>>>>> using the macro. Quoting from an earlier mail of yours: >>>>>>> >>>>>>> ./arch/x86/include/asm/uaccess.h:262: Error: static assertion failed: unreachable >>>>>> >>>>>> It points to the header file uaccess.h because at line 262 there is >>>>>> an intentional wrong use of put_guest_size(), within the body of >>>>>> __copy_to_guest_pv() function. >>>>> >>>>> Yet that's again only a helper function being inlined into the ultimate >>>>> caller. That ultimate caller is what wants identifying in the diag. Not >>>>> the least because of ... >>>>> >>>>>> This example can be misleading because {get,put}_unsafe_size() are >>>>>> defined in the same file but the diagnostics is doing the >>>>>> right thing. >>>>> >>>>> ... this. And really __copy_to_guest_pv() is the wrong place to put a >>>>> wrong put_guest_size() in, to try out how diagnostics would look like >>>>> in reality: That function falls back to copy_to_guest_ll() for all >>>>> cases it can't handle directly. You want to place a bogus put_guest() >>>>> somewhere in a .c file to see what results. >>>> >>>> I added a bogus call to put_guest() at line 387 of >>>> file xen/arch/x86/mm.c, inside function page_is_ram_type(). >>>> Assuming I did not choose another wrong place, >>>> the diagnostic seems appropriate: >>>> >>>> arch/x86/mm.c: Assembler messages: >>>> arch/x86/mm.c:387: Error: static assertion failed: unreachable >>> >>> Oh, okay, this looks appropriate then as to identifying where the >>> source construct is. However, we then still don't know where the >>> assertion in question is (there could be multiple in what the >>> original construct expands to). So I'm still inclined to ask that >>> __FILE__ / __LINE__ and/or the name of the invoking construct >>> (macro or function) be made visible in the diagnostic. >> >> Any use of __FILE__ and __LINE__ results in obtaining >> the same information already reported by the assembler error message. > > Hmm, yes, since put_guest() is itself a macro. > >> We could add an argument to the new macro to manually add some context >> at every use of the macro, but I think this would be annoying. > > That's a last resort. An alternative would be to see about converting > from macros to inline functions, where this would make a difference > here. I did some tries with example programs and the assembler error always points to file and line of the most enclosing function that caused the failure. If I am not missing something, using __FILE__ and __LINE__ does not add any information. Therefore, if the new macro is used within the body of other macros, then the resulting assembler error will point to the source of the problem (e.g., the site of a bogus call to put_guest()). In my opinion, converting put_guest() &Co. to inline functions is not convenient: the assembler error will point to the most enclosing function that would be put_unsafe_size(), instead of pointing to the source of the problem.
On Wed, 14 Feb 2024, Federico Serafini wrote: > On 12/02/24 09:43, Jan Beulich wrote: > > On 09.02.2024 10:50, Federico Serafini wrote: > > > On 08/02/24 12:14, Jan Beulich wrote: > > > > On 08.02.2024 11:45, Federico Serafini wrote: > > > > > On 07/02/24 17:19, Jan Beulich wrote: > > > > > > On 07.02.2024 16:58, Federico Serafini wrote: > > > > > > > On 07/02/24 16:24, Jan Beulich wrote: > > > > > > > > On 07.02.2024 16:08, Federico Serafini wrote: > > > > > > > > > On 07/02/24 15:16, Jan Beulich wrote: > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > An example of that is the get_unsafe_size() macro, > > > > > > > > > whose body uses STATIC_ASSERT_UNREACHABLE(). > > > > > > > > > A wrong use of get_unsafe_size() at line n > > > > > > > > > leads to a build error pointing to the line n, > > > > > > > > > isn't this the desired behavior? > > > > > > > > > > > > > > > > Aiui this would point to the line in the header file, when what > > > > > > > > you need > > > > > > > > to spot the bad use of the macro is the line in the source file > > > > > > > > actually > > > > > > > > using the macro. Quoting from an earlier mail of yours: > > > > > > > > > > > > > > > > ./arch/x86/include/asm/uaccess.h:262: Error: static assertion > > > > > > > > failed: unreachable > > > > > > > > > > > > > > It points to the header file uaccess.h because at line 262 there > > > > > > > is > > > > > > > an intentional wrong use of put_guest_size(), within the body of > > > > > > > __copy_to_guest_pv() function. > > > > > > > > > > > > Yet that's again only a helper function being inlined into the > > > > > > ultimate > > > > > > caller. That ultimate caller is what wants identifying in the diag. > > > > > > Not > > > > > > the least because of ... > > > > > > > > > > > > > This example can be misleading because {get,put}_unsafe_size() are > > > > > > > defined in the same file but the diagnostics is doing the > > > > > > > right thing. > > > > > > > > > > > > ... this. And really __copy_to_guest_pv() is the wrong place to put > > > > > > a > > > > > > wrong put_guest_size() in, to try out how diagnostics would look > > > > > > like > > > > > > in reality: That function falls back to copy_to_guest_ll() for all > > > > > > cases it can't handle directly. You want to place a bogus > > > > > > put_guest() > > > > > > somewhere in a .c file to see what results. > > > > > > > > > > I added a bogus call to put_guest() at line 387 of > > > > > file xen/arch/x86/mm.c, inside function page_is_ram_type(). > > > > > Assuming I did not choose another wrong place, > > > > > the diagnostic seems appropriate: > > > > > > > > > > arch/x86/mm.c: Assembler messages: > > > > > arch/x86/mm.c:387: Error: static assertion failed: unreachable > > > > > > > > Oh, okay, this looks appropriate then as to identifying where the > > > > source construct is. However, we then still don't know where the > > > > assertion in question is (there could be multiple in what the > > > > original construct expands to). So I'm still inclined to ask that > > > > __FILE__ / __LINE__ and/or the name of the invoking construct > > > > (macro or function) be made visible in the diagnostic. > > > > > > Any use of __FILE__ and __LINE__ results in obtaining > > > the same information already reported by the assembler error message. > > > > Hmm, yes, since put_guest() is itself a macro. > > > > > We could add an argument to the new macro to manually add some context > > > at every use of the macro, but I think this would be annoying. > > > > That's a last resort. An alternative would be to see about converting > > from macros to inline functions, where this would make a difference > > here. > > I did some tries with example programs > and the assembler error always points to file and line > of the most enclosing function that caused the failure. > If I am not missing something, using __FILE__ and __LINE__ does not add > any information. > > Therefore, if the new macro is used within the body of other macros, > then the resulting assembler error will point to the source of > the problem (e.g., the site of a bogus call to put_guest()). > > In my opinion, converting put_guest() &Co. to inline functions is not > convenient: the assembler error will point to the most enclosing > function that would be put_unsafe_size(), instead of pointing to the > source of the problem. I don't think is a good idea to add further changes to this patch. I think we should go ahead with it as-is.
On 14.02.2024 17:11, Federico Serafini wrote: > I did some tries with example programs > and the assembler error always points to file and line > of the most enclosing function that caused the failure. > If I am not missing something, using __FILE__ and __LINE__ does not add > any information. > > Therefore, if the new macro is used within the body of other macros, > then the resulting assembler error will point to the source of > the problem (e.g., the site of a bogus call to put_guest()). > > In my opinion, converting put_guest() &Co. to inline functions is not > convenient: the assembler error will point to the most enclosing > function that would be put_unsafe_size(), instead of pointing to the > source of the problem. The assembler error will point to where the inline function was expanded, sure. __FILE__ / __LINE__ ought to point to that inline function (where the macro was used) then, though? Jan
On 15.02.2024 01:05, Stefano Stabellini wrote: > I don't think is a good idea to add further changes to this patch. I > think we should go ahead with it as-is. I didn't suggest adding anything right here; there may want/need to be new prereq-s, though. I'd like to make sure that we don't (significantly) regress in terms of being able to diagnose programming mistakes. Jan
On 15/02/24 09:10, Jan Beulich wrote: > On 14.02.2024 17:11, Federico Serafini wrote: >> I did some tries with example programs >> and the assembler error always points to file and line >> of the most enclosing function that caused the failure. >> If I am not missing something, using __FILE__ and __LINE__ does not add >> any information. >> >> Therefore, if the new macro is used within the body of other macros, >> then the resulting assembler error will point to the source of >> the problem (e.g., the site of a bogus call to put_guest()). >> >> In my opinion, converting put_guest() &Co. to inline functions is not >> convenient: the assembler error will point to the most enclosing >> function that would be put_unsafe_size(), instead of pointing to the >> source of the problem. > > The assembler error will point to where the inline function was expanded, > sure. __FILE__ / __LINE__ ought to point to that inline function (where > the macro was used) then, though? This is what I get: federico@Dell:~$ cat m.c #define STRINGIFY(arg) #arg #define STATIC_ASSERT_UNREACHABLE(file, line) \ asm(".error \"static assertion failed: " file ": " STRINGIFY(line) "\"") static inline __attribute__((always_inline)) void g(int x) { switch(x) { case 0: STATIC_ASSERT_UNREACHABLE(__FILE__, __LINE__); } } static inline __attribute__((always_inline)) void f(int x) { g(x); } int main(void) { f(0); return 0; } federico@Dell:~$ gcc -O3 m.c m.c: Assembler messages: m.c:8: Error: static assertion failed: m.c: 8 Note that the linker behaves differently: federico@Dell:~$ cat m.c extern void __put_user_bad(void); static inline __attribute__((always_inline)) void g(int x) { switch(x) { case 0: __put_user_bad(); } } static inline __attribute__((always_inline)) void f(int x) { g(x); } int main(void) { f(0); return 0; } federico@Dell:~$ gcc -O3 m.c /usr/bin/ld: /tmp/ccv9KHJD.o: in function `main': m.c:(.text.startup+0x9): undefined reference to `__put_user_bad' collect2: error: ld returned 1 exit status
On 15.02.2024 11:07, Federico Serafini wrote: > On 15/02/24 09:10, Jan Beulich wrote: >> On 14.02.2024 17:11, Federico Serafini wrote: >>> I did some tries with example programs >>> and the assembler error always points to file and line >>> of the most enclosing function that caused the failure. >>> If I am not missing something, using __FILE__ and __LINE__ does not add >>> any information. >>> >>> Therefore, if the new macro is used within the body of other macros, >>> then the resulting assembler error will point to the source of >>> the problem (e.g., the site of a bogus call to put_guest()). >>> >>> In my opinion, converting put_guest() &Co. to inline functions is not >>> convenient: the assembler error will point to the most enclosing >>> function that would be put_unsafe_size(), instead of pointing to the >>> source of the problem. >> >> The assembler error will point to where the inline function was expanded, >> sure. __FILE__ / __LINE__ ought to point to that inline function (where >> the macro was used) then, though? > > This is what I get: > > federico@Dell:~$ cat m.c > #define STRINGIFY(arg) #arg > #define STATIC_ASSERT_UNREACHABLE(file, line) \ > asm(".error \"static assertion failed: " file ": " STRINGIFY(line) "\"") __FILE__ / __LINE__, if to be used, want using here, not at the use site. > static inline __attribute__((always_inline)) void g(int x) { > switch(x) { > case 0: > STATIC_ASSERT_UNREACHABLE(__FILE__, __LINE__); > } > } > > static inline __attribute__((always_inline)) void f(int x) { > g(x); > } > > int main(void) { > f(0); > return 0; > } > federico@Dell:~$ gcc -O3 m.c > m.c: Assembler messages: > m.c:8: Error: static assertion failed: m.c: 8 That's as expected. There's no mix of macros and inline functions in your example. > Note that the linker behaves differently: > > federico@Dell:~$ cat m.c > extern void __put_user_bad(void); > > static inline __attribute__((always_inline)) void g(int x) { > switch(x) { > case 0: > __put_user_bad(); > } > } > > static inline __attribute__((always_inline)) void f(int x) { > g(x); > } > > int main(void) { > f(0); > return 0; > } > federico@Dell:~$ gcc -O3 m.c > /usr/bin/ld: /tmp/ccv9KHJD.o: in function `main': > m.c:(.text.startup+0x9): undefined reference to `__put_user_bad' > collect2: error: ld returned 1 exit status The important difference is: Here we're told that there was a use of __put_user_bad, which is easy to grep for, and thus see how the supplied function / file / line(?) relate to the ultimate problem. I'm afraid I'm meanwhile confused enough by the various replies containing results of experimentation that I can't really tell anymore what case is best. Hence I can only restate my expectation for an eventual v3: Diagnosing what the issue is, no matter whether the new macro is used in another macro or in an inline function, should not become meaningfully more difficult. In how far this is the case wants clarifying in the description of the change. Jan
On 15/02/24 11:32, Jan Beulich wrote: > The important difference is: Here we're told that there was a use of > __put_user_bad, which is easy to grep for, and thus see how the > supplied function / file / line(?) relate to the ultimate problem. > > I'm afraid I'm meanwhile confused enough by the various replies > containing results of experimentation that I can't really tell > anymore what case is best. Hence I can only restate my expectation for > an eventual v3: Diagnosing what the issue is, no matter whether the new > macro is used in another macro or in an inline function, should not > become meaningfully more difficult. In how far this is the case wants > clarifying in the description of the change. I think the best thing at the moment is to deviate __{get,put}_user_bad() for Rule 16.3. I'll let maintainers further explore the possibility of having a compile-time assertion based on the assembler error.
On Mon, 19 Feb 2024, Federico Serafini wrote: > On 15/02/24 11:32, Jan Beulich wrote: > > The important difference is: Here we're told that there was a use of > > __put_user_bad, which is easy to grep for, and thus see how the > > supplied function / file / line(?) relate to the ultimate problem. > > > > I'm afraid I'm meanwhile confused enough by the various replies > > containing results of experimentation that I can't really tell > > anymore what case is best. Hence I can only restate my expectation for > > an eventual v3: Diagnosing what the issue is, no matter whether the new > > macro is used in another macro or in an inline function, should not > > become meaningfully more difficult. In how far this is the case wants > > clarifying in the description of the change. > > I think the best thing at the moment is to deviate > __{get,put}_user_bad() for Rule 16.3. > I'll let maintainers further explore the possibility of having a > compile-time assertion based on the assembler error. OK. I hope Jan is OK to deviate by in-code comment.
On 19.02.2024 21:43, Stefano Stabellini wrote: > On Mon, 19 Feb 2024, Federico Serafini wrote: >> On 15/02/24 11:32, Jan Beulich wrote: >>> The important difference is: Here we're told that there was a use of >>> __put_user_bad, which is easy to grep for, and thus see how the >>> supplied function / file / line(?) relate to the ultimate problem. >>> >>> I'm afraid I'm meanwhile confused enough by the various replies >>> containing results of experimentation that I can't really tell >>> anymore what case is best. Hence I can only restate my expectation for >>> an eventual v3: Diagnosing what the issue is, no matter whether the new >>> macro is used in another macro or in an inline function, should not >>> become meaningfully more difficult. In how far this is the case wants >>> clarifying in the description of the change. >> >> I think the best thing at the moment is to deviate >> __{get,put}_user_bad() for Rule 16.3. >> I'll let maintainers further explore the possibility of having a >> compile-time assertion based on the assembler error. > > OK. I hope Jan is OK to deviate by in-code comment. Hmm, the follow-on suggestion was to add break statements? Followed by me asking whether adding noreturn to the decls wouldn't also help. (Then again I was under the impression that there was more than just the "missing" break statements which Misra thought was an issue here.) Jan
diff --git a/xen/arch/x86/include/asm/uaccess.h b/xen/arch/x86/include/asm/uaccess.h index 7443519d5b..52faf1d919 100644 --- a/xen/arch/x86/include/asm/uaccess.h +++ b/xen/arch/x86/include/asm/uaccess.h @@ -21,9 +21,6 @@ unsigned int copy_from_guest_ll(void *to, const void __user *from, unsigned int unsigned int copy_to_unsafe_ll(void *to, const void *from, unsigned int n); unsigned int copy_from_unsafe_ll(void *to, const void *from, unsigned int n); -extern long __get_user_bad(void); -extern void __put_user_bad(void); - #define UA_KEEP(args...) args #define UA_DROP(args...) @@ -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 )
Use STATIC_ASSERT_UNREACHABLE() to improve readability and anticipate the build failure (from a linker error to an assembler error) in case of wrong size. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- xen/arch/x86/include/asm/uaccess.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)