From fdf1df3b29531c5031a33f2e3c1f4668937c1c5f Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Tue, 4 Sep 2018 14:37:01 +0000 Subject: Fix [540bed4bde]: binary format can result in "integer value too large to represent". Implemented a new utility function TclGetWideBitsFromObj(), which handles the overflowing of integers using bignums. This function can be used in a _LOT_ of other places, preventing code duplication. Done that as well. Those changes have no effect on other commands, only two new "binary format" test-cases for this specific situation: binary-44.5 and binary-44.6 --- generic/tclBasic.c | 55 ++++++-------------------------------- generic/tclBinary.c | 35 ++++++++++++------------ generic/tclCmdAH.c | 8 +++--- generic/tclInt.h | 2 ++ generic/tclObj.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++--- generic/tclStringObj.c | 26 +++--------------- tests/binary.test | 41 +++++++++++++++++----------- 7 files changed, 129 insertions(+), 110 deletions(-) diff --git a/generic/tclBasic.c b/generic/tclBasic.c index 93776dc..0190c13 100644 --- a/generic/tclBasic.c +++ b/generic/tclBasic.c @@ -7680,27 +7680,14 @@ ExprIntFunc( int objc, /* Actual parameter count. */ Tcl_Obj *const *objv) /* Actual parameter vector. */ { - long iResult; + Tcl_WideInt wResult; Tcl_Obj *objPtr; if (ExprEntierFunc(NULL, interp, objc, objv) != TCL_OK) { return TCL_ERROR; } objPtr = Tcl_GetObjResult(interp); - if (TclGetLongFromObj(NULL, objPtr, &iResult) != TCL_OK) { - /* - * Truncate the bignum; keep only bits in long range. - */ - - mp_int big; - - Tcl_GetBignumFromObj(NULL, objPtr, &big); - mp_mod_2d(&big, (int) CHAR_BIT * sizeof(long), &big); - objPtr = Tcl_NewBignumObj(&big); - Tcl_IncrRefCount(objPtr); - TclGetLongFromObj(NULL, objPtr, &iResult); - Tcl_DecrRefCount(objPtr); - } - Tcl_SetObjResult(interp, Tcl_NewLongObj(iResult)); + TclGetWideBitsFromObj(NULL, objPtr, &wResult); + Tcl_SetObjResult(interp, Tcl_NewLongObj((long)wResult)); return TCL_OK; } @@ -7719,20 +7706,7 @@ ExprWideFunc( return TCL_ERROR; } objPtr = Tcl_GetObjResult(interp); - if (TclGetWideIntFromObj(NULL, objPtr, &wResult) != TCL_OK) { - /* - * Truncate the bignum; keep only bits in wide int range. - */ - - mp_int big; - - Tcl_GetBignumFromObj(NULL, objPtr, &big); - mp_mod_2d(&big, (int) CHAR_BIT * sizeof(Tcl_WideInt), &big); - objPtr = Tcl_NewBignumObj(&big); - Tcl_IncrRefCount(objPtr); - TclGetWideIntFromObj(NULL, objPtr, &wResult); - Tcl_DecrRefCount(objPtr); - } + TclGetWideBitsFromObj(NULL, objPtr, &wResult); Tcl_SetObjResult(interp, Tcl_NewWideIntObj(wResult)); return TCL_OK; } @@ -7979,7 +7953,7 @@ ExprSrandFunc( Tcl_Obj *const *objv) /* Parameter vector. */ { Interp *iPtr = (Interp *) interp; - long i = 0; /* Initialized to avoid compiler warning. */ + Tcl_WideInt w = 0; /* Initialized to avoid compiler warning. */ /* * Convert argument and use it to reset the seed. @@ -7990,20 +7964,8 @@ ExprSrandFunc( return TCL_ERROR; } - if (TclGetLongFromObj(NULL, objv[1], &i) != TCL_OK) { - Tcl_Obj *objPtr; - mp_int big; - - if (Tcl_GetBignumFromObj(interp, objv[1], &big) != TCL_OK) { - /* TODO: more ::errorInfo here? or in caller? */ - return TCL_ERROR; - } - - mp_mod_2d(&big, (int) CHAR_BIT * sizeof(long), &big); - objPtr = Tcl_NewBignumObj(&big); - Tcl_IncrRefCount(objPtr); - TclGetLongFromObj(NULL, objPtr, &i); - Tcl_DecrRefCount(objPtr); + if (TclGetWideBitsFromObj(NULL, objv[1], &w) != TCL_OK) { + return TCL_ERROR; } /* @@ -8012,8 +7974,7 @@ ExprSrandFunc( */ iPtr->flags |= RAND_SEED_INITIALIZED; - iPtr->randSeed = i; - iPtr->randSeed &= (unsigned long) 0x7fffffff; + iPtr->randSeed = (unsigned long) (w & 0x7fffffff); if ((iPtr->randSeed == 0) || (iPtr->randSeed == 0x7fffffff)) { iPtr->randSeed ^= 123459876; } diff --git a/generic/tclBinary.c b/generic/tclBinary.c index cb5a5cb..24f228e 100644 --- a/generic/tclBinary.c +++ b/generic/tclBinary.c @@ -1963,7 +1963,6 @@ FormatNumber( Tcl_Obj *src, /* Number to format. */ unsigned char **cursorPtr) /* Pointer to index into destination buffer. */ { - long value; double dvalue; Tcl_WideInt wvalue; float fvalue; @@ -2025,7 +2024,7 @@ FormatNumber( case 'w': case 'W': case 'm': - if (Tcl_GetWideIntFromObj(interp, src, &wvalue) != TCL_OK) { + if (TclGetWideBitsFromObj(interp, src, &wvalue) != TCL_OK) { return TCL_ERROR; } if (NeedReversing(type)) { @@ -2055,19 +2054,19 @@ FormatNumber( case 'i': case 'I': case 'n': - if (TclGetLongFromObj(interp, src, &value) != TCL_OK) { + if (TclGetWideBitsFromObj(interp, src, &wvalue) != TCL_OK) { return TCL_ERROR; } if (NeedReversing(type)) { - *(*cursorPtr)++ = UCHAR(value); - *(*cursorPtr)++ = UCHAR(value >> 8); - *(*cursorPtr)++ = UCHAR(value >> 16); - *(*cursorPtr)++ = UCHAR(value >> 24); + *(*cursorPtr)++ = UCHAR(wvalue); + *(*cursorPtr)++ = UCHAR(wvalue >> 8); + *(*cursorPtr)++ = UCHAR(wvalue >> 16); + *(*cursorPtr)++ = UCHAR(wvalue >> 24); } else { - *(*cursorPtr)++ = UCHAR(value >> 24); - *(*cursorPtr)++ = UCHAR(value >> 16); - *(*cursorPtr)++ = UCHAR(value >> 8); - *(*cursorPtr)++ = UCHAR(value); + *(*cursorPtr)++ = UCHAR(wvalue >> 24); + *(*cursorPtr)++ = UCHAR(wvalue >> 16); + *(*cursorPtr)++ = UCHAR(wvalue >> 8); + *(*cursorPtr)++ = UCHAR(wvalue); } return TCL_OK; @@ -2077,15 +2076,15 @@ FormatNumber( case 's': case 'S': case 't': - if (TclGetLongFromObj(interp, src, &value) != TCL_OK) { + if (TclGetWideBitsFromObj(interp, src, &wvalue) != TCL_OK) { return TCL_ERROR; } if (NeedReversing(type)) { - *(*cursorPtr)++ = UCHAR(value); - *(*cursorPtr)++ = UCHAR(value >> 8); + *(*cursorPtr)++ = UCHAR(wvalue); + *(*cursorPtr)++ = UCHAR(wvalue >> 8); } else { - *(*cursorPtr)++ = UCHAR(value >> 8); - *(*cursorPtr)++ = UCHAR(value); + *(*cursorPtr)++ = UCHAR(wvalue >> 8); + *(*cursorPtr)++ = UCHAR(wvalue); } return TCL_OK; @@ -2093,10 +2092,10 @@ FormatNumber( * 8-bit integer values. */ case 'c': - if (TclGetLongFromObj(interp, src, &value) != TCL_OK) { + if (TclGetWideBitsFromObj(interp, src, &wvalue) != TCL_OK) { return TCL_ERROR; } - *(*cursorPtr)++ = UCHAR(value); + *(*cursorPtr)++ = UCHAR(wvalue); return TCL_OK; default: diff --git a/generic/tclCmdAH.c b/generic/tclCmdAH.c index 810fac6..94cb8aa 100644 --- a/generic/tclCmdAH.c +++ b/generic/tclCmdAH.c @@ -1398,9 +1398,9 @@ FileAttrAccessTimeCmd( * platforms. [Bug 698146] */ - long newTime; + Tcl_WideInt newTime; - if (TclGetLongFromObj(interp, objv[2], &newTime) != TCL_OK) { + if (TclGetWideIntFromObj(interp, objv[2], &newTime) != TCL_OK) { return TCL_ERROR; } @@ -1479,9 +1479,9 @@ FileAttrModifyTimeCmd( * platforms. [Bug 698146] */ - long newTime; + Tcl_WideInt newTime; - if (TclGetLongFromObj(interp, objv[2], &newTime) != TCL_OK) { + if (TclGetWideIntFromObj(interp, objv[2], &newTime) != TCL_OK) { return TCL_ERROR; } diff --git a/generic/tclInt.h b/generic/tclInt.h index 5379396..e8fecf0 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -3041,6 +3041,8 @@ MODULE_SCOPE char * TclGetStringStorage(Tcl_Obj *objPtr, MODULE_SCOPE int TclGetLoadedPackagesEx(Tcl_Interp *interp, const char *targetName, const char *packageName); +MODULE_SCOPE int TclGetWideBitsFromObj(Tcl_Interp *, Tcl_Obj *, + Tcl_WideInt *); MODULE_SCOPE int TclGlob(Tcl_Interp *interp, char *pattern, Tcl_Obj *unquotedPrefix, int globFlags, Tcl_GlobTypeData *types); diff --git a/generic/tclObj.c b/generic/tclObj.c index 16ef7c3..490e80e 100644 --- a/generic/tclObj.c +++ b/generic/tclObj.c @@ -2833,10 +2833,9 @@ Tcl_GetLongFromObj( mp_int big; UNPACK_BIGNUM(objPtr, big); - if ((size_t) big.used <= (CHAR_BIT * sizeof(long) + DIGIT_BIT - 1) + if ((size_t) big.used <= (CHAR_BIT * sizeof(unsigned long) + DIGIT_BIT - 1) / DIGIT_BIT) { - unsigned long value = 0, numBytes = sizeof(long); - long scratch; + unsigned long scratch, value = 0, numBytes = sizeof(unsigned long); unsigned char *bytes = (unsigned char *) &scratch; if (mp_to_unsigned_bin_n(&big, bytes, &numBytes) == MP_OKAY) { @@ -3107,6 +3106,73 @@ Tcl_GetWideIntFromObj( /* *---------------------------------------------------------------------- * + * TclGetWideBitsFromObj -- + * + * Attempt to return a wide integer from the Tcl object "objPtr". If the + * object is not already a int, double or bignum, an attempt will be made + * to convert it to one of these. Out-of-range values don't result in an + * error, but only the least significant 64 bits will be returned. + * + * Results: + * The return value is a standard Tcl object result. If an error occurs + * during conversion, an error message is left in the interpreter's + * result unless "interp" is NULL. + * + * Side effects: + * If the object is not already an int, double or bignum object, the + * conversion will free any old internal representation. + * + *---------------------------------------------------------------------- + */ + +int +TclGetWideBitsFromObj( + Tcl_Interp *interp, /* Used for error reporting if not NULL. */ + Tcl_Obj *objPtr, /* Object from which to get a wide int. */ + Tcl_WideInt *wideIntPtr) /* Place to store resulting wide integer. */ +{ + do { + if (objPtr->typePtr == &tclIntType) { + *wideIntPtr = objPtr->internalRep.wideValue; + return TCL_OK; + } + if (objPtr->typePtr == &tclDoubleType) { + if (interp != NULL) { + Tcl_SetObjResult(interp, Tcl_ObjPrintf( + "expected integer but got \"%s\"", + TclGetString(objPtr))); + Tcl_SetErrorCode(interp, "TCL", "VALUE", "INTEGER", NULL); + } + return TCL_ERROR; + } + if (objPtr->typePtr == &tclBignumType) { + mp_int big; + + Tcl_WideUInt value = 0, scratch; + unsigned long numBytes = sizeof(Tcl_WideInt); + unsigned char *bytes = (unsigned char *) &scratch; + + Tcl_GetBignumFromObj(NULL, objPtr, &big); + mp_mod_2d(&big, (int) (CHAR_BIT * sizeof(Tcl_WideInt)), &big); + mp_to_unsigned_bin_n(&big, bytes, &numBytes); + while (numBytes-- > 0) { + value = (value << CHAR_BIT) | *bytes++; + } + if (big.sign) { + value = -value; + } + *wideIntPtr = (Tcl_WideInt) value; + mp_clear(&big); + return TCL_OK; + } + } while (TclParseNumber(interp, objPtr, "integer", NULL, -1, NULL, + TCL_PARSE_INTEGER_ONLY)==TCL_OK); + return TCL_ERROR; +} + +/* + *---------------------------------------------------------------------- + * * FreeBignum -- * * This function frees the internal rep of a bignum. diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index f98180f..f238268 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2097,33 +2097,15 @@ Tcl_AppendFormatToObj( } #ifndef TCL_WIDE_INT_IS_LONG } else if (useWide) { - if (TclGetWideIntFromObj(NULL, segment, &w) != TCL_OK) { - Tcl_Obj *objPtr; - - if (Tcl_GetBignumFromObj(interp,segment,&big) != TCL_OK) { - goto error; - } - mp_mod_2d(&big, (int) CHAR_BIT*sizeof(Tcl_WideInt), &big); - objPtr = Tcl_NewBignumObj(&big); - Tcl_IncrRefCount(objPtr); - TclGetWideIntFromObj(NULL, objPtr, &w); - Tcl_DecrRefCount(objPtr); + if (TclGetWideBitsFromObj(interp, segment, &w) != TCL_OK) { + goto error; } isNegative = (w < (Tcl_WideInt) 0); if (w == (Tcl_WideInt) 0) gotHash = 0; #endif } else if (TclGetLongFromObj(NULL, segment, &l) != TCL_OK) { - if (TclGetWideIntFromObj(NULL, segment, &w) != TCL_OK) { - Tcl_Obj *objPtr; - - if (Tcl_GetBignumFromObj(interp,segment,&big) != TCL_OK) { - goto error; - } - mp_mod_2d(&big, (int) CHAR_BIT * sizeof(long), &big); - objPtr = Tcl_NewBignumObj(&big); - Tcl_IncrRefCount(objPtr); - TclGetLongFromObj(NULL, objPtr, &l); - Tcl_DecrRefCount(objPtr); + if (TclGetWideBitsFromObj(interp, segment, &w) != TCL_OK) { + goto error; } else { l = Tcl_WideAsLong(w); } diff --git a/tests/binary.test b/tests/binary.test index 1ee815b..54e8e94 100644 --- a/tests/binary.test +++ b/tests/binary.test @@ -1647,22 +1647,6 @@ test binary-43.2 {Tcl_BinaryObjCmd: format wide int} {} { binary format W 7810179016327718216 } lcTolleH -test binary-44.1 {Tcl_BinaryObjCmd: scan wide int} {} { - binary scan HelloTcl W x - set x -} 5216694956358656876 -test binary-44.2 {Tcl_BinaryObjCmd: scan wide int} {} { - binary scan lcTolleH w x - set x -} 5216694956358656876 -test binary-44.3 {Tcl_BinaryObjCmd: scan wide int with bit 31 set} {} { - binary scan [binary format w [expr {wide(3) << 31}]] w x - set x -} 6442450944 -test binary-44.4 {Tcl_BinaryObjCmd: scan wide int with bit 31 set} {} { - binary scan [binary format W [expr {wide(3) << 31}]] W x - set x -} 6442450944 test binary-43.5 {Tcl_BinaryObjCmd: scan wide int} {} { unset -nocomplain arg1 list [binary scan \x80[string repeat \x00 7] W arg1] $arg1 @@ -1684,6 +1668,31 @@ test binary-43.9 {Tcl_BinaryObjCmd: scan unsigned wide int} {} { list [binary scan [string repeat \x00 7]\x80[string repeat \x00 7]\x80 wuw arg1 arg2] $arg1 $arg2 } {2 9223372036854775808 -9223372036854775808} +test binary-44.1 {Tcl_BinaryObjCmd: scan wide int} {} { + binary scan HelloTcl W x + set x +} 5216694956358656876 +test binary-44.2 {Tcl_BinaryObjCmd: scan wide int} {} { + binary scan lcTolleH w x + set x +} 5216694956358656876 +test binary-44.3 {Tcl_BinaryObjCmd: scan wide int with bit 31 set} {} { + binary scan [binary format w [expr {wide(3) << 31}]] w x + set x +} 6442450944 +test binary-44.4 {Tcl_BinaryObjCmd: scan wide int with bit 31 set} {} { + binary scan [binary format W [expr {wide(3) << 31}]] W x + set x +} 6442450944 +test binary-44.5 {Tcl_BinaryObjCmd: scan wide int with bit 31 and 64 set} {} { + binary scan [binary format w [expr {(wide(3) << 31) + (wide(3) << 64)}]] w x + set x +} 6442450944 +test binary-44.6 {Tcl_BinaryObjCmd: scan wide int with bit 31 and 64 set} {} { + binary scan [binary format W [expr {(wide(3) << 31) + (wide(3) << 64)}]] W x + set x +} 6442450944 + test binary-45.1 {Tcl_BinaryObjCmd: combined wide int handling} { binary scan [binary format sws 16450 -1 19521] c* x set x -- cgit v0.12