Skip to content

Commit

Permalink
Remove undefined behaviour due to possible signed integer overflows
Browse files Browse the repository at this point in the history 
Close #223
  • Loading branch information
@tueda
tueda committed Oct 26, 2017
1 parent 364f6d5 commit 954024f
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 18 deletions.
6 changes: 4 additions & 2 deletions sources/compiler.c
View file
Expand Up @@ -1235,6 +1235,7 @@ doexpr: s += 2;
else {
dofunction: firstsumarg = 1;
do {
unsigned int ux2;
s++;
c = *s++;
if ( c == TMINUS && ( *s == TVECTOR || *s == TNUMBER
Expand All @@ -1243,7 +1244,8 @@ dofunction: firstsumarg = 1;
}
else minus = 0;
base = ( c == TNUMBER ) ? 100: 128;
x2 = 0; while ( *s >= 0 ) { x2 = base*x2 + *s++; }
ux2 = 0; while ( *s >= 0 ) { ux2 = base*ux2 + *s++; }
x2 = ux2; /* may cause an implementation-defined behaviour */
/*
!!!!!!!! What if it does not fit?
*/
Expand Down Expand Up @@ -1399,7 +1401,7 @@ dofunction: firstsumarg = 1;
break;
case TNUMBER:
case TNUMBER1:
if ( minus ) x2 = -x2;
if ( minus ) x2 = UnsignedToInt(-IntAbs(x2));
*t++ = -SNUMBER;
*t++ = x2;
break;
Expand Down
43 changes: 42 additions & 1 deletion sources/declare.h
View file
Expand Up @@ -339,7 +339,7 @@ TP=T+1;while(TP<TT){if(*TP==AR.PolyFun){TP[2]|=(DIRTYFLAG|MUSTCLEANPRF);}TP+=TP[
*/

/*
* The following two functions give the unsigned absolute value of a signed
* The following three functions give the unsigned absolute value of a signed
* integer even for the most negative integer. This is beyond the scope of
* the standard abs() function and its family, whose return-values are signed.
* In short, we should not use the unary minus operator with signed numbers
Expand All @@ -353,6 +353,12 @@ TP=T+1;while(TP<TT){if(*TP==AR.PolyFun){TP[2]|=(DIRTYFLAG|MUSTCLEANPRF);}TP+=TP[
* https://stackoverflow.com/q/1610947 (Why does stdlib.h's abs() family of functions return a signed value?)
* https://blog.regehr.org/archives/226 (A Guide to Undefined Behavior in C and C++, Part 2)
*/
static inline unsigned int IntAbs(int x)
{
if ( x >= 0 ) return x;
return(-((unsigned int)x));
}

static inline UWORD WordAbs(WORD x)
{
if ( x >= 0 ) return x;
Expand All @@ -365,6 +371,41 @@ static inline ULONG LongAbs(LONG x)
return(-((ULONG)x));
}

/*
* The following functions provide portable unsigned-to-signed conversions
* (to avoid the implementation-defined behaviour), which is expected to be
* optimized to a no-op.
*
* See also:
* https://stackoverflow.com/a/13208789 (Efficient unsigned-to-signed cast avoiding implementation-defined behavior)
*/
static inline int UnsignedToInt(unsigned int x)
{
extern void Terminate(int);
if ( x <= INT_MAX ) return(x);
if ( x >= (unsigned int)INT_MIN ) return((int)(x - INT_MIN) + INT_MIN);
Terminate(1);
return(0);
}

static inline WORD UWordToWord(UWORD x)
{
extern void Terminate(int);
if ( x <= WORD_MAX_VALUE ) return(x);
if ( x >= (UWORD)WORD_MIN_VALUE ) return((WORD)(x - WORD_MIN_VALUE) + WORD_MIN_VALUE);
Terminate(1);
return(0);
}

static inline LONG ULongToLong(ULONG x)
{
extern void Terminate(int);
if ( x <= LONG_MAX_VALUE ) return(x);
if ( x >= (ULONG)LONG_MIN_VALUE ) return((LONG)(x - LONG_MIN_VALUE) + LONG_MIN_VALUE);
Terminate(1);
return(0);
}

/*
#] Inline functions :
#[ Thread objects :
Expand Down
16 changes: 16 additions & 0 deletions sources/form3.h
View file
Expand Up @@ -236,6 +236,11 @@ typedef unsigned long ULONG;
#error INT64 is not available!
#endif

#define WORD_MIN_VALUE SHRT_MIN
#define WORD_MAX_VALUE SHRT_MAX
#define LONG_MIN_VALUE LONG_MIN
#define LONG_MAX_VALUE LONG_MAX

#elif defined(LLP64)

typedef int WORD;
Expand All @@ -249,6 +254,11 @@ typedef unsigned long long ULONG;
#define INT64 long long
#undef INT128

#define WORD_MIN_VALUE INT_MIN
#define WORD_MAX_VALUE INT_MAX
#define LONG_MIN_VALUE LLONG_MIN
#define LONG_MAX_VALUE LLONG_MAX

#elif defined(LP64)

typedef int WORD;
Expand All @@ -262,6 +272,11 @@ typedef unsigned long ULONG;
#define INT64 long
#undef INT128

#define WORD_MIN_VALUE INT_MIN
#define WORD_MAX_VALUE INT_MAX
#define LONG_MIN_VALUE LONG_MIN
#define LONG_MAX_VALUE LONG_MAX

#else
#error ILP32 or LLP64 or LP64 must be defined!
#endif
Expand Down Expand Up @@ -403,6 +418,7 @@ template<typename T> struct calc {
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <limits.h>
#ifdef ANSI
#include <stdarg.h>
#include <time.h>
Expand Down
12 changes: 9 additions & 3 deletions sources/pre.c
View file
Expand Up @@ -4953,7 +4953,9 @@ UBYTE *PreEval(UBYTE *s, LONG *x)
for(;;){
while ( *s == ' ' || *s == '\t' ) s++;
if ( *s <= '9' && *s >= '0' ) {
ParseNumber(y,s)
ULONG uy;
ParseNumber(uy,s)
y = uy; /* may cause an implementation-defined behaviour */
}
else if ( *s == '(' || *s == '{' ) {
if ( ( t = PreEval(s+1,&y) ) == 0 ) return(0);
Expand Down Expand Up @@ -5059,8 +5061,12 @@ UBYTE *PreEval(UBYTE *s, LONG *x)
else if ( *s == '&' ) tobemultiplied = 3;
else if ( *s == '|' ) tobemultiplied = 4;
else {
if ( tobeadded >= 0 ) *x += a;
else *x -= a;
ULONG ux, ua;
ux = *x;
ua = a;
if ( tobeadded >= 0 ) ux += ua;
else ux -= ua;
*x = ULongToLong(ux);
if ( *s == ')' || *s == '}' ) return(s+1);
else if ( *s == '-' || *s == '+' ) { tobeadded = 1; break; }
else return(0);
Expand Down
2 changes: 1 addition & 1 deletion sources/reken.c
View file
Expand Up @@ -3782,7 +3782,7 @@ void iniwranf(PHEAD0)
for ( i = 0; i <= (ULONG)(imax); i++ ) {
ii = (AR.wranfnpair1*i)%AR.wranfnpair2;
AR.wranfia[ii] = k;
k = j - k;
k = ULongToLong((ULONG)j - (ULONG)k);
if ( k < 0 ) k += (LONG)1 << (2*BITSINWORD-2);
j = AR.wranfia[ii];
}
Expand Down
8 changes: 4 additions & 4 deletions sources/token.c
View file
Expand Up @@ -980,14 +980,14 @@ int simp2token(SBYTE *s)
}
else {
if ( *v == TNUMBER || *v == TNUMBER1 ) {
if ( BITSINWORD == 16 ) { LONG x; WORD base;
if ( BITSINWORD == 16 ) { ULONG x; WORD base;
base = ( *v == TNUMBER ) ? 100: 128;
vv = v+1; x = 0; while ( *vv >= 0 ) { x = x*base + *vv++; }
if ( ( vv != t ) || ( ( vv - v ) > 4 ) || ( x > (MAXPOSITIVE+1) ) )
*fill++ = *s++;
else { *t = TEMPTY; s++; break; }
}
else if ( BITSINWORD == 32 ) { LONG x; WORD base;
else if ( BITSINWORD == 32 ) { ULONG x; WORD base;
base = ( *v == TNUMBER ) ? 100: 128;
vv = v+1; x = 0; while ( *vv >= 0 ) { x = x*base + *vv++; }
if ( ( vv != t ) || ( ( vv - v ) > 6 ) || ( x > (MAXPOSITIVE+1) ) )
Expand Down Expand Up @@ -1039,14 +1039,14 @@ tcommon: v++; while ( *v >= 0 ) v++;
break;
case TNUMBER:
case TNUMBER1:
if ( BITSINWORD == 16 ) { LONG x; WORD base;
if ( BITSINWORD == 16 ) { ULONG x; WORD base;
base = ( *v == TNUMBER ) ? 100: 128;
vv = v+1; x = 0; while ( *vv >= 0 ) { x = x*base + *vv++; }
if ( ( vv != t ) || ( ( vv - v ) > 4 ) || ( x > MAXPOSITIVE ) )
*fill++ = *s++;
else { *t = TEMPTY; s++; break; }
}
else if ( BITSINWORD == 32 ) { LONG x; WORD base;
else if ( BITSINWORD == 32 ) { ULONG x; WORD base;
base = ( *v == TNUMBER ) ? 100: 128;
vv = v+1; x = 0; while ( *vv >= 0 ) { x = x*base + *vv++; }
if ( ( vv != t ) || ( ( vv - v ) > 6 ) || ( x > MAXPOSITIVE ) )
Expand Down
15 changes: 8 additions & 7 deletions sources/tools.c
View file
Expand Up @@ -2933,22 +2933,23 @@ void ExpandBuffer(void **buffer, LONG *oldsize, int type)
LONG iexp(LONG x, int p)
{
int sign;
LONG y;
ULONG y;
ULONG ux;
if ( x == 0 ) return(0);
if ( p == 0 ) return(1);
if ( x < 0 ) { sign = -1; x = -x; }
else sign = 1;
sign = x < 0 ? -1 : 1;
if ( sign < 0 && ( p & 1 ) == 0 ) sign = 1;
if ( x == 1 ) return(sign);
ux = LongAbs(x);
if ( ux == 1 ) return(sign);
if ( p < 0 ) return(0);
y = 1;
while ( p ) {
if ( ( p & 1 ) != 0 ) y *= x;
if ( ( p & 1 ) != 0 ) y *= ux;
p >>= 1;
x = x*x;
ux = ux*ux;
}
if ( sign < 0 ) y = -y;
return(y);
return ULongToLong(y);
}

/*
Expand Down

0 comments on commit 954024f

Please sign in to comment.