Skip to content

Change StaticObject instance management to hopefully avoid UBSAN#470

Merged
AzothAmmo merged 1 commit into
USCiLab:developfrom
erichkeane:develop
Mar 16, 2018
Merged

Change StaticObject instance management to hopefully avoid UBSAN#470
AzothAmmo merged 1 commit into
USCiLab:developfrom
erichkeane:develop

Conversation

@erichkeane
Copy link
Copy Markdown
Contributor

The static-object hack currently used requires passing an
uninitialized reference as a reference. This fix uses a cast
to void to hopefully avoid that undefined behavior.

The static-object hack currently used requires passing an
uninitialized reference as a reference.  This fix uses a cast
to void to hopefully avoid that undefined behavior.
@unravel-dev
Copy link
Copy Markdown

Have you tested if this would get optimized by the compiler when optimizations are enabled?

@AzothAmmo
Copy link
Copy Markdown
Contributor

@volcoma The CI checks are done with optimization on; for Windows we run both a debug build and a release (optimized) build. If it's passing those everything should in theory be good, but I'll check this one out locally first to double check.

@AzothAmmo AzothAmmo added this to the v1.2.3 milestone Mar 12, 2018
@AzothAmmo AzothAmmo merged commit 0cf8150 into USCiLab:develop Mar 16, 2018
@AzothAmmo
Copy link
Copy Markdown
Contributor

Seems like this still works; if it blows up down the line we can easily revert the change.

AzothAmmo added a commit that referenced this pull request Mar 16, 2018
WSoptics pushed a commit to WSoptics/cereal that referenced this pull request Mar 23, 2018
WSoptics pushed a commit to WSoptics/cereal that referenced this pull request Mar 23, 2018
jrmadsen pushed a commit to jrmadsen/cereal that referenced this pull request Jul 7, 2019
jrmadsen pushed a commit to jrmadsen/cereal that referenced this pull request Jul 7, 2019
JerryRyan pushed a commit to JerryRyan/cereal that referenced this pull request Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants