Skip to content

Commit

Permalink
Fix undefined behavior in operator[]
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@kdt3rd
kdt3rd committed Nov 1, 2024
1 parent 0ef09f5 commit f1fffe6
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 18 deletions.
12 changes: 6 additions & 6 deletions src/Imath/ImathMatrix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1518,14 +1518,14 @@ template <class T>
IMATH_HOSTDEVICE inline T*
Matrix22<T>::getValue () IMATH_NOEXCEPT
{
return (T*) &x[0][0];
return reinterpret_cast<T*> (this);
}

template <class T>
IMATH_HOSTDEVICE inline const T*
Matrix22<T>::getValue () const IMATH_NOEXCEPT
{
return (const T*) &x[0][0];
return reinterpret_cast<const T*> (this);
}

template <class T>
Expand Down Expand Up @@ -2161,14 +2161,14 @@ template <class T>
IMATH_HOSTDEVICE inline T*
Matrix33<T>::getValue () IMATH_NOEXCEPT
{
return (T*) &x[0][0];
return reinterpret_cast<T*> (this);
}

template <class T>
IMATH_HOSTDEVICE inline const T*
Matrix33<T>::getValue () const IMATH_NOEXCEPT
{
return (const T*) &x[0][0];
return reinterpret_cast<const T*> (this);
}

template <class T>
Expand Down Expand Up @@ -3553,14 +3553,14 @@ template <class T>
IMATH_HOSTDEVICE inline T*
Matrix44<T>::getValue () IMATH_NOEXCEPT
{
return (T*) &x[0][0];
return reinterpret_cast<T*> (this);
}

template <class T>
IMATH_HOSTDEVICE inline const T*
Matrix44<T>::getValue () const IMATH_NOEXCEPT
{
return (const T*) &x[0][0];
return reinterpret_cast<const T*> (this);
}

template <class T>
Expand Down
16 changes: 14 additions & 2 deletions src/Imath/ImathShear.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,21 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Shear6
/// @}

/// Element access
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to xy, xz, etc when
/// possible should be preferred.
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 T& operator[] (int i);

/// Element access
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to xy, xz, etc when
/// possible should be preferred.
IMATH_HOSTDEVICE constexpr const T& operator[] (int i) const;

/// @{
Expand Down Expand Up @@ -332,14 +344,14 @@ template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline T&
Shear6<T>::operator[] (int i)
{
return (&xy)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T>
IMATH_HOSTDEVICE constexpr inline const T&
Shear6<T>::operator[] (int i) const
{
return (&xy)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<const T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T>
Expand Down
56 changes: 46 additions & 10 deletions src/Imath/ImathVec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,21 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Vec2
/// @}

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y when
/// possible should be preferred.
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 T& operator[] (int i) IMATH_NOEXCEPT;

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y when
/// possible should be preferred.
IMATH_HOSTDEVICE constexpr const T& operator[] (int i) const IMATH_NOEXCEPT;

/// @{
Expand Down Expand Up @@ -351,9 +363,21 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Vec3
/// @}

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y, z when
/// possible should be preferred.
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 T& operator[] (int i) IMATH_NOEXCEPT;

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y, z when
/// possible should be preferred.
IMATH_HOSTDEVICE constexpr const T& operator[] (int i) const IMATH_NOEXCEPT;

/// @{
Expand Down Expand Up @@ -672,9 +696,21 @@ template <class T> class IMATH_EXPORT_TEMPLATE_TYPE Vec4
/// @}

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y, z, w when
/// possible should be preferred.
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 T& operator[] (int i) IMATH_NOEXCEPT;

/// Element access by index.
///
/// NB: This method of access uses dynamic array accesses which
/// can prevent compiler optimizations and force temporaries to be
/// stored to the stack and other missed vectorization
/// opportunities. Use of direct access to x, y, z, w when
/// possible should be preferred.
IMATH_HOSTDEVICE constexpr const T& operator[] (int i) const IMATH_NOEXCEPT;

/// @{
Expand Down Expand Up @@ -1186,14 +1222,14 @@ template <class T>
IMATH_CONSTEXPR14 IMATH_HOSTDEVICE inline T&
Vec2<T>::operator[] (int i) IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T>
constexpr IMATH_HOSTDEVICE inline const T&
Vec2<T>::operator[] (int i) const IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<const T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T> IMATH_HOSTDEVICE inline Vec2<T>::Vec2 () IMATH_NOEXCEPT
Expand Down Expand Up @@ -1274,14 +1310,14 @@ template <class T>
IMATH_HOSTDEVICE inline T*
Vec2<T>::getValue () IMATH_NOEXCEPT
{
return (T*) &x;
return reinterpret_cast<T*> (this);
}

template <class T>
IMATH_HOSTDEVICE inline const T*
Vec2<T>::getValue () const IMATH_NOEXCEPT
{
return (const T*) &x;
return reinterpret_cast<const T*> (this);
}

template <class T>
Expand Down Expand Up @@ -1590,14 +1626,14 @@ template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline T&
Vec3<T>::operator[] (int i) IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T>
IMATH_HOSTDEVICE constexpr inline const T&
Vec3<T>::operator[] (int i) const IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<const T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T> IMATH_HOSTDEVICE inline Vec3<T>::Vec3 () IMATH_NOEXCEPT
Expand Down Expand Up @@ -1720,14 +1756,14 @@ template <class T>
IMATH_HOSTDEVICE inline T*
Vec3<T>::getValue () IMATH_NOEXCEPT
{
return (T*) &x;
return reinterpret_cast<T*> (this);
}

template <class T>
IMATH_HOSTDEVICE inline const T*
Vec3<T>::getValue () const IMATH_NOEXCEPT
{
return (const T*) &x;
return reinterpret_cast<const T*> (this);
}

template <class T>
Expand Down Expand Up @@ -2063,14 +2099,14 @@ template <class T>
IMATH_HOSTDEVICE IMATH_CONSTEXPR14 inline T&
Vec4<T>::operator[] (int i) IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T>
IMATH_HOSTDEVICE constexpr inline const T&
Vec4<T>::operator[] (int i) const IMATH_NOEXCEPT
{
return (&x)[i]; // NOSONAR - suppress SonarCloud bug report.
return reinterpret_cast<const T*> (this)[i]; // NOSONAR - suppress SonarCloud bug report.
}

template <class T> IMATH_HOSTDEVICE inline Vec4<T>::Vec4 () IMATH_NOEXCEPT
Expand Down

0 comments on commit f1fffe6

Please sign in to comment.