Skip to content

Fix unsound C++ union copy-constructor#3892

Merged
emilk merged 8 commits intomainfrom
emilk/fix-segfault
Oct 17, 2023
Merged

Fix unsound C++ union copy-constructor#3892
emilk merged 8 commits intomainfrom
emilk/fix-segfault

Conversation

@emilk
Copy link
Copy Markdown
Member

@emilk emilk commented Oct 17, 2023 •
edited by github-actions bot
Loading

What

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@emilk emilk added 🪳 bug Something isn't working 💣 crash crash, deadlock/freeze, do-no-start sdk-cpp C/C++ API specific exclude from changelog PRs with this won't show up in CHANGELOG.md labels Oct 17, 2023
@Wumpf Wumpf self-requested a review October 17, 2023 10:30
Wumpf
Wumpf approved these changes Oct 17, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Looking good, but I feel like this warrants a dedicated unit test using one of the test types?

@emilk
Copy link
Copy Markdown
Member Author

emilk commented Oct 17, 2023

Looking good, but I feel like this warrants a dedicated unit test using one of the test types?

How would you unit-test against UB without a sanatizer though?

@Wumpf
Copy link
Copy Markdown
Member

Wumpf commented Oct 17, 2023

How would you unit-test against UB without a sanatizer though?

we can't. but for me this caused deterministic failure, so covering that code by a test would be good

@emilk emilk merged commit 7f6e0af into main Oct 17, 2023
@emilk emilk deleted the emilk/fix-segfault branch October 17, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wumpf Wumpf Wumpf approved these changes

Assignees

No one assigned

Labels

🪳 bug Something isn't working 💣 crash crash, deadlock/freeze, do-no-start exclude from changelog PRs with this won't show up in CHANGELOG.md sdk-cpp C/C++ API specific

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Some C++ code-examples seg-fault

2 participants

@emilk @Wumpf