Check redeclarations of functions with bounds declarations. #91
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This changes adds a variable-sized array for parameter bounds to the function type data structure. clang has an ExtProtoInfo structure that holds additional information about prototypes that we extend also. For now, we just test that empty arrays work. We don't actually exercise populating the parameter bounds in ExtProtoInfo. We still have to update AST importing and exporting to save/restore the parameter bounds. Testing: - Passes Checked C regression tests. - Passes clang regression tests.
This change populates bounds information in function types based on parameter bounds information for parameters. Testing: - Passes existing Checked C tests. - Passes clang tests.
This add support for printing parameter bounds expressions in function types. Testing: - Dumped AST by hand and examined output. - Passed Checked C tests. - Passed clang regression tests.
Extend ASTDumper.cpp to include bounds expressions for variables, members, and functions. Add tests of dumping of simple ASTs with bounds expressions. The tests include parameters, local variables, and global variables with bounds expressions, members with bounds expressions, and function return bounds expressions.
For bounds expressions in function types, we will represent references to parameter variables positionally, not using the names. This allows to disregard the parameter names in the type. This change adds an expression for representing positional parameters. It also includes code for canonicalizing bounds expressions to use the positional parameters. That code is currently not tested. Testing: - Passes Checked C regression tests. - Passes clang regression tests.
This change finishes adding bounds expressions for function parameter types. It introduces PositionalParameterExpr, which represents a parameter by its index in a parameter list, not by its name. This requires a fair amount of boiler-plate updates throughout the code. It changes the code for building function types to canonicalize bounds expressions before adding them to the array of parameter bounds. It adds tests of AST dumping that check that bounds expressions for parameters have been rewritten to use positional parameters. It updates AST reading and writing classese to include the parameter bounds array, if there is one. It updates the design documentation to describe the IR choice we have made for Checked C and why, including the choice to use PositionalParameterExpr. Testing: - Passes Checked C regression tests. - Passes existing clang regression tests.
We add a member to FunctionType to hold the return bounds expression. The return bounds expression is abstracted before it is added to the function type. References to parameters are replaced with PositionalParameterExpr's. We could save space for FunctionType objects that do not have return bounds by storing the return bounds as part of the dynamically-allocated array for parameter bounds. We do not do that for now. There's no pressing need to save space and it would make the implementaiton a little more complicated. Testing: - Adding new AST dumping tests that check that return bounds information has been added to function types. - Passes existing Checked C regression tests. - Passes existing clang regression tests.
Handle two cases: - A function with a prototype that has a function pointer that has bounds declarations on its parameters or return is incompatible with a no-prototype version of the function. - When a function is declared using a typedef'ed function type, we need to transfer the bounds information from the type to the function declaration. We need to "concretize" the bounds expressions in this case. This code is not tested yet.
Add checking of bounds declarations on function parameters and returns. The error messages still need work: they simply state "type mismatch", which programmers will found perplexing because the bounds or bounds-safe interfaces are mismatched.
This change adds checking of bounds declarations for function pointer return types. This addresses issue Github microsoft#89. The checking was being done as part of constructing a function declaration. However, function declarations are not constructed for variables with function pointer types. Move the checking from constructing function declarations to constructing function types. To support this, refactor the code for checking bounds declarations so that there are now separate method for (1) checking that a bounds expression is valid for a type (2) checking that a bounds declaration is valid for a declaration. Checking for function return types invokes the first method. The second method invokes the first method. As part of the refactoring, check that bounds-safe interface types are not declared for local variables. This addresses Github issue microsoft#77. Revamp the error messages for return types that have bounds declarations and bounds-safe interface type annotations. There may not be a named entity to refer to in the error messages, so remove mention of that and just point at the problematic bounds expression or annotation type in the error message. It may be useful to make the error messages for variables declarations with bounds be similar to the error messages for return types. Put that off for now to avoid including an unrelated change with this change. Testing: - There will be a separate commit updating the Checked C regression tests. This commit will: -- Fix errors found in existing tests because of the additional checking -- Add additional tests of function pointer parameters and returns with bounds declarations and bounds-safe interfaces. -- Update existing error messages involving bounds declarations and return types -- Add tests making sure that bounds-safe interface types are not allowed for local variables. -- Add tests for unnamed parameters, to make sure that compilers handle this. The clang error messages are confusing when the parameter has no name. We can fix this as part of updating the error messages for variable declarations with bounds. - Update Checked C clang tests. One test checked for two error messages, but there is only one now. It is covered by existing tests, so delete it. Another test was declaring an interface type for a local variable, which is not allowed, so delete that case. - Passes existing clang regression tests.
Merge checking of return bounds declarations. There was an expected conflict in SemaDecl.cpp involving bounds declarations for return types that needed to be resolved. Testing: Passed the corresponding Checked C regression tests in a Checked C repo corresponding to this branch. Had to merge in the tests and correct a few bugs uncovered by the additional checking.
This was referenced Dec 16, 2016
awruef
pushed a commit
that referenced
this pull request
Dec 20, 2016
Now that bounds information is incorporated into function types, use it when determining function type compatibility. This allows the existing machinery for checking redeclarations of functions to check redeclarations of functions with bounds declarations too. It checks that the redeclaration has equivalent bounds declarations to the original declaration. It also checks redeclarations of functions with bounds-safe interfaces. For now, require that the bounds expressions for parameters or returns be identical syntactically for function types, modulo parameter names. We will eventually loosen this requirement to be bounds expressions that are semantically equivalent (for example, canonicalized bounds expressions). Bounds-safe interfaces are treated specially: a parameter or return with unchecked pointer type and no bounds expression is compatible with one with a bounds expression or an interface type. If both parameters or returns have bounds expressions or interface types, the expressions or types must be identical. To determine whether bounds expressions are equivalent, re-use the existing clang "profile" code for determining whether two expressions are equivalent. The main implementation change is to update mergeFunctionTypes in ASTContext.cpp to include bounds information. This allows the replacement or deletion of special-case code that worked around the lack of bounds information in function type. - Code is deleted in Sema::DiagnoseCheckedCFunctionCompatibility, Sema::CheckedCFunctionDeclCompatibility, and GetFullTypeForDeclarator in SemaType.cpp. For GetFullTypeForDeclarator, the new code in isNotAllowedForNoPrototypeFunction subsumes this check. - Code is updated in ASTContext::isNotAllowedForNoPrototypeFunction. A function can be declared via typedef. In this case, clang synthesizes parameters from the function prototype in Sema::ActOFunctionDeclarator. Extend this code to synthesize bounds information from the prototype too. Bounds expressions are abstracted with respect to parameter names in function types using positional indicates. The bounds expressions have to be concretized with respect to the newly built parameters. Currently the error messages say "type mismatch" when bounds do not match. There is a separate work item for improving the error messages (GitHub #83). Testing: - A separate commit to the Checked C repo will add tests of redeclarations of functions. It will also add header files that add Checked C bounds-safe interfaces to functions in the C standard library. - Passes existing Checked C tests. - Passing existing clang regression tests.
dopelsunce
pushed a commit
to dopelsunce/checkedc-clang
that referenced
this pull request
Sep 28, 2020
- Remove the term "draft" from the version description. - Update the acknowledgement section.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now that bounds information is incorporated into function types, use it when determining function type compatibility. This allows the existing machinery for checking redeclarations of functions to check redeclarations of functions with bounds declarations too. It checks that the redeclaration has equivalent bounds declarations to the original declaration. It also checks redeclarations of functions with bounds-safe interfaces.
For now, require that the bounds expressions for parameters or returns be identical syntactically for function types, modulo parameter names. We will eventually loosen this requirement to be bounds expressions that are semantically equivalent (for example, canonicalized bounds expressions). Bounds-safe interfaces are treated specially: a parameter or return with unchecked pointer type and no bounds expression is compatible with one with a bounds expression or an interface type. If both parameters or returns have bounds expressions or interface types, the expressions or types must be identical.
To determine whether bounds expressions are equivalent, re-use the existing clang "profile" code for determining whether two expressions are equivalent.
The main implementation change is to update mergeFunctionTypes in ASTContext.cpp to include bounds information. This allows the replacement or deletion of special-case code that worked around the lack of bounds information in function type.
A function can be declared via typedef. In this case, clang synthesizes parameters from the function prototype in Sema::ActOFunctionDeclarator. Extend this code to synthesize bounds information from the prototype too. Bounds expressions are abstracted with respect to parameter names in function types using positional indicates. The bounds expressions have to be concretized with respect to the newly built parameters.
Currently the error messages say "type mismatch" when bounds do not match. There is a separate work item for improving the error messages (GitHub #83).
Testing: