Skip to content

Commit

Permalink
Fix [0439e1e1a3]: Slow detection of illegal expr argument
Browse files Browse the repository at this point in the history
  • Loading branch information
jan.nijtmans committed Oct 8, 2024
2 parents 4fd16c7 + 880a3b1 commit e301c28
Show file tree
Hide file tree
Showing 16 changed files with 472 additions and 447 deletions.
49 changes: 33 additions & 16 deletions generic/tclExecute.c
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ static const char * GetSrcInfoForPc(const unsigned char *pc,
const unsigned char **pcBeg, Tcl_Size *cmdIdxPtr);
static Tcl_Obj ** GrowEvaluationStack(ExecEnv *eePtr, TCL_HASH_TYPE growth,
int move);
static void IllegalExprOperandType(Tcl_Interp *interp,
static void IllegalExprOperandType(Tcl_Interp *interp, const char *ord,
const unsigned char *pc, Tcl_Obj *opndPtr);
static void InitByteCodeExecution(Tcl_Interp *interp);
static inline int wordSkip(void *ptr);
Expand Down Expand Up @@ -4413,7 +4413,7 @@ TEBCresume(
TRACE(("\"%.20s\" => ILLEGAL TYPE %s \n", O2S(valuePtr),
(valuePtr->typePtr? valuePtr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "left ", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand All @@ -4422,7 +4422,7 @@ TEBCresume(
TRACE(("\"%.20s\" => ILLEGAL TYPE %s \n", O2S(value2Ptr),
(value2Ptr->typePtr? value2Ptr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, value2Ptr);
IllegalExprOperandType(interp, "right ", pc, value2Ptr);
CACHE_STACK_INFO();
goto gotError;
}
Expand Down Expand Up @@ -5982,7 +5982,7 @@ TEBCresume(
O2S(value2Ptr), (valuePtr->typePtr?
valuePtr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "left ", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand All @@ -5993,7 +5993,7 @@ TEBCresume(
O2S(value2Ptr), (value2Ptr->typePtr?
value2Ptr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, value2Ptr);
IllegalExprOperandType(interp, "right ", pc, value2Ptr);
CACHE_STACK_INFO();
goto gotError;
}
Expand Down Expand Up @@ -6203,7 +6203,7 @@ TEBCresume(
O2S(value2Ptr), O2S(valuePtr),
(valuePtr->typePtr? valuePtr->typePtr->name: "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "left ", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand All @@ -6224,7 +6224,7 @@ TEBCresume(
O2S(value2Ptr), O2S(valuePtr),
(value2Ptr->typePtr? value2Ptr->typePtr->name: "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, value2Ptr);
IllegalExprOperandType(interp, "right ", pc, value2Ptr);
CACHE_STACK_INFO();
goto gotError;
}
Expand Down Expand Up @@ -6366,7 +6366,7 @@ TEBCresume(
TRACE(("\"%.20s\" => ERROR: illegal type %s\n", O2S(valuePtr),
(valuePtr->typePtr? valuePtr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand All @@ -6388,7 +6388,7 @@ TEBCresume(
TRACE_APPEND(("ERROR: illegal type %s\n",
(valuePtr->typePtr? valuePtr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand Down Expand Up @@ -6420,7 +6420,7 @@ TEBCresume(
TRACE_APPEND(("ERROR: illegal type %s \n",
(valuePtr->typePtr? valuePtr->typePtr->name : "null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand Down Expand Up @@ -6473,7 +6473,7 @@ TEBCresume(
TRACE_APPEND(("ERROR: illegal type %s\n",
(valuePtr->typePtr? valuePtr->typePtr->name:"null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "", pc, valuePtr);
CACHE_STACK_INFO();
goto gotError;
}
Expand All @@ -6491,7 +6491,7 @@ TEBCresume(
TRACE_APPEND(("ERROR: illegal type %s\n",
(valuePtr->typePtr? valuePtr->typePtr->name:"null")));
DECACHE_STACK_INFO();
IllegalExprOperandType(interp, pc, valuePtr);
IllegalExprOperandType(interp, "", pc, valuePtr);
CACHE_STACK_INFO();
} else {
/*
Expand Down Expand Up @@ -9330,7 +9330,7 @@ ValidatePcAndStackTop(
TclNewLiteralStringObj(message, "\n executing ");
Tcl_IncrRefCount(message);
Tcl_AppendLimitedToObj(message, cmd, numChars, 100, NULL);
fprintf(stderr,"%s\n", TclGetString(message));
fprintf(stderr, "%s\n", TclGetString(message));
Tcl_DecrRefCount(message);
} else {
fprintf(stderr, "\n");
Expand Down Expand Up @@ -9362,7 +9362,8 @@ static void
IllegalExprOperandType(
Tcl_Interp *interp, /* Interpreter to which error information
* pertains. */
const unsigned char *pc, /* Points to the instruction being executed
const char *ord, /* "first ", "second " or "" */
const unsigned char *pc, /* Points to the instruction being executed
* when the illegal type was found. */
Tcl_Obj *opndPtr) /* Points to the operand holding the value
* with the illegal type. */
Expand All @@ -9380,6 +9381,22 @@ IllegalExprOperandType(

if (GetNumberFromObj(NULL, opndPtr, &ptr, &type) != TCL_OK) {
Tcl_Size numBytes;
if (TclHasInternalRep(opndPtr, &tclDictType)) {
Tcl_DictObjSize(NULL, opndPtr, &numBytes);
if (numBytes > 0) {
listRep:
Tcl_SetObjResult(interp, Tcl_ObjPrintf(
"cannot use a list as %soperand of \"%s\"", ord, op));
Tcl_SetErrorCode(interp, "ARITH", "DOMAIN", "list", (char *)NULL);
return;
}
}
Tcl_Size objcPtr;
Tcl_Obj **objvPtr;
if ((TclMaxListLength(TclGetString(opndPtr), TCL_INDEX_NONE, NULL) > 1)
&& (Tcl_ListObjGetElements(NULL, opndPtr, &objcPtr, &objvPtr) == TCL_OK)) {
goto listRep;
}
const char *bytes = TclGetStringFromObj(opndPtr, &numBytes);

if (numBytes == 0) {
Expand All @@ -9399,8 +9416,8 @@ IllegalExprOperandType(
}

Tcl_SetObjResult(interp, Tcl_ObjPrintf(
"can't use %s \"%s\" as operand of \"%s\"", description,
TclGetString(opndPtr), op));
"cannot use %s \"%s\" as %soperand of \"%s\"", description,
TclGetString(opndPtr), ord, op));
Tcl_SetErrorCode(interp, "ARITH", "DOMAIN", description, (char *)NULL);
}

Expand Down
20 changes: 14 additions & 6 deletions generic/tclStrToD.c
Original file line number Diff line number Diff line change
Expand Up @@ -1586,13 +1586,21 @@ TclParseNumber(

if (status != TCL_OK) {
if (interp != NULL) {
Tcl_Obj *msg = Tcl_ObjPrintf("expected %s but got \"",
Tcl_Obj *msg = Tcl_ObjPrintf("expected %s but got ",
expected);

Tcl_AppendLimitedToObj(msg, bytes, numBytes, 50, "");
Tcl_AppendToObj(msg, "\"", -1);
if (state == BAD_OCTAL) {
Tcl_AppendToObj(msg, " (looks like invalid octal number)", -1);
Tcl_Size argc;
const char **argv;
if ((TclMaxListLength(bytes, TCL_INDEX_NONE, NULL) > 1)
&& Tcl_SplitList(NULL, bytes, &argc, &argv) == TCL_OK) {
Tcl_Free(argv);
Tcl_AppendToObj(msg, "a list", -1);
} else {
Tcl_AppendToObj(msg, "\"", -1);
Tcl_AppendLimitedToObj(msg, bytes, numBytes, 50, "");
Tcl_AppendToObj(msg, "\"", -1);
if (state == BAD_OCTAL) {
Tcl_AppendToObj(msg, " (looks like invalid octal number)", -1);
}
}
Tcl_SetObjResult(interp, msg);
Tcl_SetErrorCode(interp, "TCL", "VALUE", "NUMBER", (char *)NULL);
Expand Down
2 changes: 1 addition & 1 deletion tests/assemble.test
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ test assemble-7.43 {uplus} {
}
}
-returnCodes error
-result {can't use non-numeric floating-point value "NaN" as operand of "+"}
-result {cannot use non-numeric floating-point value "NaN" as operand of "+"}
}
test assemble-7.43.1 {tryCvtToNumeric} {
-body {
Expand Down
22 changes: 11 additions & 11 deletions tests/binary.test
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ test binary-8.9 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-8.10 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format c $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-8.11 {Tcl_BinaryObjCmd: format} {
set a {0x50 0x51}
binary format c1 $a
Expand Down Expand Up @@ -348,7 +348,7 @@ test binary-9.10 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-9.11 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format s $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-9.12 {Tcl_BinaryObjCmd: format} {
set a {0x50 0x51}
binary format s1 $a
Expand Down Expand Up @@ -387,7 +387,7 @@ test binary-10.10 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-10.11 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format S $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-10.12 {Tcl_BinaryObjCmd: format} {
set a {0x50 0x51}
binary format S1 $a
Expand Down Expand Up @@ -429,7 +429,7 @@ test binary-11.11 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-11.12 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format i $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-11.13 {Tcl_BinaryObjCmd: format} {
set a {0x50 0x51}
binary format i1 $a
Expand Down Expand Up @@ -471,7 +471,7 @@ test binary-12.11 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-12.12 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format I $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-12.13 {Tcl_BinaryObjCmd: format} {
set a {0x50 0x51}
binary format I1 $a
Expand Down Expand Up @@ -528,7 +528,7 @@ test binary-13.16 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-13.17 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {1.6 3.4}
binary format f $a
} -result "expected floating-point number but got \"1.6 3.4\""
} -result "expected floating-point number but got a list"
test binary-13.18 {Tcl_BinaryObjCmd: format} bigEndian {
set a {1.6 3.4}
binary format f1 $a
Expand Down Expand Up @@ -589,7 +589,7 @@ test binary-14.14 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-14.15 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {1.6 3.4}
binary format d $a
} -result "expected floating-point number but got \"1.6 3.4\""
} -result "expected floating-point number but got a list"
test binary-14.16 {Tcl_BinaryObjCmd: format} bigEndian {
set a {1.6 3.4}
binary format d1 $a
Expand Down Expand Up @@ -1811,7 +1811,7 @@ test binary-48.16 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-48.17 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format t $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-48.18 {Tcl_BinaryObjCmd: format} bigEndian {
set a {0x50 0x51}
binary format t1 $a
Expand Down Expand Up @@ -1858,7 +1858,7 @@ test binary-49.11 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-49.12 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {0x50 0x51}
binary format n $a
} -result "expected integer but got \"0x50 0x51\""
} -result "expected integer but got a list"
test binary-49.13 {Tcl_BinaryObjCmd: format} littleEndian {
set a {0x50 0x51}
binary format n1 $a
Expand Down Expand Up @@ -1941,7 +1941,7 @@ test binary-51.14 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-51.15 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {1.6 3.4}
binary format q $a
} -result "expected floating-point number but got \"1.6 3.4\""
} -result "expected floating-point number but got a list"
test binary-51.16 {Tcl_BinaryObjCmd: format} {} {
set a {1.6 3.4}
binary format Q1 $a
Expand Down Expand Up @@ -2003,7 +2003,7 @@ test binary-53.16 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
test binary-53.17 {Tcl_BinaryObjCmd: format} -returnCodes error -body {
set a {1.6 3.4}
binary format r $a
} -result "expected floating-point number but got \"1.6 3.4\""
} -result "expected floating-point number but got a list"
test binary-53.18 {Tcl_BinaryObjCmd: format} {} {
set a {1.6 3.4}
binary format R1 $a
Expand Down
24 changes: 12 additions & 12 deletions tests/compExpr-old.test
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,10 @@ test compExpr-old-6.8 {CompileBitXorExpr: error compiling bitxor arm} -body {
} -returnCodes error -match glob -result *
test compExpr-old-6.9 {CompileBitXorExpr: runtime error in bitxor arm} {
list [catch {expr {24.0^3}} msg] $msg
} {1 {can't use floating-point value "24.0" as operand of "^"}}
} {1 {cannot use floating-point value "24.0" as left operand of "^"}}
test compExpr-old-6.10 {CompileBitXorExpr: runtime error in bitxor arm} {
list [catch {expr {"a"^"b"}} msg] $msg
} {1 {can't use non-numeric string "a" as operand of "^"}}
} {1 {cannot use non-numeric string "a" as left operand of "^"}}

test compExpr-old-7.1 {CompileBitAndExpr: just equality expr} {expr 3==2} 0
test compExpr-old-7.2 {CompileBitAndExpr: just equality expr} {expr 2.0==2} 1
Expand All @@ -304,10 +304,10 @@ test compExpr-old-7.11 {CompileBitAndExpr: error compiling bitand arm} -body {
} -returnCodes error -match glob -result *
test compExpr-old-7.12 {CompileBitAndExpr: runtime error in bitand arm} {
list [catch {expr {24.0&3}} msg] $msg
} {1 {can't use floating-point value "24.0" as operand of "&"}}
} {1 {cannot use floating-point value "24.0" as left operand of "&"}}
test compExpr-old-7.13 {CompileBitAndExpr: runtime error in bitand arm} {
list [catch {expr {"a"&"b"}} msg] $msg
} {1 {can't use non-numeric string "a" as operand of "&"}}
} {1 {cannot use non-numeric string "a" as left operand of "&"}}

test compExpr-old-8.1 {CompileEqualityExpr: just relational expr} {expr 3>=2} 1
test compExpr-old-8.2 {CompileEqualityExpr: just relational expr} {expr 2<=2.1} 1
Expand Down Expand Up @@ -365,10 +365,10 @@ test compExpr-old-10.9 {CompileShiftExpr: error compiling shift arm} -body {
} -returnCodes error -match glob -result *
test compExpr-old-10.10 {CompileShiftExpr: runtime error} {
list [catch {expr {24.0>>43}} msg] $msg
} {1 {can't use floating-point value "24.0" as operand of ">>"}}
} {1 {cannot use floating-point value "24.0" as left operand of ">>"}}
test compExpr-old-10.11 {CompileShiftExpr: runtime error} {
list [catch {expr {"a"<<"b"}} msg] $msg
} {1 {can't use non-numeric string "a" as operand of "<<"}}
} {1 {cannot use non-numeric string "a" as left operand of "<<"}}

test compExpr-old-11.1 {CompileAddExpr: just multiply expr} {expr 4*-2} -8
test compExpr-old-11.2 {CompileAddExpr: just multiply expr} {expr 0xff%2} 1
Expand All @@ -387,10 +387,10 @@ test compExpr-old-11.9 {CompileAddExpr: error compiling add arm} -body {
} -returnCodes error -match glob -result *
test compExpr-old-11.10 {CompileAddExpr: runtime error} {
list [catch {expr {24.0+"xx"}} msg] $msg
} {1 {can't use non-numeric string "xx" as operand of "+"}}
} {1 {cannot use non-numeric string "xx" as right operand of "+"}}
test compExpr-old-11.11 {CompileAddExpr: runtime error} {
list [catch {expr {"a"-"b"}} msg] $msg
} {1 {can't use non-numeric string "a" as operand of "-"}}
} {1 {cannot use non-numeric string "a" as left operand of "-"}}
test compExpr-old-11.12 {CompileAddExpr: runtime error} {
list [catch {expr {3/0}} msg] $msg
} {1 {divide by zero}}
Expand Down Expand Up @@ -418,10 +418,10 @@ test compExpr-old-12.9 {CompileMultiplyExpr: error compiling multiply arm} -body
} -returnCodes error -match glob -result *
test compExpr-old-12.10 {CompileMultiplyExpr: runtime error} {
list [catch {expr {24.0*"xx"}} msg] $msg
} {1 {can't use non-numeric string "xx" as operand of "*"}}
} {1 {cannot use non-numeric string "xx" as right operand of "*"}}
test compExpr-old-12.11 {CompileMultiplyExpr: runtime error} {
list [catch {expr {"a"/"b"}} msg] $msg
} {1 {can't use non-numeric string "a" as operand of "/"}}
} {1 {cannot use non-numeric string "a" as left operand of "/"}}

test compExpr-old-13.1 {CompileUnaryExpr: unary exprs} {expr -0xff} -255
test compExpr-old-13.2 {CompileUnaryExpr: unary exprs} {expr +0o00123} 83
Expand All @@ -439,10 +439,10 @@ test compExpr-old-13.9 {CompileUnaryExpr: error compiling unary expr} -body {
} -returnCodes error -match glob -result *
test compExpr-old-13.10 {CompileUnaryExpr: runtime error} {
list [catch {expr {~"xx"}} msg] $msg
} {1 {can't use non-numeric string "xx" as operand of "~"}}
} {1 {cannot use non-numeric string "xx" as operand of "~"}}
test compExpr-old-13.11 {CompileUnaryExpr: runtime error} {
list [catch {expr ~4.0} msg] $msg
} {1 {can't use floating-point value "4.0" as operand of "~"}}
} {1 {cannot use floating-point value "4.0" as operand of "~"}}
test compExpr-old-13.12 {CompileUnaryExpr: just primary expr} {expr 0x123} 291
test compExpr-old-13.13 {CompileUnaryExpr: just primary expr} {
set a 27
Expand Down
2 changes: 1 addition & 1 deletion tests/dict.test
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ test dict-17.12 {dict filter command: script} -returnCodes error -body {
}
} -cleanup {
unset k v
} -result {expected boolean value but got "a b"}
} -result {expected boolean value but got a list}
test dict-17.13 {dict filter command: script} -body {
list [catch {dict filter {a b} script {k v} {error x}} msg] $msg \
$::errorInfo
Expand Down
Loading

0 comments on commit e301c28

Please sign in to comment.