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

Add null violation checks for std::unique_ptr, std::shared_ptr, std::optional and std::expected to prevent UB #945

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

bluetarpmedia
Copy link
Contributor

assert_not_null now verifies std::unique_ptr, std::shared_ptr, std::optional and std::expected in addition to the original null pointer check.

These states are considered to be "null" and will result in a violation error if dereferenced or accessed:

  • std::unique_ptr that owns nothing
  • std::shared_ptr with no managed object
  • std::optional with no value
  • std::expected containing an unexpected value

Null violation errors will be reported in the following cases:

test: () = {
    ex: std::expected<int, bool> = std::unexpected(false);
    op: std::optional<int> = std::nullopt;

    up:= unique.new<int>(1);
    sp:= shared.new<int>(2);
    up.reset();
    sp.reset();

    v1 := ex*;   // Null violation error
    v2 := op*;   // Null violation error
    v3 := up*;   // Null violation error
    v4 := sp*;   // Null violation error
}

Add new regression tests:

  • pure2-assert-expected-not-null
  • pure2-assert-optional-not-null
  • pure2-assert-shared-ptr-not-null
  • pure2-assert-unique-ptr-not-null

…:optional and std::expected in addition to the original null pointer check

These states are considered to be "null" and will result in a violation error:

- std::unique_ptr that owns nothing
- std::shared_ptr with no managed object
- std::optional with no value
- std::expected containing an unexpected value
Copy link
Contributor

@JohelEGP JohelEGP left a comment

Choose a reason for hiding this comment

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

This seems good enough.

It seems like a more general solution would use x is T when CPP2_TYPEOF(x) wraps a T.
But we just can't guarantee that the precondition of x* is the same as x is T.

@hsutter hsutter merged commit 96a4abc into hsutter:main Jan 17, 2024
15 checks passed
@hsutter
Copy link
Owner

hsutter commented Jan 17, 2024

Thanks!

@bluetarpmedia bluetarpmedia deleted the assert_not_null branch January 22, 2024 01:42
@JohelEGP
Copy link
Contributor

The test output isn't platform-independent.
See #556:

  • Update tests that finish through std::terminate.
    • Add std::set_terminate(std::abort); to avoid platform-dependent output on termination.

hsutter added a commit that referenced this pull request Jan 25, 2024
bluetarpmedia pushed a commit to bluetarpmedia/cppfront that referenced this pull request Feb 7, 2024
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.

3 participants