Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 22:01, Craig Rodrigues rodr...@freebsd.org wrote: Volatile is not the solution, it is completely orthogonal. The correct way would be to use unsigned integers, for which wrapping is defined, then convert those back and forth when presenting the results to the user. OK, converting expr.y to use unsigned integers would require a bit of work. Note that clang has, for a few releases, had builtins that allow overflow-checked operations and will generate very efficient code. In op_times, I believe the following should work: long long mul; #if __has_builtin(__builtin_smulll_overflow) if (__builtin_smulll_overflow(a-u.i, b-u.i, mul)) errx(ERR_EXIT, overflow); #else mul = a-u.i * b-u.i; #endif r = make_integer(mul); I don't know if recent versions of gcc implement these builtins yet. I think they were added to clang around 3.4, possibly slightly earlier. David ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 23:11, Garrett Cooper yaneurab...@gmail.com wrote: On Mar 22, 2015, at 15:09, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 23:04, Garrett Cooper yaneurab...@gmail.com wrote: On Mar 22, 2015, at 15:01, Craig Rodrigues rodr...@freebsd.org wrote: ... OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. +1 I’d still like to know why clang 3.5 doesn’t have this behavior though — there might be other potential issues lurking around that need to be solved (either here, in ports, or both). Because this version optimizes better around undefined behavior. There are most likely many issues lurking around, and most certainly in ports. I would recommend using UBSan to tackle this kind of thing. I hope this got a ports tinderbox run first… It is called an exp-run in ports land, but that only really tests whether ports *build* successfully. For most ports, there is no good way of testing them, certainly not if they don't have their own testing facilities. That said, I have built quite a number of ports with 3.6.0, even before the import, and saw zero problems until now. Then again, I'm just one user, and that is a very low sample size. ;) I don't expect a great many problems with integer wrapping though. You have to realize that quite a lot of open source projects already went through this phase, when gcc started optimizing integer wrapping a few years ago. We are just slowly catching up. Adding UBSan to tinderbox runs for toolchain upgrades [in the future] might be a good idea. Maybe, but please note that both ASan and UBSan are still beta-ish under FreeBSD, there will most likely be bumps along the road. Any bug reports in that area are welcome. -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Sun, Mar 22, 2015 at 3:01 PM, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 2:36 PM, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 22:32, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric d...@freebsd.org wrote: Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no Well, another alternative is to patch expr.y: Index: expr.y === --- expr.y (revision 280353) +++ expr.y (working copy) @@ -393,7 +393,7 @@ } void -assert_plus(intmax_t a, intmax_t b, intmax_t r) +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r) { /* * sum of two positive numbers must be positive, @@ -420,7 +420,7 @@ } void -assert_minus(intmax_t a, intmax_t b, intmax_t r) +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r) { /* special case subtraction of INTMAX_MIN */ if (b == INTMAX_MIN a 0) There were already some patches previously done to this file to add volatile, so maybe this would be OK to do. What do you think? Volatile is not the solution, it is completely orthogonal. The correct way would be to use unsigned integers, for which wrapping is defined, then convert those back and forth when presenting the results to the user. OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. Thanks for committing the fix. I wasn't aware of this topic, but it is explained quite nicely in this LLVM blog post: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html#signed_overflow Do you think we should further change expr.y with something like this: Index: expr.y === --- expr.y (revision 280357) +++ expr.y (working copy) @@ -445,12 +445,13 @@ } /* - * We depend on undefined behaviour giving a result (in r). - * To test this result, pass it as volatile. This prevents - * optimizing away of the test based on the undefined behaviour. + * We depend on undefined signed integer overflow behaviour + * giving a result (in r). + * This file must be compiled with the -fwrapv compiler + * flag which forces defined behavior for signed integer overflow. */ void -assert_times(intmax_t a, intmax_t b, volatile intmax_t r) +assert_times(intmax_t a, intmax_t b, intmax_t r) { /* * If the first operand is 0, no overflow is possible, -- Craig ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 22:02, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 11:26 AM, jenkins-ad...@freebsd.org wrote: See https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/867/ Can someone with toolchain expertise look at this? After the clang 3.6.1 import, /bin/expr behaves differently. With clang 3.5.0: # expr 4611686018427387904 + 4611686018427387904 expr: overflow With clang 3.6.1: # expr 4611686018427387904 + 4611686018427387904 -9223372036854775808 It works fine for me: $ /usr/obj/usr/src/bin/expr/expr 4611686018427387904 + 4611686018427387904 expr: overflow Are you using any special compilation flags? That said, having taken a quick look at expr.y, it does seem to rely on signed integer wrapping. -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 22:23, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 22:02, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 11:26 AM, jenkins-ad...@freebsd.org wrote: See https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/867/ Can someone with toolchain expertise look at this? After the clang 3.6.1 import, /bin/expr behaves differently. With clang 3.5.0: # expr 4611686018427387904 + 4611686018427387904 expr: overflow With clang 3.6.1: # expr 4611686018427387904 + 4611686018427387904 -9223372036854775808 It works fine for me: $ /usr/obj/usr/src/bin/expr/expr 4611686018427387904 + 4611686018427387904 expr: overflow Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Mar 22, 2015, at 14:36, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 22:32, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric d...@freebsd.org wrote: Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no Well, another alternative is to patch expr.y: Index: expr.y === --- expr.y (revision 280353) +++ expr.y (working copy) @@ -393,7 +393,7 @@ } void -assert_plus(intmax_t a, intmax_t b, intmax_t r) +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r) { /* * sum of two positive numbers must be positive, @@ -420,7 +420,7 @@ } void -assert_minus(intmax_t a, intmax_t b, intmax_t r) +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r) { /* special case subtraction of INTMAX_MIN */ if (b == INTMAX_MIN a 0) There were already some patches previously done to this file to add volatile, so maybe this would be OK to do. What do you think? Volatile is not the solution, it is completely orthogonal. The correct way would be to use unsigned integers, for which wrapping is defined, then convert those back and forth when presenting the results to the user. Before doing that — what changed in the past week that changed the behavior of expr? Thanks! signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Sun, Mar 22, 2015 at 11:26 AM, jenkins-ad...@freebsd.org wrote: See https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/867/ Can someone with toolchain expertise look at this? After the clang 3.6.1 import, /bin/expr behaves differently. With clang 3.5.0: # expr 4611686018427387904 + 4611686018427387904 expr: overflow With clang 3.6.1: # expr 4611686018427387904 + 4611686018427387904 -9223372036854775808 -- Craig ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Mar 22, 2015, at 14:02, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 11:26 AM, jenkins-ad...@freebsd.org wrote: See https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/867/ Can someone with toolchain expertise look at this? After the clang 3.6.1 import, /bin/expr behaves differently. With clang 3.5.0: # expr 4611686018427387904 + 4611686018427387904 expr: overflow With clang 3.6.1: # expr 4611686018427387904 + 4611686018427387904 -9223372036854775808 I suspect -fwrapv is at fault, but I need to do some digging first.. Thanks! signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 22:32, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric d...@freebsd.org wrote: Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no Well, another alternative is to patch expr.y: Index: expr.y === --- expr.y (revision 280353) +++ expr.y (working copy) @@ -393,7 +393,7 @@ } void -assert_plus(intmax_t a, intmax_t b, intmax_t r) +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r) { /* * sum of two positive numbers must be positive, @@ -420,7 +420,7 @@ } void -assert_minus(intmax_t a, intmax_t b, intmax_t r) +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r) { /* special case subtraction of INTMAX_MIN */ if (b == INTMAX_MIN a 0) There were already some patches previously done to this file to add volatile, so maybe this would be OK to do. What do you think? Volatile is not the solution, it is completely orthogonal. The correct way would be to use unsigned integers, for which wrapping is defined, then convert those back and forth when presenting the results to the user. -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric d...@freebsd.org wrote: Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no Well, another alternative is to patch expr.y: Index: expr.y === --- expr.y (revision 280353) +++ expr.y (working copy) @@ -393,7 +393,7 @@ } void -assert_plus(intmax_t a, intmax_t b, intmax_t r) +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r) { /* * sum of two positive numbers must be positive, @@ -420,7 +420,7 @@ } void -assert_minus(intmax_t a, intmax_t b, intmax_t r) +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r) { /* special case subtraction of INTMAX_MIN */ if (b == INTMAX_MIN a 0) There were already some patches previously done to this file to add volatile, so maybe this would be OK to do. What do you think? -- Craig ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Sun, Mar 22, 2015 at 2:36 PM, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 22:32, Craig Rodrigues rodr...@freebsd.org wrote: On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric d...@freebsd.org wrote: Ah right, that was on i386, on amd64 it does result in -2^63. It is indeed caused by reliance on signed integer wrapping. This diff should fix it, without rewriting the utility: Index: bin/expr/Makefile === --- bin/expr/Makefile (revision 280156) +++ bin/expr/Makefile (working copy) @@ -6,6 +6,9 @@ PROG= expr SRCS= expr.y YFLAGS= +# expr relies on signed integer wrapping +CFLAGS+= -fwrapv + NO_WMISSING_VARIABLE_DECLARATIONS= .if ${MK_TESTS} != no Well, another alternative is to patch expr.y: Index: expr.y === --- expr.y (revision 280353) +++ expr.y (working copy) @@ -393,7 +393,7 @@ } void -assert_plus(intmax_t a, intmax_t b, intmax_t r) +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r) { /* * sum of two positive numbers must be positive, @@ -420,7 +420,7 @@ } void -assert_minus(intmax_t a, intmax_t b, intmax_t r) +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r) { /* special case subtraction of INTMAX_MIN */ if (b == INTMAX_MIN a 0) There were already some patches previously done to this file to add volatile, so maybe this would be OK to do. What do you think? Volatile is not the solution, it is completely orthogonal. The correct way would be to use unsigned integers, for which wrapping is defined, then convert those back and forth when presenting the results to the user. OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. -- Craig ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Mar 22, 2015, at 15:01, Craig Rodrigues rodr...@freebsd.org wrote: ... OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. +1 I’d still like to know why clang 3.5 doesn’t have this behavior though — there might be other potential issues lurking around that need to be solved (either here, in ports, or both). Thanks! signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On 22 Mar 2015, at 23:04, Garrett Cooper yaneurab...@gmail.com wrote: On Mar 22, 2015, at 15:01, Craig Rodrigues rodr...@freebsd.org wrote: ... OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. +1 I’d still like to know why clang 3.5 doesn’t have this behavior though — there might be other potential issues lurking around that need to be solved (either here, in ports, or both). Because this version optimizes better around undefined behavior. There are most likely many issues lurking around, and most certainly in ports. I would recommend using UBSan to tackle this kind of thing. -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867
On Mar 22, 2015, at 15:09, Dimitry Andric d...@freebsd.org wrote: On 22 Mar 2015, at 23:04, Garrett Cooper yaneurab...@gmail.com wrote: On Mar 22, 2015, at 15:01, Craig Rodrigues rodr...@freebsd.org wrote: ... OK, converting expr.y to use unsigned integers would require a bit of work. Can you commit your patch to the Makefile? It fixes the problem for now. +1 I’d still like to know why clang 3.5 doesn’t have this behavior though — there might be other potential issues lurking around that need to be solved (either here, in ports, or both). Because this version optimizes better around undefined behavior. There are most likely many issues lurking around, and most certainly in ports. I would recommend using UBSan to tackle this kind of thing. I hope this got a ports tinderbox run first… Adding UBSan to tinderbox runs for toolchain upgrades [in the future] might be a good idea. signature.asc Description: Message signed with OpenPGP using GPGMail