Team2890/allwpilib
4
0
Fork 0
mirror of https://github.com/wpilibsuite/allwpilib synced 2026-06-28 02:11:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

[ntcore] Fix undefined comparison behavior when array is empty (#5448)

Browse Source 
If both arrays are empty, it returns true, avoiding UB with memcmp potentially getting a nullptr.
This commit is contained in:
Gold856
2023-07-17 20:16:54 -04:00
committed by GitHub
parent 17eb9161cd
commit 1f6428ab63
2 changed files with 79 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
ntcore/src/main/native/cpp/Value.cpp
View File

@@ -317,6 +317,9 @@ bool nt::operator==(const Value& lhs, const Value& rhs) {
if (lhs.m_val.data.arr_boolean.size != rhs.m_val.data.arr_boolean.size) {
return false;
}
if (lhs.m_val.data.arr_boolean.size == 0) {
return true;
}
return std::memcmp(lhs.m_val.data.arr_boolean.arr,
rhs.m_val.data.arr_boolean.arr,
lhs.m_val.data.arr_boolean.size *
@@ -325,6 +328,9 @@ bool nt::operator==(const Value& lhs, const Value& rhs) {
if (lhs.m_val.data.arr_int.size != rhs.m_val.data.arr_int.size) {
return false;
}
if (lhs.m_val.data.arr_int.size == 0) {
return true;
}
return std::memcmp(lhs.m_val.data.arr_int.arr, rhs.m_val.data.arr_int.arr,
lhs.m_val.data.arr_int.size *
sizeof(lhs.m_val.data.arr_int.arr[0])) == 0;
@@ -332,6 +338,9 @@ bool nt::operator==(const Value& lhs, const Value& rhs) {
if (lhs.m_val.data.arr_float.size != rhs.m_val.data.arr_float.size) {
return false;
}
if (lhs.m_val.data.arr_float.size == 0) {
return true;
}
return std::memcmp(lhs.m_val.data.arr_float.arr,
rhs.m_val.data.arr_float.arr,
lhs.m_val.data.arr_float.size *
@@ -340,11 +349,17 @@ bool nt::operator==(const Value& lhs, const Value& rhs) {
if (lhs.m_val.data.arr_double.size != rhs.m_val.data.arr_double.size) {
return false;
}
if (lhs.m_val.data.arr_double.size == 0) {
return true;
}
return std::memcmp(lhs.m_val.data.arr_double.arr,
rhs.m_val.data.arr_double.arr,
lhs.m_val.data.arr_double.size *
sizeof(lhs.m_val.data.arr_double.arr[0])) == 0;
case NT_STRING_ARRAY:
if (lhs.m_val.data.arr_string.size == 0) {
return true;
}
return static_cast<StringArrayStorage*>(lhs.m_storage.get())->strings ==
static_cast<StringArrayStorage*>(rhs.m_storage.get())->strings;
default:

64
ntcore/src/test/native/cpp/ValueTest.cpp
View File

@@ -336,6 +336,58 @@ TEST_F(ValueTest, BooleanArrayComparison) {
vec = {1, 0};
v2 = Value::MakeBooleanArray(vec);
ASSERT_NE(v1, v2);
// empty
vec = {};
v1 = Value::MakeBooleanArray(vec);
v2 = Value::MakeBooleanArray(vec);
ASSERT_EQ(v1, v2);
}
TEST_F(ValueTest, IntegerArrayComparison) {
std::vector<int64_t> vec{-42, 0, 1};
auto v1 = Value::MakeIntegerArray(vec);
auto v2 = Value::MakeIntegerArray(vec);
ASSERT_EQ(v1, v2);
// different contents
vec = {-42, 1, 1};
v2 = Value::MakeIntegerArray(vec);
ASSERT_NE(v1, v2);
// different size
vec = {-42, 0};
v2 = Value::MakeIntegerArray(vec);
ASSERT_NE(v1, v2);
// empty
vec = {};
v1 = Value::MakeIntegerArray(vec);
v2 = Value::MakeIntegerArray(vec);
ASSERT_EQ(v1, v2);
}
TEST_F(ValueTest, FloatArrayComparison) {
std::vector<float> vec{0.5, 0.25, 0.5};
auto v1 = Value::MakeFloatArray(vec);
auto v2 = Value::MakeFloatArray(vec);
ASSERT_EQ(v1, v2);
// different contents
vec = {0.5, 0.5, 0.5};
v2 = Value::MakeFloatArray(vec);
ASSERT_NE(v1, v2);
// different size
vec = {0.5, 0.25};
v2 = Value::MakeFloatArray(vec);
ASSERT_NE(v1, v2);
// empty
vec = {};
v1 = Value::MakeFloatArray(vec);
v2 = Value::MakeFloatArray(vec);
ASSERT_EQ(v1, v2);
}
TEST_F(ValueTest, DoubleArrayComparison) {
@@ -353,6 +405,12 @@ TEST_F(ValueTest, DoubleArrayComparison) {
vec = {0.5, 0.25};
v2 = Value::MakeDoubleArray(vec);
ASSERT_NE(v1, v2);
// empty
vec = {};
v1 = Value::MakeDoubleArray(vec);
v2 = Value::MakeDoubleArray(vec);
ASSERT_EQ(v1, v2);
}
TEST_F(ValueTest, StringArrayComparison) {
@@ -390,6 +448,12 @@ TEST_F(ValueTest, StringArrayComparison) {
vec.push_back("goodbye");
v2 = Value::MakeStringArray(std::move(vec));
ASSERT_NE(v1, v2);
// empty
vec.clear();
v1 = Value::MakeStringArray(vec);
v2 = Value::MakeStringArray(std::move(vec));
ASSERT_EQ(v1, v2);
}
} // namespace nt