Skip to content

Conditional fail with constant numbers #308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 of 3 tasks
Dayvison opened this issue Jun 1, 2018 · 1 comment
Closed
1 of 3 tasks

Conditional fail with constant numbers #308

Dayvison opened this issue Jun 1, 2018 · 1 comment

Comments

@Dayvison
Copy link

Dayvison commented Jun 1, 2018

Is this a BUG REPORT, FEATURE REQUEST or QUESTION?:

  • Bug Report
  • Feature Request
  • Question

What happened:
The condition should give true, but instead give false
What you expected to happen:

How to reproduce it (as minimally and precisely as possible):

new min1 = 15;
new max1 = 30;
#define min2  15
#define max2  30
main()
{
    printf("%d", (min1 < 20 < max1));
    printf("%d", (min2 < 20 < max2));
    printf("%d", (15 < 20 < 30));
}

Anything else we need to know?:
Const, define and explict numbers are the same
new + const works

new const min = 15;
new const max = 30;
main()
{
    printf("%d", (min < 20 < max));
}

Environment:

  • Operating System Linux
  • Compiler version 3.10.7(All versions do the same)
  • How are you invoking the compiler? Pawno, Sublime, vscode, sampctl or command-line? command-line
  • If using sampctl, the version number
@YashasSamaga
Copy link
Member

YashasSamaga commented Jun 9, 2018

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 result.

Why does new const qualified variables do not show this bug?

Because the compiler does not try to evaluate the expression at compile-time. If you look at the generated assembly, there are instructions to carry out the less-than operation at run-time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants