From 1578ab15a32aafe6ea2cf909abe8cecd112726ff Mon Sep 17 00:00:00 2001 From: Yashas Date: Sat, 9 Jun 2018 10:58:20 +0530 Subject: [PATCH 1/2] fix result propogation in constexpr operator chains ## compile-time chained operator evaluation mechanism Constants are stored in `value` object. The object has two members: `constval` and `boolresult`. The `constval` member stores the actual value which the object represents: `15` and `10` for example in `15 < 20`. All operators are in a chain of operators. On most occasions, there is just one operator in the chain but it is still a valid singleton chain. The `boolresult` member stores the result of the previous operations which the constant was involved in if any. For example, in `15 < 20 < 30`, the `boolresult` member of object representing `20` stores the result of `15 < 20`. In more complex expressions such as, `15 < 20 < 30 < 40 < 50`, the `boolresult` member of object representing `40` stores the result of the `15 < 20 < 30 < 40`. The `boolresult` of the first constant in a chain is set to `TRUE` by default. **Details:** The processing of chains begins at `plnge_rel` which maintains two `value` objects: `lval` and `lval2`. The `lval` is taken by `plnge_rel` as an argument of the first constant (stored in `constval` field) value in the operator chain. The result of the full chain is also passed through it by storing the result in the `constval` field (like `15 < 20` evaluates to the constant `1`; the result of `15 < 20` is `1` which is also a constant). The `boolresult` of `lval` is set to `TRUE` as this is the first constant in the chain. The `plnge_rel` progresses through the chain one by one with `lval` and `lval2` always being the left operand and the right operand respectively. For each operator it calls `plnge2` which dumps instructions for loading constant and the right operand into the staging buffer. If the right operand turns out to be a constant, the staging buffer is flushed and `static cell calc(cell left,void (*oper)(),cell right,char *boolresult);` is called to evaluate the compile-time expression with the constant value of the left and right operand passed as `left` and `right` arguments along with the operator and `boolresult` of the left operand. `calc` evluates the expression and returns the right operand and sets the `boolresult` to the result of the operator chain. After the call returns, `constval` of `lval` (left operand) is set to the `constval` of `lval2` (right operand). As of now, the `lval` now stores the right operand and the result of the operator chain uptil now. `plnge2` returns and control is transfered back to `plnge_rel`. `plnge_rel` copies the value stored in `lval2` into `lval`. This implies that the `boolresult` and `constval` fields of `lval2` are copied into `lval` as the previous right operand is now the left operand for the next operator in the chain. **Oops! BUG!** The `boolresult` which was stored in `lval` is lost and is replaced by `FALSE` which was stored in `lval2` which is the default for `value` objets (set by `clear_value`). The compiler should've ensured that the `boolresult` of `lval` was retained. `plnge2` is again called to process the right operand and evaluate the expression. This keeps repeating until the entire chain has been evaluated. After the entire chain has been evaluated, `lval` has `constval` as the most recent right operand and `boolresult` has the result of the entire chain. The compiler sets `constval` of `lval` to `boolresult` of `lval` and its tag to `bool`. Note that this is done in the last step. The `lval` now is a constant representing the result of the entire chain of operators, i.e. `1` if the chain evaluated to `TRUE` or `0` otherwise. **Relavant bugged code:** ``` static int plnge_rel(int *opstr,int opoff,int (*hier)(value *lval),value *lval) { . . . lval->boolresult=TRUE; do { /* same check as in plnge(), but "chkbitwise" is always TRUE */ if (count>0 && bitwise_opercount!=0) error(212); if (count>0) { relop_prefix(); *lval=lval2; /* copy right hand expression of the previous iteration */ } /* if */ opidx+=opoff; plnge2(op1[opidx],hier,lval,&lval2); if (count++>0) relop_suffix(); } while (nextop(&opidx,opstr)); /* enddo */ lval->constval=lval->boolresult; lval->tag=pc_addtag("bool"); /* force tag to be "bool" */ return FALSE; /* result of expression is not an lvalue */ } ``` ``` static void plnge2(void (*oper)(void), int (*hier)(value *lval), value *lval1,value *lval2) { . . . if (check_userop(oper,lval1->tag,lval2->tag,2,NULL,&lval1->tag)) { lval1->ident=iEXPRESSION; lval1->constval=0; } else if (lval1->ident==iCONSTEXPR && lval2->ident==iCONSTEXPR) { /* only constant expression if both constant */ stgdel(index,cidx); /* scratch generated code and calculate */ check_tagmismatch(lval1->tag,lval2->tag,FALSE,-1); lval1->constval=calc(lval1->constval,oper,lval2->constval,&lval1->boolresult); } else { . . . } ``` ``` static cell calc(cell left,void (*oper)(),cell right,char *boolresult) { . . . else if (oper==os_le) return *boolresult &= (char)(left <= right), right; else if (oper==os_ge) return *boolresult &= (char)(left >= right), right; else if (oper==os_lt) return *boolresult &= (char)(left < right), right; else if (oper==os_gt) return *boolresult &= (char)(left > right), right; . . . } ``` As you can see, the `boolresult` is ANDed with the result of the current operation. If one of the operators in the chain earlier had evaluated to false, the `&` will force `boolresult` to remain `FALSE` irrespective of whether the current expression evaluates to true or not. ### What's the bug? The compiler loses the `boolresult` of `lval` in `plnge_rel` and replaces it with the `boolresult` of `lval2` which is always initalized to `FALSE` by `clear_val`. The fix is to ensure that `lval` retains the value. This commits does this by copying the `boolresult` of `lval2` into `lval` before `*lval=lval2`. This ensures that the `boolresult` is retained in `lval`. --- source/compiler/sc3.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c index c632b40..5895db9 100644 --- a/source/compiler/sc3.c +++ b/source/compiler/sc3.c @@ -512,6 +512,7 @@ static int plnge_rel(int *opstr,int opoff,int (*hier)(value *lval),value *lval) error(212); if (count>0) { relop_prefix(); + lval2.boolresult=lval->boolresult; *lval=lval2; /* copy right hand expression of the previous iteration */ } /* if */ opidx+=opoff; From 394330854b8f27ec4b4061c9091509cfa6a242d9 Mon Sep 17 00:00:00 2001 From: Yashas Date: Sun, 8 Jul 2018 14:47:23 +0530 Subject: [PATCH 2/2] add test for i308 --- source/compiler/tests/CMakeLists.txt | 8 ++++++++ .../tests/constexpr_result_prop_gh_308.pwn | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 source/compiler/tests/constexpr_result_prop_gh_308.pwn diff --git a/source/compiler/tests/CMakeLists.txt b/source/compiler/tests/CMakeLists.txt index ef77935..bad72be 100644 --- a/source/compiler/tests/CMakeLists.txt +++ b/source/compiler/tests/CMakeLists.txt @@ -52,6 +52,14 @@ set_tests_properties(meaningless_class_specifiers_gh_172 PROPERTIES PASS_REGULAR .*\\.pwn\\(1 \\-\\- 2\\) : warning 238: meaningless combination of class specifiers \\(const variable arguments\\) ") +add_compiler_test(constexpr_result_prop_gh_308 ${CMAKE_CURRENT_SOURCE_DIR}/constexpr_result_prop_gh_308.pwn) +set_tests_properties(constexpr_result_prop_gh_308 PROPERTIES PASS_REGULAR_EXPRESSION +".*\\.pwn\\(2\\) : warning 237: user warning: \\\"Test passed.\\\" +.*\\.pwn\\(6\\) : warning 237: user warning: \\\"Test passed.\\\" +.*\\.pwn\\(10\\) : warning 237: user warning: \\\"Test passed.\\\" +.*\\.pwn\\(14\\) : warning 237: user warning: \\\"Test passed.\\\" +") + # Crashers # # These tests simply check that the compiler doesn't crash. diff --git a/source/compiler/tests/constexpr_result_prop_gh_308.pwn b/source/compiler/tests/constexpr_result_prop_gh_308.pwn new file mode 100644 index 0000000..0592bad --- /dev/null +++ b/source/compiler/tests/constexpr_result_prop_gh_308.pwn @@ -0,0 +1,19 @@ +#if (30 < 40 < 50) + #warning "Test passed." +#endif + +#if !(30 < 40 < 35) + #warning "Test passed." +#endif + +#if (30 < 40) + #warning "Test passed." +#endif + +#if !(40 < 35) + #warning "Test passed." +#endif + +main () { + +}