svn commit: r227369 - head/bin/sh

[Bruce Evans]

On Wed, 9 Nov 2011, Jilles Tjoelker wrote:

> On Wed, Nov 09, 2011 at 09:35:51AM +0100, Stefan Farfeleder wrote:
>> On Tue, Nov 08, 2011 at 11:54:39PM +0000, Jilles Tjoelker wrote:
>>> Author: jilles
>>> Date: Tue Nov  8 23:54:39 2011
>>> New Revision: 227369
>>> URL: http://svn.freebsd.org/changeset/base/227369
>
>>> Log:
>>>   sh: Remove undefined behaviour due to overflow in +/-/* in arithmetic.
>
>>>   With i386 base gcc and i386 base clang, arith_yacc.o remains unchanged.
>
>>> Modified:
>>>   head/bin/sh/arith_yacc.c
>
>>> Modified: head/bin/sh/arith_yacc.c
>>> ==============================================================================
>>> --- head/bin/sh/arith_yacc.c	Tue Nov  8 23:44:26 2011	(r227368)
>>> +++ head/bin/sh/arith_yacc.c	Tue Nov  8 23:54:39 2011	(r227369)
>>> @@ -131,11 +131,11 @@ static arith_t do_binop(int op, arith_t
>>>  			yyerror("divide error");
>>>  		return op == ARITH_REM ? a % b : a / b;
>>>  	case ARITH_MUL:
>>> -		return a * b;
>>> +		return (uintmax_t)a * (uintmax_t)b;
>>>  	case ARITH_ADD:
>>> -		return a + b;
>>> +		return (uintmax_t)a + (uintmax_t)b;
>>>  	case ARITH_SUB:
>>> -		return a - b;
>>> +		return (uintmax_t)a - (uintmax_t)b;
>>>  	case ARITH_LSHIFT:
>>>  		return a << b;
>>>  	case ARITH_RSHIFT:
>
>> Isn't the behaviour undefined too when you convert an out-of-range
>> uintmax_t value back into an intmax_t value?
>
> The result is implementation-defined or an implementation-defined signal
> is raised.

C doesn't allow any signal, at least in C90 and n869.txt draft C99:

%        6.3.1.3  Signed and unsigned integers
%        ...
%        [#3] Otherwise, the new type is signed and the value  cannot
%        be  represented in it; the result is implementation-defined.
%        J.3  Implementation-defined behavior
%        ...
%        J.3.5  Integers
% 
%        [#1]
%        ...
%          -- The result of converting an integer to a signed integer
%             type when the value cannot be represented in an  object
%             of that type (6.3.1.3).

n869.txt barely mentions signals, especially here.  Its only literal
match for "signal raised" is in Annex H for LIA, which says that if
an arithmetic exception raises a signal, then the signal shall be
SIGFPE, and this is mainly for floating point.  It has many more literal
matches for "exception raised", since Annex F for IEEE754 requires
exceptions to be raised a lot; these exceptions normally don't generate
signals.

> GCC documentation (gcc.info 4.5 Integers implementation) says this
>
> ] * `The result of, or the signal raised by, converting an integer to a
> ]   signed integer type when the value cannot be represented in an
> ]   object of that type (C90 6.2.1.2, C99 6.3.1.3).'

", or the signal raised by, " in this seems to be a bug in gcc
documentation.  The documentation of implementation-defined behaviour
shouldn't mention that specifed behaviour is implemented, at least
without distinguishing the part that is as specified.

>
> ]   For conversion to a type of width N, the value is reduced modulo
> ]   2^N to be within range of the type; no signal is raised.
>
> which is exactly what we need.

Of course, a correct implementation would give a random result, so that
no one depends on implementation-defined behaviour.

Bruce