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 memory safety problem with Array.from_cpointer #3675

Merged
merged 4 commits into from
Jan 27, 2021

Conversation

ergl
Copy link
Member

@ergl ergl commented Oct 6, 2020

Fixes #3668. String already checked for a potential null pointer before constructing, so it was safe. Also added a test case to check this.

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Oct 6, 2020
@ponylang-main
Copy link
Contributor

Hi @ergl,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 3675.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

@ergl
Copy link
Member Author

ergl commented Oct 7, 2020

Not sure what's up with that failed check, it's erroring with Segmentation fault (core dumped), but didn't fail every time. Is this something that has happened before? (also, didn't think my last commit would trigger ci, maybe [skip ci] only applies outside PRs?)

@SeanTAllen
Copy link
Member

There's a bug I'm working on that causes periodic segfaults. That's probably what the issue is.

@SeanTAllen SeanTAllen changed the title Prevent Array.from_cpointer from accepting a null pointer Fix memory safety problem with Array.from_cpointer Oct 7, 2020
@@ -0,0 +1,7 @@
## Fix memory safety problem with Array.from_cpointer
Copy link
Member

Choose a reason for hiding this comment

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

Excellent title.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

.release-notes/3675.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jasoncarr0 jasoncarr0 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for all the fixes

@ergl
Copy link
Member Author

ergl commented Oct 8, 2020

@SeanTAllen This should be ready to merge, I believe. Do you want to wait until the periodic segfault issue is resolved?

@SeanTAllen
Copy link
Member

@ergl I'm going to wait to merge for a bit until hopefully the segfault is resolved. The next release is in 3 weeks (if not sooner). It should definitely be merged by then.

@SeanTAllen SeanTAllen merged commit 9a812a4 into ponylang:master Jan 27, 2021
github-actions bot pushed a commit that referenced this pull request Jan 27, 2021
github-actions bot pushed a commit that referenced this pull request Jan 27, 2021
@ergl ergl deleted the ergl/fix-3668 branch January 28, 2021 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array.from_cpointer with CPointer.create violates memory safety
4 participants