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

Check for initializers on struct/array variables #445

Closed
dtarditi opened this issue Jan 31, 2018 · 1 comment
Closed

Check for initializers on struct/array variables #445

dtarditi opened this issue Jan 31, 2018 · 1 comment

Comments

@dtarditi
Copy link
Contributor

If a variable has a member or element with checked pointer type that may be used to access memory, it must be initialized when declared. We currently aren't checking this. We are checking that variables that have check pointer types that may be used to access memory have initializers.

dtarditi added a commit to microsoft/checkedc that referenced this issue Jan 31, 2018
Variables with checked pointer type that may be used to access memory must be initialized when they are declared.  This also applies to variables with members or elements with checked pointer type that may be used to access memory.  Update the specification to make this clear.   Include some examples.  This addresses issue #98.

The compiler already checks these requirements for scalar variables.  I opened microsoft/checkedc-clang#445 to track checking these requirements for struct and array variables.

Add tests for the struct/array cases to the Checked C tests.
dtarditi added a commit that referenced this issue Mar 15, 2018
…aces. (#461)

This change extends the Checked C version of clang to allow both  interop types and bounds expressions in a bounds-safe interfaces.  Before, we allowed only one or the other, which didn't provide proper support for pointer types with multiple levels of pointers.  A bounds expression and an interop type expression can now appear after the colon, in any order.   For example, a programmers can write `int f(int **p : itype(array_ptr<ptr<int>>) count(len, int len)`.    This change addresses issue #445 and also fixes issue #443.

Within the IR, we were storing both bounds expression and interop types using a single field in declarations and function t ypes.  We also stored interop types as bounds expressions, even though they were really a different AST node.  The change replaces these occurrences of `BoundsExpr *` with the value class `BoundsAnnotations`.   It took some gyrations before I arrived at this simple representation choice.   This abstraction let us add additional information such as where clauses later.   Also, InteropTypeBoundsAnnotation is no lo longer a subclass of BoundsExpr.  It is a distinct class that inherits directly from Expr.   The changes led to a large amount of boilerplate changes: replacing occurrences of BoundsExpr *, modifying the AST class structure, and so on.

The changes  also include:
1.  Modifying the code that adds bounds expressions/interop type to AST nodes to infer bounds expressions and interop types when one of them is missing.  We now always infer an interop type from the existence of a bounds expression (this change fixes the causes of issue #443).  We infer bounds for checked parameter arrays and nt_array_ptrs from types or interop types.  (there are still declarations where we don't infer bound by default when perhaps we could).   Before, we could not infer bounds for interop types because we had no place to put them.
2.  Revising the bounds inference code to handle the fact that both may appear now. We try use bounds expressions before trying to infer bounds based on the interop type.
3.  Improving the error messages generated for more conflicts between interop types on redeclarations.
4, Centralizing the code for parsing what I now call "bounds annotations" (a bounds expression and a bounds annotation).   This change simplifies the code, as we no longer have duplicated logic for deferred parsing of bounds expressions.
5. Checked that bounds expressions aren't declared for interop types that are ptr types.

I also updated some code in the Checked C rewriter that was accessing interop type information.

Testing:
- Modify existing clang Checked C tests for revised error messages and AST dumps from the compiler.
- Adding new tests of bounds expressions/interop types to the Checked C repo.  That will be covered by a separate PR.
- Removing a workaround in the Checked C LLVM test suite for KSDdist.  That will be covered by a separate PR
- Pass revised Checked C clang tests, Checked C regression tests, and LNT testing
awruef pushed a commit to awruef/checkedc-clang that referenced this issue Apr 4, 2018
…aces. (microsoft#461)

This change extends the Checked C version of clang to allow both  interop types and bounds expressions in a bounds-safe interfaces.  Before, we allowed only one or the other, which didn't provide proper support for pointer types with multiple levels of pointers.  A bounds expression and an interop type expression can now appear after the colon, in any order.   For example, a programmers can write `int f(int **p : itype(array_ptr<ptr<int>>) count(len, int len)`.    This change addresses issue microsoft#445 and also fixes issue microsoft#443.

Within the IR, we were storing both bounds expression and interop types using a single field in declarations and function t ypes.  We also stored interop types as bounds expressions, even though they were really a different AST node.  The change replaces these occurrences of `BoundsExpr *` with the value class `BoundsAnnotations`.   It took some gyrations before I arrived at this simple representation choice.   This abstraction let us add additional information such as where clauses later.   Also, InteropTypeBoundsAnnotation is no lo longer a subclass of BoundsExpr.  It is a distinct class that inherits directly from Expr.   The changes led to a large amount of boilerplate changes: replacing occurrences of BoundsExpr *, modifying the AST class structure, and so on.

The changes  also include:
1.  Modifying the code that adds bounds expressions/interop type to AST nodes to infer bounds expressions and interop types when one of them is missing.  We now always infer an interop type from the existence of a bounds expression (this change fixes the causes of issue microsoft#443).  We infer bounds for checked parameter arrays and nt_array_ptrs from types or interop types.  (there are still declarations where we don't infer bound by default when perhaps we could).   Before, we could not infer bounds for interop types because we had no place to put them.
2.  Revising the bounds inference code to handle the fact that both may appear now. We try use bounds expressions before trying to infer bounds based on the interop type.
3.  Improving the error messages generated for more conflicts between interop types on redeclarations.
4, Centralizing the code for parsing what I now call "bounds annotations" (a bounds expression and a bounds annotation).   This change simplifies the code, as we no longer have duplicated logic for deferred parsing of bounds expressions.
5. Checked that bounds expressions aren't declared for interop types that are ptr types.

I also updated some code in the Checked C rewriter that was accessing interop type information.

Testing:
- Modify existing clang Checked C tests for revised error messages and AST dumps from the compiler.
- Adding new tests of bounds expressions/interop types to the Checked C repo.  That will be covered by a separate PR.
- Removing a workaround in the Checked C LLVM test suite for KSDdist.  That will be covered by a separate PR
- Pass revised Checked C clang tests, Checked C regression tests, and LNT testing
@dtarditi
Copy link
Contributor Author

@sxl463 implemented this in PR #506.

sulekhark pushed a commit that referenced this issue Feb 27, 2021
The removed conditional stopped constraint variables being created for fields of
structures that were defined in macros. The original reason for adding this is
unclear, but removing it seems to be correct.
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

No branches or pull requests

1 participant