Skip to content

Disallow casts to null-terminated pointers in checked scopes. #391

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

Closed
dtarditi opened this issue Oct 5, 2017 · 2 comments
Closed

Disallow casts to null-terminated pointers in checked scopes. #391

dtarditi opened this issue Oct 5, 2017 · 2 comments
Assignees
Labels
work item This labels issues that are not exactly bugs but are about improvements.

Comments

@dtarditi
Copy link
Member

dtarditi commented Oct 5, 2017

We should not allow casts from unchecked, ptr, or array_ptr values to null-terminated in checked scopes. Those pointers might not pointed to a null-terminated array, or the null terminator may be overwritten.

@dtarditi
Copy link
Member Author

dtarditi commented Jul 4, 2019

I believe we are already disallow implicit casts. This work item is to disallow explicit casts.

dtarditi pushed a commit that referenced this issue Jul 18, 2019
Disallow cast from other checked pointer types to nt_array_ptr in
checked scopes because the source pointer might not point to a
NULL_terminated array. Casting from an unchecked pointer to a
nt_array_ptr pointer should also be prohibited; this has already been
handled as no unchecked pointers are allowed in checked scopes.

Also added a new error message in
clang/include/clang/Basic/DiagnosticSemaKinds.td for casting to
nt_array_ptr in checked scopes.

The test file tests/typechecking/checked_scope_basic.c was updated
with a new function test_cast_to_nt_array_ptr to test
casting to nt_array_ptr.
mgrang pushed a commit that referenced this issue Sep 27, 2019
Cherry-picked from commit e13fcff

    Disallow cast from other checked pointer types to nt_array_ptr in
    checked scopes because the source pointer might not point to a
    NULL_terminated array. Casting from an unchecked pointer to a
    nt_array_ptr pointer should also be prohibited; this has already been
    handled as no unchecked pointers are allowed in checked scopes.

    Also added a new error message in
    clang/include/clang/Basic/DiagnosticSemaKinds.td for casting to
    nt_array_ptr in checked scopes.

    The test file tests/typechecking/checked_scope_basic.c was updated
    with a new function test_cast_to_nt_array_ptr to test
    casting to nt_array_ptr.
dopelsunce pushed a commit to dopelsunce/checkedc-clang that referenced this issue Sep 28, 2020
* Reenable expected warning for bounds cast memory access

* Add additional bounds cast memory access test
@sulekhark sulekhark assigned mgrang and unassigned jiezhoucs Feb 3, 2021
@mgrang
Copy link

mgrang commented Feb 4, 2021

This was fixed in #626 and tests were added in checkedc/checkedc#367.

@mgrang mgrang closed this as completed Feb 4, 2021
sulekhark pushed a commit that referenced this issue Feb 27, 2021
…nge. (#391)

This PR addresses function and variable declarations (because they are
the most obvious case and reasonably straightforward) and checked
regions (because they came up in some existing regression tests). We'll
leave #387 open for the tail of unhandled cases.

Also:

- When 3C tries to change a non-writable file, issue an error diagnostic
  (not an assertion failure because there are known unhandled cases)
  rather than silently discarding the change.

- Add a `-dump-unwritable-changes` flag to the `3c` tool to dump the new
  version of the file when that diagnostic appears.

- Add an error diagnostic when 3C tries to change a file under the base
  dir other than the main file in stdout mode. This is a separate
  feature (part of #328) but ended up being easy to implement along with
  the diagnostic for a non-writable file.

- Add tests for all the fixes (but not `-dump-unwritable-changes`).

- Fix a bug where `-warn-all-root-cause` didn't work without
  `-warn-root-cause`, because this affected one of the new tests. The
  use of `-warn-all-root-cause` without `-warn-root-cause` in the
  affected test will serve as a regression test for this fix as well.

Fixes part of #387 and a few unrelated minor issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work item This labels issues that are not exactly bugs but are about improvements.
Projects
None yet
Development

No branches or pull requests

3 participants