base: Fix integer conversion in MaskLeastSignificant, add more asserts
T x = (1 << bits) was being truncated for sizeof(T) > sizeof(bits).
Also add more static_asserts to BITSTRUCT_DEFINE_END to make it more
error-proof.
Test: make test-art-{host,target}-gtest-bit_{struct,utils}_test{32,64}
Change-Id: Ifedf53c1211b4a9492ebd785c321a39d906dc87a
diff --git a/runtime/base/bit_struct_detail.h b/runtime/base/bit_struct_detail.h
index 9f629c0..824d7df 100644
--- a/runtime/base/bit_struct_detail.h
+++ b/runtime/base/bit_struct_detail.h
@@ -56,20 +56,6 @@
/* else */ type_unsigned>::type;
};
- // Ensure the minimal type storage for 'T' matches its declared BitStructSizeOf.
- // Nominally used by the BITSTRUCT_DEFINE_END macro.
- template <typename T>
- static constexpr bool ValidateBitStructSize() {
- const size_t kBitStructSizeOf = BitStructSizeOf<T>();
- const size_t kExpectedSize = (BitStructSizeOf<T>() < kBitsPerByte)
- ? kBitsPerByte
- : RoundUpToPowerOfTwo(kBitStructSizeOf);
-
- // Ensure no extra fields were added in between START/END.
- const size_t kActualSize = sizeof(T) * kBitsPerByte;
- return kExpectedSize == kActualSize;
- }
-
// Denotes the beginning of a bit struct.
//
// This marker is required by the C++ standard in order to
@@ -84,6 +70,49 @@
private:
typename MinimumTypeUnsignedHelper<kSize>::type _;
};
+
+ // Check if type "T" has a member called _ in it.
+ template <typename T>
+ struct HasUnderscoreField {
+ private:
+ using TrueT = std::integral_constant<bool, true>::type;
+ using FalseT = std::integral_constant<bool, false>::type;
+
+ template <typename C>
+ static constexpr auto Test(void*) -> decltype(std::declval<C>()._, TrueT{}); // NOLINT
+
+ template <typename>
+ static constexpr FalseT Test(...);
+
+ public:
+ static constexpr bool value = decltype(Test<T>(0))::value;
+ };
+
+ // Infer the type of the member of &T::M.
+ template <typename T, typename M>
+ M GetMemberType(M T:: *);
+
+ // Ensure the minimal type storage for 'T' matches its declared BitStructSizeOf.
+ // Nominally used by the BITSTRUCT_DEFINE_END macro.
+ template <typename T>
+ static constexpr bool ValidateBitStructSize() {
+ static_assert(std::is_union<T>::value, "T must be union");
+ static_assert(std::is_standard_layout<T>::value, "T must be standard-layout");
+ static_assert(HasUnderscoreField<T>::value, "T must have the _ DefineBitStructSize");
+
+ const size_t kBitStructSizeOf = BitStructSizeOf<T>();
+ static_assert(std::is_same<decltype(GetMemberType(&T::_)),
+ DefineBitStructSize<kBitStructSizeOf>>::value,
+ "T::_ must be a DefineBitStructSize of the same size");
+
+ const size_t kExpectedSize = (BitStructSizeOf<T>() < kBitsPerByte)
+ ? kBitsPerByte
+ : RoundUpToPowerOfTwo(kBitStructSizeOf);
+
+ // Ensure no extra fields were added in between START/END.
+ const size_t kActualSize = sizeof(T) * kBitsPerByte;
+ return kExpectedSize == kActualSize;
+ }
} // namespace detail
} // namespace art
diff --git a/runtime/base/bit_struct_test.cc b/runtime/base/bit_struct_test.cc
index 872ada3..9fc9762 100644
--- a/runtime/base/bit_struct_test.cc
+++ b/runtime/base/bit_struct_test.cc
@@ -70,17 +70,10 @@
int8_t data;
};
-template <typename T>
-void ZeroInitialize(T& value) {
- memset(&value, 0, sizeof(T));
- // TODO: replace with value initialization
-}
-
TEST(BitStructs, Custom) {
CustomBitStruct expected(0b1111);
- BitStructField<CustomBitStruct, /*lsb*/4, /*width*/4> f;
- ZeroInitialize(f);
+ BitStructField<CustomBitStruct, /*lsb*/4, /*width*/4> f{}; // NOLINT
EXPECT_EQ(1u, sizeof(f));
@@ -102,8 +95,7 @@
VALIDATE_BITSTRUCT_SIZE(TestTwoCustom);
- TestTwoCustom cst;
- ZeroInitialize(cst);
+ TestTwoCustom cst{}; // NOLINT
// Test the write to most-significant field doesn't clobber least-significant.
cst.f4_a = CustomBitStruct(0b0110);
@@ -130,8 +122,7 @@
}
TEST(BitStructs, Number) {
- BitStructNumber<uint16_t, /*lsb*/4, /*width*/4> bsn;
- ZeroInitialize(bsn);
+ BitStructNumber<uint16_t, /*lsb*/4, /*width*/4> bsn{}; // NOLINT
EXPECT_EQ(2u, sizeof(bsn));
bsn = 0b1111;
@@ -163,8 +154,7 @@
EXPECT_EQ(1u, sizeof(u4));
EXPECT_EQ(1u, sizeof(alias_all));
}
- TestBitStruct tst;
- ZeroInitialize(tst);
+ TestBitStruct tst{}; // NOLINT
// Check minimal size selection is correct.
EXPECT_EQ(1u, sizeof(TestBitStruct));
@@ -239,8 +229,7 @@
TEST(BitStructs, Mixed) {
EXPECT_EQ(4u, sizeof(MixedSizeBitStruct));
- MixedSizeBitStruct tst;
- ZeroInitialize(tst);
+ MixedSizeBitStruct tst{}; // NOLINT
// Check operator assignment.
tst.u3 = 0b111u;
diff --git a/runtime/base/bit_utils.h b/runtime/base/bit_utils.h
index da3c704..5d83654 100644
--- a/runtime/base/bit_utils.h
+++ b/runtime/base/bit_utils.h
@@ -388,7 +388,8 @@
if (bits >= BitSizeOf<T>()) {
return std::numeric_limits<unsigned_T>::max();
} else {
- return static_cast<unsigned_T>((1 << bits) - 1);
+ auto kOne = static_cast<unsigned_T>(1); // Do not truncate for T>size_t.
+ return static_cast<unsigned_T>((kOne << bits) - kOne);
}
}
diff --git a/runtime/base/bit_utils_test.cc b/runtime/base/bit_utils_test.cc
index 0276d8d..3a80600 100644
--- a/runtime/base/bit_utils_test.cc
+++ b/runtime/base/bit_utils_test.cc
@@ -350,6 +350,8 @@
static_assert(MaskLeastSignificant(2) == 0b11, "TestMaskLeastSignificant#3");
static_assert(MaskLeastSignificant<uint8_t>(8) == 0xFF, "TestMaskLeastSignificant#4");
static_assert(MaskLeastSignificant<int8_t>(8) == 0xFF, "TestMaskLeastSignificant#5");
+static_assert(MaskLeastSignificant<uint64_t>(63) == (std::numeric_limits<uint64_t>::max() >> 1u),
+ "TestMaskLeastSignificant#6");
static_assert(BitFieldClear(0xFF, /*lsb*/0, /*width*/0) == 0xFF, "TestBitFieldClear#1");
static_assert(BitFieldClear(std::numeric_limits<uint32_t>::max(), /*lsb*/0, /*width*/32) == 0x0,