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

Fix compiler crash rewriting return function pointer type. #483

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

dtarditi
Copy link
Member

Summary:

The compiler hit an assert while rewrting types with bounds-safe interfaces to make them checked types. This problem was found by trying to call the signal functionin a checked scope. The problem occurred when a function return type was a function pointer type with a bounds-safe interface.
The construction of new type location information asserted about a mismatch in the expected size of the type location buffer.

The problem was that we were rewriting parameter and return types and then modifying types using interop type information. We need to do this in the opposite order to construct new type location information properly.

Detailed explanation:

Clang has a binary serialization of type locations, where it expects source locations for types that are components of a type to be serialized as part of the type location information for the enclosing type. For types with type location information, the representation divides data into "local data" - data specific to the type and "child data" (type location information for the child). The local data is followed by the child data when laying out data. The local data may be variable length. The code is in include\clang\ast\TypeLoc.h. which has poor commenting and relies on obscure template meta-programming. See getInnerTypeLoc() for the messy details of how they decided to do this.

During tree transforms, if types are rewritten, the transform maintains this serialization. That takes some work because the tree transform typically operates bottom-up: transform child nodes of an AST node and then transform the AST node. This means that the type location information for a child type node is rewritten before the parent type node information (which is variable length and may change the position of the child node's type location). The solution is to use a type location builder: the data for a child node is accumulated into the type location builder. For a parent node, the type location build buffer is expanded if necessary. The data for the child node is moved to the back of the buffer.

The problem was that the type location information for a child type was out-of-sync with what was expected, if we replaced the child type with a new type. based on bounds-safe information. The specific child node in question was the return type. The solution is to choose return type, generate the type location information by recursively transforming the return type, and then filling in the data for the parent (the function prototype).

Testing:

  • Add tests in the Checked C repo of calls in checked scopes to functions that
    return pointer types with bounds-safe interfaces.

The compiler hit an assert while rewrting types with bounds-safe interfaces
to make them checked types.   This problem was found by trying to call
the signal functionin  a checked scope.   The problem occurred when a
function return type was a function pointer type with a bounds-safe interface.
The construction of new type location information asserted about a mismatch
in the expected size of the type location buffer.

The problem was that we were rewriting parameter and return types and then
modifying types using interop type information.  We need to do this in the
opposite order to construct new type location information properly.

Testing:
- Add tests in the Checked C repo of calls in checked scopes to functions that
 return pointer types with bounds-safe interfaces.
Copy link
Collaborator

@awruef awruef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK.

dtarditi added a commit to checkedc/checkedc that referenced this pull request Apr 26, 2018
… interfaces. (#281)

The compiler crashed when `signal` with a bounds-safe interface was called in a checked scope.  PR checkedc/checkedc-clang#483 fixes this.    This change adds more tests of type checking of functions that return function pointers, including testing uses in and outside of checked scopes.
@dtarditi dtarditi merged commit 9f9ccfc into master Apr 26, 2018
@dtarditi dtarditi deleted the issue478 branch April 26, 2018 22:31
sulekhark pushed a commit that referenced this pull request Jul 8, 2021
Use of VarArg parameters are assumed to be unsafe even though CheckedC will
accept them with checked pointer types. If we want to support VarArgs with
checked pointer types, we can remove the constraint to WILD here. We would then
need to update TypeExprRewriter to rewrite the type in these expression.
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.

2 participants