Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use simple facts about expression equality. Expand checking of bounds…
… declarations. (#476) This change extends the compiler to collect and use simple facts about expression equality when checking bounds declarations. It also turns on checking of bounds declarations for unchecked scopes by default. It was only on by default for checked scopes. The checking of bounds declarations is still simple, so it may produce (many) unnecessary warnings. It can be turned off using `-Wno-check-bounds-decls` (everywhere), `-Wno-check-bounds-decl-unchecked-scope` (for unchecked scopes), and `-Wno-check-bounds-decls-checked-scope` (for checked scopes), and. This change addresses issues #474 and #472. Equality facts are gathered at assignments and initializers. They are not propagated across statements. For example, given `x = y`, when checking the bounds for `x` are implied by the bounds for `y`, we assume `x == y`. In general, given `e1 = e2`, we check that `e1` and `e2` are non-modifying expressions and collect the fact that `e1 == e2`. (We still need to check that the assignment doesn't modify an lvalue used by` e2`, and will add that in the future. We may miss issuing warnings that we should issue,) - We pass a list of sets of equivalent expressions to CanonBounds, which checks whether two expressions compute the same value. We try to compare expressions and if that fails, we consult the list of sets. We check each expression to see if it is equivalent to an expression in a set (we don't recursively use the equality information). - When setting up the equality fact `e1 == e2`, we add an lvalue-to-rvalue cast for the IR for `e1`, . Given a declarator for a variable v with an initializer, we make sure to turn v into a lvalue-to-rvalue cast of a decl-ref. - There was code in SemaBounds.cpp that puts an expression into a standard form by ignoring non-value changing operations. That code is moved to CanonBounds.cpp. - The types of subexpressions of an expression affect the meaning of an expression. When comparing two expressions, check that the types of their subexpressions match. This wasn't a concern before when the code was strictly comparing lexical equality. There is a TODO for work needed for bounds-safe interfaces, tracked by Github issue #475. The change fixes a few bugs found during testing of the code for checking equivalence of expressions: - CanonBounds.cpp needs to treat implicit and C-style casts as being equivalent if they cast to/from the same type. - In SemaBounds.cpp, when we infer the rvalue bounds of a bounds cast expression of the form *_bounds_cast<T>(`e1`, bounds-expr), the bounds need to be for `(T) e1`, not `e1` (`e1` has the wrong type). - In SemaExpr.cpp, when type checking bounds(`e1`, `e2`), we need to check that e1 and e2 point to compatible types, not exactly the same type. Testing: - Updated the Checked C repo tests. This will be covered by a separate pull request: - Added more tests of expression equivalence (lexical_equality should be renamed, as it checks for expression equivalence using a few additional facts about C expressions). - Updated tests to handle the fact that checking of bounds declaration is on everywhere by default. Replace a bunch of TODO's with an expected warning message. Also deleted some warnings that are no longer issued because the compiler can prove the bounds are valid. - Kept checking of bounds declarations on for most tests and only disabled it for some parsing and typechecking tests. - In some cases, made simple changes to avoid warnings about bounds declarations (the original cases are captured in the new Checked C clang repo file test/CheckedC/static-checking/bounds-decl-challenges.c) - Update the Checked C clang tests: - In bounds-decl-checking.c, remove expected warnings that are now gone because the compiler understand equality facts. Add code that checks the compiler understands equality facts involving arrays. Include a negative test where the compiler concludes that an array is too small for the declared bounds for an array_ptr variable. - Passed automated testing on Linux x64 (clang regression tests and LNT testing)
- Loading branch information
