Date: Mon, 4 Jul 2011 23:49:17 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: Stefan Esser <se@freebsd.org> Cc: Garrett Wollman <wollman@csail.mit.edu>, freebsd-standards@freebsd.org Subject: Re: New patches for expr (was: Re: Fwd: [RFC] Consistent numeric range for "expr" on all architectures) Message-ID: <20110704214917.GA77218@stack.nl> In-Reply-To: <4E0E2535.1060204@freebsd.org> References: <4E09AF8E.5010509@freebsd.org> <4E0B860E.50504@freebsd.org> <20110630164653.GA82980@zim.MIT.EDU> <4E0CACFB.8040906@freebsd.org> <19980.47094.184089.398324@khavrinen.csail.mit.edu> <20110701092909.F1164@besplex.bde.org> <4E0E2535.1060204@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 01, 2011 at 09:51:17PM +0200, Stefan Esser wrote:
> The first file (expr.y.diff1) contains a "diff -w -u3" of the functional
> changes that I suggest. It is only meant to show the actual code changes
> (applying this diff results in mis-indented code).
On amd64, INT_MIN % -1 also causes a divide error SIGFPE. Therefore,
this check should not be skipped, but perhaps the result 0 should be
manually put in (in recent sh(1) both $((INT_MIN / -1)) and $((INT_MIN %
-1)) are errors).
The overflow checks assert_plus, assert_minus and assert_times are not
standard-compliant because they first let the overflow occur and then
attempt to check for it. The C standard permits an optimizing compiler
to remove the checks (or anything else, as the meaning of the entire
program is destroyed if there is overflow, but given that the program
must still work if there is no overflow, it is pretty hard to avoid
generating the proper add, subtract or multiply instructions).
Note that the checks were already wrong, so as long as you verify you do
not trigger new unexpected "optimizations", it should be OK to commit
regardless.
Sadly, C makes it really hard to check this correctly, while the CPU
(amd64 at least) provides a simple flag bit.
> I also include a SHAR of regression tests meant to be committed to
> /usr/src/tools/regression/bin/expr. All tests pass with the current
> patched version of expr (while the currently shipped version dumps core
> in one test case).
The tests still have "test" instead of "expr" at some places, see below.
> # This is a shell archive. Save it in a file, remove anything before
> # this line, and then unpack it by entering "sh file". Note, it may
> # create directories; files and directories will be owned by you and
> # have default permissions.
> #
> # This archive contains:
> #
> # expr/Makefile
> # expr/regress.sh
> # expr/regress.t
> #
> echo x - expr/Makefile
> sed 's/^X//' >expr/Makefile << 'f9600fcce6c6ed7d48045c253fb4f463'
> X# $FreeBSD: head/tools/regression/bin/test/Makefile 215022 2010-11-08 23:15:10Z jilles $
> X
> Xall:
> X sh regress.sh
> f9600fcce6c6ed7d48045c253fb4f463
> echo x - expr/regress.sh
> sed 's/^X//' >expr/regress.sh << '78ebeeef8ff008d708ef3dfc31f2d4b1'
> X#!/bin/sh
> X
> [snip]
> X
> X#
> X# expr.sh - check if test(1) or builtin test works
test -> expr
> X#
> X# $FreeBSD:$
> X
> X# force a specified test program, e.g. `env test=/bin/expr sh regress.sh'
> X: ${test=expr}
test -> expr
> X
> Xt ()
> X{
> X # $1 -> exit code
> X # $2 -> result written to stdout
> X # $3 -> $test expression
> X
> X count=$((count+1))
> X # check for syntax errors
Does this comment still mean something?
> X output="`eval $test $3 2>/dev/null`"
> X ret=$?
> X if [ "$ret" -ne "$1" ]; then
> X printf "not ok %s - (got $ret, expected $1)\n" "$count $3"
> X elif [ "$output" != "$2" ]; then
> X printf "not ok %s - (unexpected output)\n" "$count $3"
> X else
> X printf "ok %s\n" "$count $3"
> X fi
> X}
> X
> [snip]
--
Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110704214917.GA77218>