diff options
author | Denys Vlasenko | 2023-06-14 11:05:48 +0200 |
---|---|---|
committer | Denys Vlasenko | 2023-06-14 11:07:30 +0200 |
commit | 46dccd2ec0eafd850b2168d4dfe4e74949fd3424 (patch) | |
tree | cb853ae6459c7fd7c48fbcaa42d46056fe77c03f | |
parent | a02450ff0bfa45618e72fc7103ea3a8f0e7fff80 (diff) | |
download | busybox-46dccd2ec0eafd850b2168d4dfe4e74949fd3424.zip busybox-46dccd2ec0eafd850b2168d4dfe4e74949fd3424.tar.gz |
shell/math: fix nested ?: and do not parse variables in not-taken branch
Fixes arith-ternary1.tests and arith-ternary_nested.tests
function old new delta
evaluate_string 1043 1101 +58
arith_apply 1087 1137 +50
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 108/0) Total: 108 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
-rw-r--r-- | shell/ash_test/ash-arith/arith-ternary1.right | 3 | ||||
-rwxr-xr-x | shell/ash_test/ash-arith/arith-ternary1.tests | 7 | ||||
-rw-r--r-- | shell/ash_test/ash-arith/arith-ternary2.right | 4 | ||||
-rwxr-xr-x | shell/ash_test/ash-arith/arith-ternary2.tests | 7 | ||||
-rw-r--r-- | shell/ash_test/ash-arith/arith-ternary_nested.right | 1 | ||||
-rwxr-xr-x | shell/ash_test/ash-arith/arith-ternary_nested.tests | 2 | ||||
-rw-r--r-- | shell/math.c | 80 |
7 files changed, 61 insertions, 43 deletions
diff --git a/shell/ash_test/ash-arith/arith-ternary1.right b/shell/ash_test/ash-arith/arith-ternary1.right index c968f11..6b751d7 100644 --- a/shell/ash_test/ash-arith/arith-ternary1.right +++ b/shell/ash_test/ash-arith/arith-ternary1.right @@ -1,5 +1,2 @@ 42:42 a=0 -6:6 -a=b=+err+ -b=6 diff --git a/shell/ash_test/ash-arith/arith-ternary1.tests b/shell/ash_test/ash-arith/arith-ternary1.tests index 5a54e34..3532ce5 100755 --- a/shell/ash_test/ash-arith/arith-ternary1.tests +++ b/shell/ash_test/ash-arith/arith-ternary1.tests @@ -3,10 +3,3 @@ a=0 # The not-taken branch should not evaluate echo 42:$((1 ? 42 : (a+=2))) echo "a=$a" - -a='b=+err+' -b=5 -# The not-taken branch should not even parse variables -echo 6:$((0 ? a : ++b)) -echo "a=$a" -echo "b=$b" diff --git a/shell/ash_test/ash-arith/arith-ternary2.right b/shell/ash_test/ash-arith/arith-ternary2.right index aa54bd9..a549b1b 100644 --- a/shell/ash_test/ash-arith/arith-ternary2.right +++ b/shell/ash_test/ash-arith/arith-ternary2.right @@ -1 +1,3 @@ -5:5 +6:6 +a=b=+err+ +b=6 diff --git a/shell/ash_test/ash-arith/arith-ternary2.tests b/shell/ash_test/ash-arith/arith-ternary2.tests index eefc8e7..cb31639 100755 --- a/shell/ash_test/ash-arith/arith-ternary2.tests +++ b/shell/ash_test/ash-arith/arith-ternary2.tests @@ -1,2 +1,7 @@ exec 2>&1 -echo 5:$((1?2?3?4?5:6:7:8:9)) +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/ash_test/ash-arith/arith-ternary_nested.right b/shell/ash_test/ash-arith/arith-ternary_nested.right new file mode 100644 index 0000000..aa54bd9 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested.right @@ -0,0 +1 @@ +5:5 diff --git a/shell/ash_test/ash-arith/arith-ternary_nested.tests b/shell/ash_test/ash-arith/arith-ternary_nested.tests new file mode 100755 index 0000000..eefc8e7 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested.tests @@ -0,0 +1,2 @@ +exec 2>&1 +echo 5:$((1?2?3?4?5:6:7:8:9)) diff --git a/shell/math.c b/shell/math.c index 9ca7c6b..2b7a349 100644 --- a/shell/math.c +++ b/shell/math.c @@ -331,6 +331,28 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ top_of_stack = NUMPTR - 1; + if (op == TOK_CONDITIONAL_SEP) { + /* "expr1 ? expr2 : expr3" operation */ + var_or_num_t *expr1 = &top_of_stack[-2]; + if (expr1 < numstack) { + return "malformed ?: operator"; + } + err = arith_lookup_val(math_state, expr1); + if (err) + return err; + if (expr1->val != 0) /* select expr2 or expr3 */ + top_of_stack--; + err = arith_lookup_val(math_state, top_of_stack); + if (err) + return err; + NUMPTR = expr1 + 1; + expr1->val = top_of_stack->val; + expr1->var_name = NULL; + return NULL; + } + if (op == TOK_CONDITIONAL) /* Example: $((a ? b)) */ + return "malformed ?: operator"; + /* Resolve name to value, if needed */ err = arith_lookup_val(math_state, top_of_stack); if (err) @@ -350,25 +372,12 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ else if (op != TOK_UPLUS) { /* Binary operators */ arith_t right_side_val; - int bad_second_val; /* Binary operators need two arguments */ if (top_of_stack == numstack) goto syntax_err; /* ...and they pop one */ NUMPTR = top_of_stack; /* this decrements NUMPTR */ - - bad_second_val = (top_of_stack->var_name == SECOND_VAL_VALID); - if (op == TOK_CONDITIONAL) { /* ? operation */ - /* Make next if (...) protect against - * $((expr1 ? expr2)) - that is, missing ": expr" */ - bad_second_val = !bad_second_val; - } - if (bad_second_val) { - /* Protect against $((expr <not_?_op> expr1 : expr2)) */ - return "malformed ?: operator"; - } - top_of_stack--; /* now points to left side */ if (op != TOK_ASSIGN) { @@ -380,19 +389,7 @@ arith_apply(arith_state_t *math_state, operator op, var_or_num_t *numstack, var_ right_side_val = rez; rez = top_of_stack->val; - if (op == TOK_CONDITIONAL) /* ? operation */ - rez = (rez ? right_side_val : top_of_stack[1].second_val); - else if (op == TOK_CONDITIONAL_SEP) { /* : operation */ - if (top_of_stack == numstack) { - /* Protect against $((expr : expr)) */ - return "malformed ?: operator"; - } - top_of_stack->val = rez; - top_of_stack->second_val = right_side_val; - top_of_stack->var_name = SECOND_VAL_VALID; - return NULL; - } - else if (op == TOK_BOR || op == TOK_OR_ASSIGN) + if (op == TOK_BOR || op == TOK_OR_ASSIGN) rez |= right_side_val; else if (op == TOK_OR) rez = right_side_val || rez; @@ -833,7 +830,7 @@ evaluate_string(arith_state_t *math_state, const char *expr) lasttok = TOK_NUM; goto next; } - /* Not (y), but ...x~y): fall through to evaluate x~y */ + /* Not (y), but ...x~y). Fall through to evaluate x~y */ } else { operator prev_prec = PREC(prev_op); fix_assignment_prec(prec); @@ -841,11 +838,17 @@ evaluate_string(arith_state_t *math_state, const char *expr) if (prev_prec < prec || (prev_prec == prec && is_right_associative(prec)) ) { - /* ...x~y@: push @ on opstack */ - opstackptr++; /* undo removal of ~ op */ - goto push_op; + /* Unless a?b?c:d:... and we are at the second : */ + if (op != TOK_CONDITIONAL_SEP + || prev_op != TOK_CONDITIONAL_SEP + ) { + /* ...x~y@: push @ on opstack */ + opstackptr++; /* undo removal of ~ op */ + goto push_op; + } + /* else: a?b?c:d:. Evaluate b?c:d, replace it on stack with result. Then repeat */ } - /* ...x~y@: evaluate x~y, replace it on stack with result. Then repeat */ + /* else: ...x~y@. Evaluate x~y, replace it on stack with result. Then repeat */ } dbg("arith_apply(prev_op:%02x, numstack:%d)", prev_op, (int)(numstackptr - numstack)); errmsg = arith_apply(math_state, prev_op, numstack, &numstackptr); @@ -856,6 +859,21 @@ dbg(" numstack:%d val:%lld %lld %p", (int)(numstackptr - numstack), numstackptr[-1].var_name == SECOND_VAL_VALID ? numstackptr[-1].second_val : 0, numstackptr[-1].var_name ); + /* For ternary ?: we need to remove ? from opstack too, not just : */ + if (prev_op == TOK_CONDITIONAL_SEP) { + // This is caught in arith_apply() + //if (opstackptr == opstack) { + // /* Example: $((2:3)) */ + // errmsg = "where is your ? in ?:"; + // goto err_with_custom_msg; + //} + opstackptr--; + if (*opstackptr != TOK_CONDITIONAL) { + /* Example: $((1,2:3)) */ + errmsg = "malformed ?: operator"; + goto err_with_custom_msg; + } + } } /* while (opstack not empty) */ if (op == TOK_RPAREN) /* unpaired RPAREN? */ goto err; |