Bug 191719 - [patch] expr(1) doesn't detect some overflow errors with multiplication
Summary: [patch] expr(1) doesn't detect some overflow errors with multiplication
Status: Closed DUPLICATE of bug 196867
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-07 19:12 UTC by Enji Cooper
Modified: 2015-02-05 11:13 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Enji Cooper freebsd_committer 2014-07-07 19:12:21 UTC
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 }
Comment 1 Enji Cooper freebsd_committer 2014-07-07 19:46:54 UTC
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.
Comment 2 Jilles Tjoelker freebsd_committer 2014-07-07 21:51:34 UTC
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.
Comment 3 Enji Cooper freebsd_committer 2015-02-05 11:13:38 UTC
se@ is resolving this as part of PR 196867. Resolving as a duplicate.

*** This bug has been marked as a duplicate of bug 196867 ***