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

Disallow inferred bounds containing modifying expressions. #480

Merged
merged 5 commits into from
Apr 21, 2018
Merged

Conversation

dtarditi
Copy link
Member

Bounds expressions aren't allowed to contain modifying expressions. It is possible for bounds inference to create bounds expressions containing modifying expressions when inferring bounds expressions for member operator expressions.

We have code that attempts to disallow the creation of invalid bounds expressions by making the bounds be unknown. This code is wrong because it can cause the compiler to skip checking bounds declarations when the inferred target bounds for the LHS of an assignment contains a modifying expression.

Instead of trying to hack the results of the inference analysis, cast a wide net by directly checking that bounds expressions created by bounds inference are non-modifying expressions. This directly prevents the creation of bounds checks with side-effects embedded in them and incorrect analysis of bounds expressions with side effects.

  • Rework the checking of bounds for call expressions to avoid the complicated short-circuiting code. The new code is simpler and easier to understand.
  • Generalize the diagnostics code for identifying modifying bounds expressions to allow notes to be printed. This lets us print notes identifying problematic modifying expressions. Change the code to prevent duplicate messages. Print an error message the first time an expression is seen. The expansion of bounds to a standard form can create the situation where an expression occurs multiple times.
  • Delete disabled code for identifying problematic modifying arguments. The current approach of specifically identifying the argument in the context of a specific bounds is better because it is more precise, and easier for a programmer to understand.
  • The code for identifying volatile variables used in a bounds expression was too restrictive: it flagged even taking the address of a volatile variable. We should only flag reads of volatile variables.

Testing:

  • Added new tests to the Checked C repo of expressions that result in inferred bounds that contain modifying expressions. It was tough to even come up with code that did that, beyond structure member accesses.
  • Passing existing Checked C tests on Linux (clang and LNT tests, including converted code with LNT tests).
  • Passed Checked C tests on Windows locally.

- Don't expand parameter bounds first and then substitute the
  actual arguments.  This leads to error messages with unnecessary
  casts in them.
- Suppress printing duplicate errors for the same modifying expression
  used multiple times in a bounds expression.
@dtarditi dtarditi merged commit d99b3d3 into master Apr 21, 2018
@dtarditi dtarditi deleted the issue479 branch April 26, 2018 18:46
sulekhark pushed a commit that referenced this pull request Jul 8, 2021
* change assert to consistency fail(macro)

* remove checked regions around inconsistent function calls; fix logic error

* add macro check

* don't add type params if they're all inconsistant

* add test

* remove commented code

* allow existing macro type params to be valid

* minor efficientcy

* use more appropriate rewrite check
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.

1 participant