From e1279858394a6079be6816cbedaa3f10e74057cc Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 16 Jun 2023 19:43:53 +0200 Subject: shell/math: fix ?: to not evaluate not-taken branches This fixes ash-arith-arith-ternary1/2.tests function old new delta evaluate_string 1271 1432 +161 arith_apply 968 1000 +32 arith 22 36 +14 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 3/0 up/down: 207/0) Total: 207 bytes Signed-off-by: Denys Vlasenko --- .../ash_test/ash-arith/arith-ternary-assign.right | 1 + .../ash_test/ash-arith/arith-ternary-assign.tests | 3 + shell/ash_test/ash-arith/arith-ternary-comma.right | 1 + shell/ash_test/ash-arith/arith-ternary-comma.tests | 3 + .../ash_test/ash-arith/arith-ternary-preincr.right | 1 + .../ash_test/ash-arith/arith-ternary-preincr.tests | 3 + shell/ash_test/ash-arith/arith-ternary3.right | 1 + shell/ash_test/ash-arith/arith-ternary3.tests | 4 + .../ash_test/ash-arith/arith-ternary_nested3.right | 2 + .../ash_test/ash-arith/arith-ternary_nested3.tests | 6 ++ .../hush-arith/arith-ternary-assign.right | 1 + .../hush-arith/arith-ternary-assign.tests | 3 + .../hush_test/hush-arith/arith-ternary-comma.right | 1 + .../hush_test/hush-arith/arith-ternary-comma.tests | 3 + .../hush-arith/arith-ternary-preincr.right | 1 + .../hush-arith/arith-ternary-preincr.tests | 3 + shell/hush_test/hush-arith/arith-ternary1.right | 2 + shell/hush_test/hush-arith/arith-ternary1.tests | 5 ++ shell/hush_test/hush-arith/arith-ternary2.right | 3 + shell/hush_test/hush-arith/arith-ternary2.tests | 7 ++ shell/hush_test/hush-arith/arith-ternary3.right | 1 + shell/hush_test/hush-arith/arith-ternary3.tests | 4 + .../hush-arith/arith-ternary_nested3.right | 2 + .../hush-arith/arith-ternary_nested3.tests | 6 ++ shell/math.c | 85 ++++++++++++++++------ shell/math.h | 3 +- 26 files changed, 131 insertions(+), 24 deletions(-) create mode 100644 shell/ash_test/ash-arith/arith-ternary-assign.right create mode 100755 shell/ash_test/ash-arith/arith-ternary-assign.tests create mode 100644 shell/ash_test/ash-arith/arith-ternary-comma.right create mode 100755 shell/ash_test/ash-arith/arith-ternary-comma.tests create mode 100644 shell/ash_test/ash-arith/arith-ternary-preincr.right create mode 100755 shell/ash_test/ash-arith/arith-ternary-preincr.tests create mode 100644 shell/ash_test/ash-arith/arith-ternary3.right create mode 100755 shell/ash_test/ash-arith/arith-ternary3.tests create mode 100644 shell/ash_test/ash-arith/arith-ternary_nested3.right create mode 100755 shell/ash_test/ash-arith/arith-ternary_nested3.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary-assign.right create mode 100755 shell/hush_test/hush-arith/arith-ternary-assign.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary-comma.right create mode 100755 shell/hush_test/hush-arith/arith-ternary-comma.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary-preincr.right create mode 100755 shell/hush_test/hush-arith/arith-ternary-preincr.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary1.right create mode 100755 shell/hush_test/hush-arith/arith-ternary1.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary2.right create mode 100755 shell/hush_test/hush-arith/arith-ternary2.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary3.right create mode 100755 shell/hush_test/hush-arith/arith-ternary3.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary_nested3.right create mode 100755 shell/hush_test/hush-arith/arith-ternary_nested3.tests (limited to 'shell') diff --git a/shell/ash_test/ash-arith/arith-ternary-assign.right b/shell/ash_test/ash-arith/arith-ternary-assign.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-assign.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/ash_test/ash-arith/arith-ternary-assign.tests b/shell/ash_test/ash-arith/arith-ternary-assign.tests new file mode 100755 index 000000000..fa18fe7b9 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-assign.tests @@ -0,0 +1,3 @@ +exec 2>&1 +a='@' +echo 42:$((a=1?42:3,a)) diff --git a/shell/ash_test/ash-arith/arith-ternary-comma.right b/shell/ash_test/ash-arith/arith-ternary-comma.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-comma.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/ash_test/ash-arith/arith-ternary-comma.tests b/shell/ash_test/ash-arith/arith-ternary-comma.tests new file mode 100755 index 000000000..5e05b58c4 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-comma.tests @@ -0,0 +1,3 @@ +exec 2>&1 +x='@' +echo 42:$((1?4:x,20*2+2)) diff --git a/shell/ash_test/ash-arith/arith-ternary-preincr.right b/shell/ash_test/ash-arith/arith-ternary-preincr.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-preincr.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/ash_test/ash-arith/arith-ternary-preincr.tests b/shell/ash_test/ash-arith/arith-ternary-preincr.tests new file mode 100755 index 000000000..3985c7079 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary-preincr.tests @@ -0,0 +1,3 @@ +exec 2>&1 +x='@' +echo 42:$((1?42:++x)) diff --git a/shell/ash_test/ash-arith/arith-ternary3.right b/shell/ash_test/ash-arith/arith-ternary3.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary3.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/ash_test/ash-arith/arith-ternary3.tests b/shell/ash_test/ash-arith/arith-ternary3.tests new file mode 100755 index 000000000..0bf9f3002 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary3.tests @@ -0,0 +1,4 @@ +exec 2>&1 +# "EXPR ?..." should check _evaluated_ EXPR, +# not its last value +echo 42:$((1 < 1 ? -1 : 1 > 1 ? 1 : 42)) diff --git a/shell/ash_test/ash-arith/arith-ternary_nested3.right b/shell/ash_test/ash-arith/arith-ternary_nested3.right new file mode 100644 index 000000000..1a34fde65 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested3.right @@ -0,0 +1,2 @@ +42:42 +a=2:2 diff --git a/shell/ash_test/ash-arith/arith-ternary_nested3.tests b/shell/ash_test/ash-arith/arith-ternary_nested3.tests new file mode 100755 index 000000000..b69dcc6e9 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested3.tests @@ -0,0 +1,6 @@ +exec 2>&1 +x='@' +a=2 +# After processing nested ?:, outermost ?: should still rememeber to NOT evaluate a*=2 +echo 42:$((1?0?41:42:(a*=2))) +echo "a=2:$a" diff --git a/shell/hush_test/hush-arith/arith-ternary-assign.right b/shell/hush_test/hush-arith/arith-ternary-assign.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-assign.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/hush_test/hush-arith/arith-ternary-assign.tests b/shell/hush_test/hush-arith/arith-ternary-assign.tests new file mode 100755 index 000000000..fa18fe7b9 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-assign.tests @@ -0,0 +1,3 @@ +exec 2>&1 +a='@' +echo 42:$((a=1?42:3,a)) diff --git a/shell/hush_test/hush-arith/arith-ternary-comma.right b/shell/hush_test/hush-arith/arith-ternary-comma.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-comma.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/hush_test/hush-arith/arith-ternary-comma.tests b/shell/hush_test/hush-arith/arith-ternary-comma.tests new file mode 100755 index 000000000..5e05b58c4 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-comma.tests @@ -0,0 +1,3 @@ +exec 2>&1 +x='@' +echo 42:$((1?4:x,20*2+2)) diff --git a/shell/hush_test/hush-arith/arith-ternary-preincr.right b/shell/hush_test/hush-arith/arith-ternary-preincr.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-preincr.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/hush_test/hush-arith/arith-ternary-preincr.tests b/shell/hush_test/hush-arith/arith-ternary-preincr.tests new file mode 100755 index 000000000..3985c7079 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary-preincr.tests @@ -0,0 +1,3 @@ +exec 2>&1 +x='@' +echo 42:$((1?42:++x)) diff --git a/shell/hush_test/hush-arith/arith-ternary1.right b/shell/hush_test/hush-arith/arith-ternary1.right new file mode 100644 index 000000000..6b751d7b8 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary1.right @@ -0,0 +1,2 @@ +42:42 +a=0 diff --git a/shell/hush_test/hush-arith/arith-ternary1.tests b/shell/hush_test/hush-arith/arith-ternary1.tests new file mode 100755 index 000000000..3532ce54d --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary1.tests @@ -0,0 +1,5 @@ +exec 2>&1 +a=0 +# The not-taken branch should not evaluate +echo 42:$((1 ? 42 : (a+=2))) +echo "a=$a" diff --git a/shell/hush_test/hush-arith/arith-ternary2.right b/shell/hush_test/hush-arith/arith-ternary2.right new file mode 100644 index 000000000..a549b1b5c --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary2.right @@ -0,0 +1,3 @@ +6:6 +a=b=+err+ +b=6 diff --git a/shell/hush_test/hush-arith/arith-ternary2.tests b/shell/hush_test/hush-arith/arith-ternary2.tests new file mode 100755 index 000000000..cb3163932 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary2.tests @@ -0,0 +1,7 @@ +exec 2>&1 +a='b=+err+' +b=5 +# The not-taken branch should not parse variables +echo 6:$((0 ? a : ++b)) +echo "a=$a" +echo "b=$b" diff --git a/shell/hush_test/hush-arith/arith-ternary3.right b/shell/hush_test/hush-arith/arith-ternary3.right new file mode 100644 index 000000000..6644d86bf --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary3.right @@ -0,0 +1 @@ +42:42 diff --git a/shell/hush_test/hush-arith/arith-ternary3.tests b/shell/hush_test/hush-arith/arith-ternary3.tests new file mode 100755 index 000000000..0bf9f3002 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary3.tests @@ -0,0 +1,4 @@ +exec 2>&1 +# "EXPR ?..." should check _evaluated_ EXPR, +# not its last value +echo 42:$((1 < 1 ? -1 : 1 > 1 ? 1 : 42)) diff --git a/shell/hush_test/hush-arith/arith-ternary_nested3.right b/shell/hush_test/hush-arith/arith-ternary_nested3.right new file mode 100644 index 000000000..1a34fde65 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary_nested3.right @@ -0,0 +1,2 @@ +42:42 +a=2:2 diff --git a/shell/hush_test/hush-arith/arith-ternary_nested3.tests b/shell/hush_test/hush-arith/arith-ternary_nested3.tests new file mode 100755 index 000000000..b69dcc6e9 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary_nested3.tests @@ -0,0 +1,6 @@ +exec 2>&1 +x='@' +a=2 +# After processing nested ?:, outermost ?: should still rememeber to NOT evaluate a*=2 +echo 42:$((1?0?41:42:(a*=2))) +echo "a=2:$a" diff --git a/shell/math.c b/shell/math.c index a398bcb98..b1aabef9d 100644 --- a/shell/math.c +++ b/shell/math.c @@ -356,6 +356,11 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ NUMPTR = top_of_stack; /* this decrements NUMPTR */ top_of_stack--; /* now points to left side */ + if (math_state->evaluation_disabled) { + dbg("binary op %02x skipped", op); + goto ret_NULL; + } + right_side_val = rez; rez = top_of_stack->val; if (op == TOK_BOR || op == TOK_OR_ASSIGN) @@ -428,6 +433,11 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ } } + if (math_state->evaluation_disabled) { + dbg("unary op %02x skipped", op); + goto ret_NULL; + } + if (is_assign_op(op)) { char buf[sizeof(arith_t)*3 + 2]; @@ -446,6 +456,7 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ } top_of_stack->val = rez; + ret_NULL: /* Erase var name, it is just a number now */ top_of_stack->var_name = NULL; return NULL; @@ -594,8 +605,10 @@ static arith_t strto_arith_t(const char *nptr, char **endptr) static arith_t evaluate_string(arith_state_t *math_state, const char *expr) { +#define EVAL_DISABLED ((unsigned long long)math_state->evaluation_disabled) +#define TOP_BIT_ULL ((unsigned long long)LLONG_MAX + 1) operator lasttok; - const char *errmsg; + const char *errmsg = NULL; const char *start_expr = expr = skip_whitespace(expr); unsigned expr_len = strlen(expr) + 2; /* Stack of integers/names */ @@ -614,7 +627,6 @@ evaluate_string(arith_state_t *math_state, const char *expr) /* Start with a left paren */ dbg("(%d) op:TOK_LPAREN", (int)(opstackptr - opstack)); *opstackptr++ = lasttok = TOK_LPAREN; - errmsg = NULL; while (1) { const char *p; @@ -653,19 +665,26 @@ evaluate_string(arith_state_t *math_state, const char *expr) p = endofname(expr); if (p != expr) { /* Name */ - size_t var_name_size = (p - expr) + 1; /* +1 for NUL */ - numstackptr->var_name = alloca(var_name_size); - safe_strncpy(numstackptr->var_name, expr, var_name_size); - dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name); - expr = skip_whitespace(p); - /* If it is not followed by "=" operator... */ - if (expr[0] != '=' /* not "=..." */ - || expr[1] == '=' /* or "==..." */ - ) { - /* Evaluate variable to value */ - errmsg = arith_lookup_val(math_state, numstackptr); - if (errmsg) - goto err_with_custom_msg; + if (!math_state->evaluation_disabled) { + size_t var_name_size = (p - expr) + 1; /* +1 for NUL */ + numstackptr->var_name = alloca(var_name_size); + safe_strncpy(numstackptr->var_name, expr, var_name_size); + dbg("[%d] var:'%s'", (int)(numstackptr - numstack), numstackptr->var_name); + expr = skip_whitespace(p); + /* If it is not followed by "=" operator... */ + if (expr[0] != '=' /* not "=..." */ + || expr[1] == '=' /* or "==..." */ + ) { + /* Evaluate variable to value */ + errmsg = arith_lookup_val(math_state, numstackptr); + if (errmsg) + goto err_with_custom_msg; + } + } else { + dbg("[%d] var:IGNORED", (int)(numstackptr - numstack)); + numstackptr->var_name = NULL; + numstackptr->val = 0; //paranoia, probably not needed + expr = p; } push_num: numstackptr++; @@ -814,10 +833,11 @@ evaluate_string(arith_state_t *math_state, const char *expr) * pop prev_op * if can't evaluate prev_op (it is lower precedence than op): * push prev_op back - * goto P + * goto C * evaluate prev_op on top of numstack - * P: push op - * N: loop to parse the rest of string + * C:if op is "?": check result, set disable flag if needed + * push op + * N:loop to parse the rest of string */ while (opstackptr != opstack) { operator prev_op = *--opstackptr; @@ -840,7 +860,7 @@ evaluate_string(arith_state_t *math_state, const char *expr) ) { /* ...x~y@. push @ on opstack */ opstackptr++; /* undo removal of ~ op */ - goto push_op; + goto check_cond; } /* else: ...x~y@. Evaluate x~y, replace it on stack with result. Then repeat */ } @@ -863,21 +883,41 @@ dbg(" numstack:%d val:%lld '%s'", (int)(numstackptr - numstack), numstackptr[ errmsg = "malformed ?: operator"; goto err_with_custom_msg; } + /* Example: a=1?2:3,a. We just executed ":". + * Prevent assignment from being still disabled. + */ + math_state->evaluation_disabled >>= 1; + dbg("':' executed: evaluation_disabled=%llx (restored)", EVAL_DISABLED); } } /* while (opstack not empty) */ if (op == TOK_RPAREN) /* unpaired RPAREN? */ goto err; - } + check_cond: + if (op == TOK_CONDITIONAL) { + /* We know the value of EXPR in "EXPR ? ..." + * Should we stop evaluating now? */ + if (math_state->evaluation_disabled & TOP_BIT_ULL) + goto err; /* >63 levels of ?: nesting not supported */ + math_state->evaluation_disabled <<= 1; + if (numstackptr[-1].val == 0) + math_state->evaluation_disabled |= 1; + dbg("'?' entered: evaluation_disabled=%llx", EVAL_DISABLED); + } + } /* if */ /* else: LPAREN or UNARY: push it on opstack */ - push_op: + /* Push this operator to opstack */ dbg("(%d) op:%02x insert_op:%02x", (int)(opstackptr - opstack), op, insert_op); *opstackptr++ = lasttok = op; - next: ; + next: if (insert_op != 0xff) { op = insert_op; insert_op = 0xff; dbg("inserting %02x", op); + if (op == TOK_CONDITIONAL_SEP) { + math_state->evaluation_disabled ^= 1; + dbg("':' entered: evaluation_disabled=%llx (negated)", EVAL_DISABLED); + } goto tok_found1; } } /* while (1) */ @@ -896,6 +936,7 @@ arith(arith_state_t *math_state, const char *expr) { math_state->errmsg = NULL; math_state->list_of_recursed_names = NULL; + math_state->evaluation_disabled = 0; return evaluate_string(math_state, expr); } diff --git a/shell/math.h b/shell/math.h index 41ef6e8df..452ddaddd 100644 --- a/shell/math.h +++ b/shell/math.h @@ -73,13 +73,12 @@ typedef long arith_t; typedef const char* FAST_FUNC (*arith_var_lookup_t)(const char *name); typedef void FAST_FUNC (*arith_var_set_t)(const char *name, const char *val); -//typedef const char* FAST_FUNC (*arith_var_endofname_t)(const char *name); typedef struct arith_state_t { const char *errmsg; arith_var_lookup_t lookupvar; arith_var_set_t setvar; -// arith_var_endofname_t endofname; + uint64_t evaluation_disabled; void *list_of_recursed_names; } arith_state_t; -- cgit v1.2.3