Bug 191719
| Summary: | [patch] expr(1) doesn't detect some overflow errors with multiplication | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Enji Cooper <ngie> |
| Component: | bin | Assignee: | freebsd-bugs (Nobody) <bugs> |
| Status: | Closed DUPLICATE | ||
| Severity: | Affects Some People | CC: | jilles |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
A proposed fix can be found here: https://github.com/yaneurabeya/freebsd/commit/14fd327099ddd5421b095a46fdb9077c07dcc6e7 . It passes all of the (slightly) modified NetBSD t_expr testcases. All the overflow checks except for division and modulo are fundamentally broken because they first let overflow happen and then attempt to check for it. Since signed integer overflow is undefined behaviour, this allows the compiler to optimize the overflow checks away, or worse. Also, INTMAX_MIN modulo -1 causes SIGFPE on amd64, like expr -e -- $((-0x7fffffffffffffff - 1)) % -1 Either op_rem() should not sabotage assert_div()'s check or op_rem() should return a zero-valued integer for anything modulo -1. se@ is resolving this as part of PR 196867. Resolving as a duplicate. *** This bug has been marked as a duplicate of bug 196867 *** |
If I do the following case, it doesn't fail on FreeBSD like it does NetBSD: % expr -- -4611686018427387904 \* 3 4611686018427387904 # python2 -c 'print -4611686018427387904 * 3' -13835058055282163712 This is could be fixed by adding a signage check to assert_times, like what's in assert_plus: if the signage of two numbers is not the same, i.e. one is negative, the other is positive -- then the value must be negative. 395 void 396 assert_plus(intmax_t a, intmax_t b, intmax_t r) 397 { 398 /* 399 * sum of two positive numbers must be positive, 400 * sum of two negative numbers must be negative 401 */ 402 if ((a > 0 && b > 0 && r <= 0) || 403 (a < 0 && b < 0 && r >= 0)) 404 errx(ERR_EXIT, "overflow"); 405 } ... 447 void 448 assert_times(intmax_t a, intmax_t b, intmax_t r) 449 { 450 /* 451 * if first operand is 0, no overflow is possible, 452 * else result of division test must match second operand 453 */ 454 if (a != 0 && r / a != b) 455 errx(ERR_EXIT, "overflow"); 456 }