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

4 files changed, 52 insertions, 31 deletions

diff --git a/runtime/base/bit_struct_detail.h b/runtime/base/bit_struct_detail.h
index 9f629c0970..824d7df652 100644
--- a/runtime/base/bit_struct_detail.h
+++ b/runtime/base/bit_struct_detail.h
@@ -56,20 +56,6 @@ namespace detail {
                                 /* 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 @@ namespace detail {
    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 872ada324c..9fc9762054 100644
--- a/runtime/base/bit_struct_test.cc
+++ b/runtime/base/bit_struct_test.cc
@@ -70,17 +70,10 @@ struct CustomBitStruct {
   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 @@ TEST(BitStructs, TwoCustom) {
 
   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, TwoCustom) {
 }
 
 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 @@ TEST(BitStructs, Test1) {
     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 @@ BITSTRUCT_DEFINE_END(MixedSizeBitStruct);
 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 da3c7048b6..5d836545e9 100644
--- a/runtime/base/bit_utils.h
+++ b/runtime/base/bit_utils.h
@@ -388,7 +388,8 @@ inline static constexpr std::make_unsigned_t<T> MaskLeastSignificant(size_t bits
   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 0276d8ded2..3a80600b57 100644
--- a/runtime/base/bit_utils_test.cc
+++ b/runtime/base/bit_utils_test.cc
@@ -350,6 +350,8 @@ static_assert(MaskLeastSignificant(1) == 0b1, "TestMaskLeastSignificant#2");
 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,