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

typecheck return bounds expression for function pointer types #89

Closed
dtarditi opened this issue Dec 10, 2016 · 1 comment
Closed

typecheck return bounds expression for function pointer types #89

dtarditi opened this issue Dec 10, 2016 · 1 comment
Assignees
Milestone

Comments

@dtarditi
Copy link
Contributor

The Checked C clang implementation is not typechecking returnb ounds expressions for function pointer types. The following two methods compile without errors:

void fn121(int (*fnptr)(void) : count(5));
void fn122(array_ptr<void> (*fnptr)(void) : count(5));

The first case declares a return bounds expression of count(5) for an integer return value, which is not legal. The second case declares a return bounds expression of count(5) for an array_ptr return value, which is also illegal.

@dtarditi dtarditi self-assigned this Dec 10, 2016
@dtarditi dtarditi added this to the Sprint 11 milestone Dec 10, 2016
dtarditi added a commit to dtarditi/checkedc-clang that referenced this issue Dec 13, 2016
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.
dtarditi added a commit that referenced this issue Dec 14, 2016
This change adds checking of bounds declarations for function pointer return types.  This addresses GitHub issue #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 construction of function declarations to construction of function types.

To support this, refactor the code for checking bounds declarations so that there are now separate methods 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 existing checking for variable declarations 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 #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 variable 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 in the Checked C repo.   That 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.
@dtarditi
Copy link
Contributor Author

This issue has been addressed.

dtarditi pushed a commit that referenced this issue Aug 12, 2020
This commit resolves the assertion violation without breaking any
regression tests. There definitely still some problems in the handling
of AddrOf.
dtarditi pushed a commit that referenced this issue Aug 12, 2020
dopelsunce pushed a commit to dopelsunce/checkedc-clang that referenced this issue Sep 28, 2020
*Add a paragraph describing bounds declarations for integer-typed variables.   These are needed when a pointer is cast to an integer that is then assigned to a variable.  This brings the specification into agreement with the clang Checked C implementation, which already allows this.    This address issue microsoft#79.

Also fix some typos in the interoperation section and clarify some wording.
Adjust the tag in the example to only be one-bit wide.   There are some additional complications with handling multi-bit tags.   See GitHub issue microsoft#89 for details.  To avoid these complications, we just change the example to use a 1-bit tag.

Fix a bad LaTeX page break for an example by removing a line from an example,    There was a single line on one page with the bad page break.
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