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 undefined behavior in operator[] #449

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

kdt3rd
Copy link
Contributor

@kdt3rd kdt3rd commented Nov 1, 2024

If you take the address of a member, the pointer is only valid within the size of that member, such that accesses past that (i.e. to get to y or z of a vec3) are undefined behavior. by using this with a reinterpret cast, that tells the compiler the pointer space contains all values, and so avoids that. However, this may prevent other optimizations, so a larger change is recommended.

Addresses #446, thanks to @AlexMWells for the analysis

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

matches discussion notes

src/Imath/ImathShear.h Outdated Show resolved Hide resolved
Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM

If you take the address of a member, the pointer is only valid within
the size of that member, such that accesses past that (i.e. to get to y
or z of a vec3) are undefined behavior. by using this with a reinterpret
cast, that tells the compiler the pointer space contains all values, and
so avoids that. However, this may prevent other optimizations, so a
larger change is recommended.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

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

LGTM

@kdt3rd kdt3rd merged commit cc0d361 into AcademySoftwareFoundation:main Nov 2, 2024
31 checks passed
@kdt3rd kdt3rd deleted the fix_446 branch November 2, 2024 20:17
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.

4 participants