Message ID | 1420827419-18655-3-git-send-email-rth@twiddle.net |
---|---|
State | New |
Headers | show |
On 09 Jan 2015 10:16, Richard Henderson wrote: > Finally, use __builtin_unreachable to tell the compiler about > impossible paths. i'm not sure this is the way to go. the expectation (as required by POSIX) is that you're guaranteed there are no side-effects when NDEBUG is applied to assert. by using __builtin_unreachable, that is no longer the case. yes, assert in this context is internal to glibc and so doesn't have to conform to POSIX directly. but i think forcing developers to remember this kind of nuance just leads to bugs, especially when you consider glibc imports code from other projects (like gnulib). the code shrinkage is attractive, but the other factors considered (and that NDEBUG usage in glibc in general is uncommon) leads me to think this path is not worth the pain. -mike
On 03/01/2015 11:43 PM, Mike Frysinger wrote: > On 09 Jan 2015 10:16, Richard Henderson wrote: >> Finally, use __builtin_unreachable to tell the compiler about >> impossible paths. > > i'm not sure this is the way to go. the expectation (as required by POSIX) > is that you're guaranteed there are no side-effects when NDEBUG is applied > to assert. by using __builtin_unreachable, that is no longer the case. I wonder if, during the gcc 6 development cycle, we should experiment with a __builtin_side_effects_p, akin to __builtin_constant_p. Then we could still follow POSIX re no side effects but even in the external context provide the information derivable from __builtin_unreachable. I.e. #define assert(X) \ (__builtin_side_effects_p (X) || !(X) ? 0 : __builtin_unreachable ()) r~
On Mon, Mar 02, 2015 at 09:18:31AM -0800, Richard Henderson wrote: > On 03/01/2015 11:43 PM, Mike Frysinger wrote: > > On 09 Jan 2015 10:16, Richard Henderson wrote: > >> Finally, use __builtin_unreachable to tell the compiler about > >> impossible paths. > > > > i'm not sure this is the way to go. the expectation (as required by POSIX) > > is that you're guaranteed there are no side-effects when NDEBUG is applied > > to assert. by using __builtin_unreachable, that is no longer the case. > > I wonder if, during the gcc 6 development cycle, we should experiment with a > __builtin_side_effects_p, akin to __builtin_constant_p. Then we could still > follow POSIX re no side effects but even in the external context provide the > information derivable from __builtin_unreachable. I.e. > > #define assert(X) \ > (__builtin_side_effects_p (X) || !(X) ? 0 : __builtin_unreachable ()) Tom wrote a patch implementing something like that a while back (for C only) <https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01984.html>, but the patch never made it in. Maybe I could revive and adjust it during gcc 6 stage 1 if it would help you. Marek
> i'm not sure this is the way to go. the expectation (as required by POSIX) > is that you're guaranteed there are no side-effects when NDEBUG is applied > to assert. by using __builtin_unreachable, that is no longer the case. The requirement (which should be in ISO C, not in POSIX) is specified at the C language level. "No side effects" at the C level certainly does not preclude a different correct translation of the program to machine code. > the code shrinkage is attractive, but the other factors considered (and that > NDEBUG usage in glibc in general is uncommon) leads me to think this path is > not worth the pain. In fact, many distributions build libc with -DNDEBUG. In terms of quantity of execution that happens, I'd bet most use of libc is with -DNDEBUG. Thanks, Roland
> I wonder if, during the gcc 6 development cycle, we should experiment with a > __builtin_side_effects_p, akin to __builtin_constant_p. Then we could still > follow POSIX re no side effects but even in the external context provide the > information derivable from __builtin_unreachable. I.e. > > #define assert(X) \ > (__builtin_side_effects_p (X) || !(X) ? 0 : __builtin_unreachable ()) It would be ideal to do this in a way that some new warning option or fortify enablement or whatnot can give compile-time warnings about side effects in assert expressions. How uselessly false-positivey that would be depends on how much a particular codebase uses the style of data gathering or consistency-checking function calls in assert expressions (i.e., things that the compiler would consider to have side effects, perhaps even with complete and perfect inlining, but the programmer reasonably considers to have no actual semantic side effects).
On 02 Mar 2015 14:56, Roland McGrath wrote: > > i'm not sure this is the way to go. the expectation (as required by POSIX) > > is that you're guaranteed there are no side-effects when NDEBUG is applied > > to assert. by using __builtin_unreachable, that is no longer the case. > > The requirement (which should be in ISO C, not in POSIX) is specified at > the C language level. i don't have an ISO C reference, so i look at POSIX, and it explicitly defines it this way. > "No side effects" at the C level certainly does not > preclude a different correct translation of the program to machine code. sure ... i wasn't saying otherwise. my point is that __builtin_unreachable doesn't do that -- it explicitly is documented by gcc as: If control flow reaches the point of the __builtin_unreachable, the program is undefined. if there was a solution as Richard alluded to that allows for expansion of the expression all the time w/out breaking things when the assert is actually hit, i'm all for that. -mike
> > "No side effects" at the C level certainly does not > > preclude a different correct translation of the program to machine code. > > sure ... i wasn't saying otherwise. my point is that __builtin_unreachable > doesn't do that -- it explicitly is documented by gcc as: > If control flow reaches the point of the __builtin_unreachable, the program is > undefined. > > if there was a solution as Richard alluded to that allows for expansion of the > expression all the time w/out breaking things when the assert is actually hit, > i'm all for that. I don't understand what your concern is. A call to __assert_fail leads to a call to abort and that is no more or less well-defined or recoverable than __builtin_unreachable.
On 04 Mar 2015 11:25, Roland McGrath wrote: > > > "No side effects" at the C level certainly does not > > > preclude a different correct translation of the program to machine code. > > > > sure ... i wasn't saying otherwise. my point is that __builtin_unreachable > > doesn't do that -- it explicitly is documented by gcc as: > > If control flow reaches the point of the __builtin_unreachable, the program is > > undefined. > > > > if there was a solution as Richard alluded to that allows for expansion of the > > expression all the time w/out breaking things when the assert is actually hit, > > i'm all for that. > > I don't understand what your concern is. A call to __assert_fail leads to > a call to abort and that is no more or less well-defined or recoverable > than __builtin_unreachable. yes, when NDEBUG is not defined. but that's not what we're discussing here. if your intention is for assert statements to crash the program even when NDEBUG is defined, then what exactly is the point of it ? why don't we just delete it instead ? -mike
diff --git a/include/assert.h b/include/assert.h index d7e2759..bd3ef51 100644 --- a/include/assert.h +++ b/include/assert.h @@ -29,6 +29,6 @@ hidden_proto (__assert_perror_fail) #ifdef NDEBUG # undef assert # undef assert_perror -# define assert(expr) ((void)(0 && (expr))) -# define assert_perror(errnum) ((void)(0 && (errnum))) +# define assert(e) ((e) ? (void)0 : __builtin_unreachable ()) +# define assert_perror(e) (!(e) ? (void)0 : __builtin_unreachable ()) #endif