Skip to content
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

Fix assert in bounds widening "BinaryNode operator must equal parent operator" #932

Closed
mgrang opened this issue Nov 4, 2020 · 1 comment · Fixed by #943
Closed

Fix assert in bounds widening "BinaryNode operator must equal parent operator" #932

mgrang opened this issue Nov 4, 2020 · 1 comment · Fixed by #943
Assignees
Labels
bug This labels issues that are bugs.

Comments

@mgrang
Copy link

mgrang commented Nov 4, 2020

Handle the following case in bounds widening:

void f(int i) {
  _Nt_array_ptr<char> p : count(0) = "a";

  if (*(p + (0 * 1)))
  {}
}

Currently, we hit an assert:

clang-9: /usr/local/magrang/master/src/clang/lib/AST/PreorderAST.cpp:46: void clang::PreorderAST::RemoveNode(clang::Node*, clang::Node*): Assertion `B->Opc == P->Opc && "BinaryNode operator must equal parent operator"' failed.

This is because we have incorrectly added the requirement that in order to remove a BinaryNode its opcode should be equal to its parent opcode which is not valid in this case.

@mgrang mgrang self-assigned this Nov 4, 2020
@mgrang mgrang added the bug This labels issues that are bugs. label Nov 4, 2020
mgrang pushed a commit that referenced this issue Nov 17, 2020
We had erroneously added the condition that to coalesce nodes the binary
operators of the parent and child node must be equal. This fails for the case
where we first constant-fold nodes having a differen operator than the parent
and then need to coalesce the constant folded node into the parent.

Fixes issue #932
@mgrang
Copy link
Author

mgrang commented Nov 17, 2020

Fixed in #943

mgrang pushed a commit that referenced this issue Nov 19, 2020
…s parent (#943)

We can only coalesce if the operator of the current and parent node is
commutative and associative. This is because after coalescing we later
need to sort the nodes and if the operator is not commutative and
associative then sorting would be incorrect.

We can coalesce in the following scenarios:
1. The current and parent nodes have the same operator OR
2. The current node is the only child of its operator node (maybe as a
result of constant folding).

Fixes issue #932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This labels issues that are bugs.
Projects
None yet
1 participant