[src/trunk]: src/bin/expr Rework perform_arith_op() in expr(1) to omit Undefi...

[kamil <kamil%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/7a99f6a825aa
branches:  trunk
changeset: 319811:7a99f6a825aa
user:      kamil <kamil%NetBSD.org@localhost>
date:      Tue Jun 12 18:12:18 2018 +0000

description:
Rework perform_arith_op() in expr(1) to omit Undefined Behavior

The current implementation of operations - + * / % could cause Undefined
Behavior and in narrow cases (INT64_MIN / -1 and INT64_MIN % -1) SIGFPE
and crash duping core.

Detected with MKSANITIZER enabled for the Undefined Behavior variation:
# eval expr '4611686018427387904 + 4611686018427387904'
/public/src.git/bin/expr/expr.y:315:12: runtime error: signed integer overflow: 4611686018427387904 + 4611686018427387904 cannot be represented in type 'long'

All bin/t_expr ATF tests pass now in a sanitized userland.

Sponsored by <The NetBSD Foundation>

diffstat:

 bin/expr/expr.y |  79 ++++++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 39 deletions(-)

diffs (132 lines):

diff -r 49ec79949d2f -r 7a99f6a825aa bin/expr/expr.y
--- a/bin/expr/expr.y   Tue Jun 12 15:41:35 2018 +0000
+++ b/bin/expr/expr.y   Tue Jun 12 18:12:18 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: expr.y,v 1.39 2016/09/05 01:00:07 sevan Exp $ */
+/* $NetBSD: expr.y,v 1.40 2018/06/12 18:12:18 kamil Exp $ */
 
 /*_
  * Copyright (c) 2000 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
 %{
 #include <sys/cdefs.h>
 #ifndef lint
-__RCSID("$NetBSD: expr.y,v 1.39 2016/09/05 01:00:07 sevan Exp $");
+__RCSID("$NetBSD: expr.y,v 1.40 2018/06/12 18:12:18 kamil Exp $");
 #endif /* not lint */
 
 #include <sys/types.h>
@@ -273,8 +273,7 @@
 static int64_t
 perform_arith_op(const char *left, const char *op, const char *right)
 {
-       int64_t res, sign, l, r;
-       u_int64_t temp;
+       int64_t res, l, r;
 
        res = 0;
 
@@ -307,66 +306,68 @@
 
        switch(op[0]) {
        case '+':
-               /* 
-                * Do the op into an unsigned to avoid overflow and then cast
-                * back to check the resulting signage. 
+               /*
+                * Check for over-& underflow.
                 */
-               temp = l + r;
-               res = (int64_t) temp;
-               /* very simplistic check for over-& underflow */
-               if ((res < 0 && l > 0 && r > 0)
-                   || (res > 0 && l < 0 && r < 0)) 
+               if ((l > 0 && r <= INT64_MAX - l) ||
+                   (l < 0 && r >= INT64_MIN - l)) {
+                       res = l + r;
+               } else {
                        yyerror("integer overflow or underflow occurred for "
                             "operation '%s %s %s'", left, op, right);
+               }
                break;
        case '-':
-               /* 
-                * Do the op into an unsigned to avoid overflow and then cast
-                * back to check the resulting signage. 
+               /*
+                * Check for over-& underflow.
                 */
-               temp = l - r;
-               res = (int64_t) temp;
-               /* very simplistic check for over-& underflow */
-               if ((res < 0 && l > 0 && l > r)
-                   || (res > 0 && l < 0 && l < r) ) 
+               if ((r > 0 && l < INT64_MIN + r) ||
+                   (r < 0 && l > INT64_MAX + r)) {
                        yyerror("integer overflow or underflow occurred for "
                            "operation '%s %s %s'", left, op, right);
+               } else {
+                       res = l - r;
+               }
                break;
        case '/':
-               if (r == 0) 
+               if (r == 0)
                        yyerror("second argument to '%s' must not be zero", op);
+               if (l == INT64_MIN && r == -1)
+                       yyerror("integer overflow or underflow occurred for "
+                           "operation '%s %s %s'", left, op, right);
                res = l / r;
                        
                break;
        case '%':
                if (r == 0)
                        yyerror("second argument to '%s' must not be zero", op);
+               if (l == INT64_MIN && r == -1)
+                       yyerror("integer overflow or underflow occurred for "
+                           "operation '%s %s %s'", left, op, right);
                res = l % r;
                break;
        case '*':
-               /* shortcut */
-               if ((l == 0) || (r == 0)) {
-                       res = 0;
-                       break;
+               /*
+                * Check for over-& underflow.
+                */
+
+               /* Simplify the conditions */
+               if (l < 0 && r < 0 && l != INT64_MIN && r != INT64_MIN) {
+                       l = -l;
+                       r = -r;
                }
-                               
-               sign = 1;
-               if (l < 0)
-                       sign *= -1;
-               if (r < 0)
-                       sign *= -1;
 
-               res = l * r;
-               /*
-                * XXX: not the most portable but works on anything with 2's
-                * complement arithmetic. If the signs don't match or the
-                * result was 0 on 2's complement this overflowed.
-                */
-               if ((res < 0 && sign > 0) || (res > 0 && sign < 0) || 
-                   (res == 0))
+               if ((l < 0 && r < 0) ||
+                   ((l != 0 && r != 0) &&
+                    (((l > 0 && r > 0) && (l > INT64_MAX / r)) ||
+                    ((((l < 0 && r > 0) || (l > 0 && r < 0)) &&
+                     (r != -1 && (l < INT64_MIN / r))))))) {
                        yyerror("integer overflow or underflow occurred for "
                            "operation '%s %s %s'", left, op, right);
                        /* NOTREACHED */
+               } else {
+                       res = l * r;
+               }
                break;
        }
        return res;