Skip to content

Removed forced unsigned int cast in type_def bit-shift operators #1178

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 11, 2025
Merged

Removed forced unsigned int cast in type_def bit-shift operators #1178

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f22e3aa
Removed UB in type_def bit-shift operators
ockeydockey Sep 2, 2025
4724d01
Changed shift operators to allow both signed and unsigned operands fo…
ockeydockey Sep 9, 2025
d17dca8
Fixed constexpr errors for CPP11
ockeydockey Sep 10, 2025
b380fce
Changed is_arithmetic checks to use is_integral since valid shifts re…
ockeydockey Sep 10, 2025
f7a828a
Removed need for CPP11 since changes are CPP03 compatible
ockeydockey Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 58 additions & 4 deletions include/etl/type_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,28 @@ namespace etl
}

//*********************************************************************
ETL_CONSTEXPR14 type_def& operator <<=(int rhs) ETL_NOEXCEPT
template <typename T>
ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, type_def&>::type
#else
type_def&
#endif
operator <<=(T rhs) ETL_NOEXCEPT
{
value <<= rhs;
return *this;
}

//*********************************************************************
ETL_CONSTEXPR14 type_def& operator >>=(int rhs) ETL_NOEXCEPT
template <typename T>
ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, type_def&>::type
#else
type_def&
#endif
operator >>=(T rhs) ETL_NOEXCEPT
{
value >>= rhs;
return *this;
Expand Down Expand Up @@ -589,19 +603,59 @@ namespace etl
//*********************************************************************
// << operator
//*********************************************************************
friend ETL_CONSTEXPR type_def operator <<(const type_def& lhs, int rhs) ETL_NOEXCEPT
template <typename T>
friend ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, type_def>::type
#else
type_def
#endif
operator <<(const type_def& lhs, T rhs) ETL_NOEXCEPT
{
return type_def(lhs.value << rhs);
}

//*********************************************************************
template <typename T>
friend ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type
#else
T
#endif
operator <<(T lhs, const type_def& rhs) ETL_NOEXCEPT
{
return lhs << rhs.value;
}

//*********************************************************************
// >> operator
//*********************************************************************
friend ETL_CONSTEXPR type_def operator >>(const type_def& lhs, int rhs) ETL_NOEXCEPT
template <typename T>
friend ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, type_def>::type
#else
type_def
#endif
operator >>(const type_def& lhs, T rhs) ETL_NOEXCEPT
{
return type_def(lhs.value >> rhs);
}

//*********************************************************************
template <typename T>
friend ETL_CONSTEXPR
#if ETL_USING_CPP11
typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type
#else
T
#endif
operator >>(T lhs, const type_def& rhs) ETL_NOEXCEPT
{
return lhs >> rhs.value;
}

Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Symmetry with ‘T >> type_def’: promotions and negative counts

  • Consider returning decltype(lhs >> rhs.value) to match built‑ins (C++11+), or document truncation to T.
  • Constrain T to integrals and cast the count to unsigned.
-#if ETL_USING_CPP11
-      typename etl::enable_if<etl::is_arithmetic<T>::value, T>::type
+#if ETL_USING_CPP11
+      typename etl::enable_if<etl::is_integral<T>::value, T>::type

Body (outside the hunk):

-  return lhs >> rhs.value;
+  // ETL_ASSERT(rhs.value >= 0);
+  return lhs >> static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value);

🏁 Script executed:

#!/bin/bash
# Locate the definition of type_def to inspect TValue and member 'value'
rg -nP '^(template\s*<[^>]+>\s*)?(class|struct)\s+type_def\b' -C5 include/etl/type_def.h

# Search for any alias or typedef of TValue within the same file
rg -nP 'using\s+TValue' -n include/etl/type_def.h
rg -nP 'typedef\s+.*TValue' -n include/etl/type_def.h

# Show the operator>> implementation context around the original snippet
sed -n '640,680p' include/etl/type_def.h

Length of output: 1215

Restrict operator>>(T, type_def) to integral T and guard against negative counts

  • Change the enable_if to etl::is_integral<T>::value instead of is_arithmetic<T>
  • Return decltype(lhs >> rhs.value) (C++11+) or document that returning T truncates the promoted result
  • Insert ETL_ASSERT(rhs.value >= 0); and cast the shift count to an unsigned type (e.g.
    static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value)) to avoid undefined behaviour on negative values
🤖 Prompt for AI Agents 
In include/etl/type_def.h around lines 646 to 658, the binary right-shift
operator is too permissive and can UB on negative shift counts: change the
enable_if condition from etl::is_arithmetic<T>::value to
etl::is_integral<T>::value so only integral lhs types are allowed; for C++11
builds change the return type to decltype(lhs >> rhs.value) (guarded by
ETL_USING_CPP11) so the promoted result type is preserved (otherwise leave the
existing T return but add a note); add ETL_ASSERT(rhs.value >= 0); and cast the
shift count to an unsigned type when invoking the builtin shift, e.g.
static_cast<typename etl::make_unsigned<TValue>::type>(rhs.value), to avoid
undefined behaviour from negative shift counts.

//*********************************************************************
// < operator
//*********************************************************************
Expand Down
19 changes: 16 additions & 3 deletions test/test_type_def.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ namespace
CHECK_EQUAL(i |= 0x003DU, uint32_t(t |= type_t(0x003DU)));
CHECK_EQUAL(i ^= 0xAA55U, uint32_t(t ^= 0xAA55U));
CHECK_EQUAL(i ^= 0xAA55U, uint32_t(t ^= type_t(0xAA55U)));
CHECK_EQUAL(i <<= 2, uint32_t(t <<= 2));
CHECK_EQUAL(i <<= 2U, uint32_t(t <<= 2U));
CHECK_EQUAL(i >>= 2, uint32_t(t >>= 2));
CHECK_EQUAL(i >>= 2U, uint32_t(t >>= 2U));
CHECK_EQUAL(i %= 23, uint32_t(t %= 23));

Expand All @@ -195,8 +197,10 @@ namespace

uint32_t i1 = 0x5A3DUL;
uint32_t i2 = 0xB47AUL;
uint32_t i3 = 3UL;
type_t t1(0x5A3DUL);
type_t t2(0xB47AUL);
type_t t3(3UL);

CHECK_EQUAL(i1 + Two, t1 + Two);
CHECK_EQUAL(Two + i1, Two + t1);
Expand All @@ -205,11 +209,11 @@ namespace
CHECK_EQUAL(i1 - Two, t1 - Two);
CHECK_EQUAL(i2 - i1, i2 - t1);
CHECK_EQUAL(i2 - i1, t2 - t1);

CHECK_EQUAL(i1 * Two, t1 * Two);
CHECK_EQUAL(Two * i1, Two * t1);
CHECK_EQUAL(i1 * i2, t1 * t2);

CHECK_EQUAL(i1 / Two, t1 / Two);
CHECK_EQUAL(i2 / i1, i2 / t1);
CHECK_EQUAL(i2 / i1, t2 / t1);
Expand All @@ -227,7 +231,16 @@ namespace
CHECK_EQUAL(uint32_t(0xAA55) ^ i1, type_t(0xAA55UL) ^ t1);

CHECK_EQUAL(i1 << 2, t1 << 2);
CHECK_EQUAL(i1 << 2U, t1 << 2U);
CHECK_EQUAL(2 << i1, 2 << t1);
CHECK_EQUAL(2U << i1, 2U << t1);
CHECK_EQUAL(2U << i1, 2U << t1);
CHECK_EQUAL(i1 << i3, t1 << t3);
CHECK_EQUAL(i1 >> 2, t1 >> 2);
CHECK_EQUAL(i1 >> 2U, t1 >> 2U);
CHECK_EQUAL(2 >> i1, 2 >> t1);
CHECK_EQUAL(2U >> i1, 2U >> t1);
CHECK_EQUAL(i1 >> i3, t1 >> t3);

CHECK_EQUAL(i1 % uint32_t(23), t1 % uint32_t(23));
CHECK_EQUAL(uint32_t(23) % i1, uint32_t(23) % t1);
Expand Down Expand Up @@ -313,7 +326,7 @@ namespace

return value;
}

TEST(test_arithmetic_constexpr)
{
constexpr arithmetic_type_t value_plus = CreatePlus();
Expand Down
Loading