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

Conditional operators: infer rvalue bounds [2/n] #924

Merged
merged 3 commits into from
Nov 10, 2020

Conversation

kkjeer
Copy link
Contributor

@kkjeer kkjeer commented Oct 20, 2020

This PR infers rvalue bounds for conditional operators e1 ? e2 : e3 as the greatest lower bound of the rvalue bounds of e2 and e3:

  • If the bounds of e2 and e3 are equivalent, the bounds of the conditional operator are the bounds of e2.
  • If the bounds of e2 are bounds(any), the bounds of the conditional operator are the bounds of e3.
  • If the bounds of e3 are bounds(any), the bounds of the conditional operator are the bounds of e2.
  • Otherwise, the bounds of the conditional operator are bounds(unknown).

Testing:

  • Added a new test to verify the inferred bounds for conditional operators.
  • Added expected warnings and errors for conditional operator bounds.
  • Passed automated testing on Linux.

Copy link
Contributor

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@mgrang mgrang requested a review from sulekhark November 7, 2020 00:12
Copy link
Contributor

@mgrang mgrang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kkjeer
Copy link
Contributor Author

kkjeer commented Nov 10, 2020

Some notes from the PR review discussion:

  1. Why are bounds(p, p + 4) not returned as the greatest lower bound of bounds(p, p + 4) and bounds(q, q + 5)?
    A: The current implementation does not order bounds expressions beyond the following: bounds(unknown) < B < bounds(any), where B is a non-unknown, non-any bounds expression. There are complications involved in trying to order bounds expressions, especially if at least one bounds expression is a variable-sized range. Not ordering these bounds expressions avoids the need to split the bounds into a base and offset in order to convert to a range.

  2. What is the CheckingState data structure and what is it used for?
    A: CheckingState is a class that holds several members that are used to maintain an internal state while recursively checking an expression. It is commented in SemaBounds.cpp.

  3. If a pointer p has inferred bounds of any (e.g. from an assignment p = 0), can p be used to access memory? What is the dynamic bounds check here?
    A: The bounds used for the dynamic bounds check are the lvalue bounds of p. E.g. if p has declared bounds bounds(p, p + 5), the runtime check for p[i] will check that 0 <= i < 5.

  4. If a pointer p with declared bounds bounds(p, p + 3) has inferred bounds bounds(p, p + 4), do future assignments to p have to imply bounds(p, p + 4)?
    A: The inferred bounds for an expression only last for one top-level clang CFG statement. The target bounds for a variable are always the programmer-declared bounds (never the inferred bounds). In this example, all assignments to p should imply the declared bounds of bounds(p, p + 3).

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

Successfully merging this pull request may close these issues.

None yet

3 participants