[NFCI][SYCL] Eliminate UB in ANSI alias violation accessing ByteArray's (#7023)

As part of it provide a nicer (IMO) interface to it. Also, asserts are
now moved into the underlying utility functions making it easier to read
the core logic up the call stack.

Author: aelovikov-intel

sycl/source/detail/device_binary_image.hpp

@@ -10,6 +10,7 @@
 #include <sycl/detail/os_util.hpp>
 #include <sycl/detail/pi.hpp>
 
+#include <cstring>
 #include <memory>
 
 namespace sycl {
@@ -27,9 +28,34 @@ class ByteArray {
   ConstIterator begin() const { return Ptr; }
   ConstIterator end() const { return Ptr + Size; }
 
+  template <typename... Ts> auto consume() {
+    if constexpr (sizeof...(Ts) == 1)
+      return consumeOneElem<Ts...>();
+    else
+      return std::tuple{consumeOneElem<Ts>()...};
+  }
+
+  void dropBytes(std::size_t Bytes) {
+    assert(Bytes <= Size && "Not enough bytes left!");
+    Ptr += Bytes;
+    Size -= Bytes;
+  }
+
+  template <typename T> void drop() { return dropBytes(sizeof(T)); }
+
+  bool empty() const { return Size == 0; }
+
 private:
+  template <typename T> T consumeOneElem() {
+    assert(sizeof(T) <= Size && "Out of bounds!");
+    T Val;
+    std::memcpy(&Val, Ptr, sizeof(T));
+    drop<T>();
+    return Val;
+  }
+
   const std::uint8_t *Ptr;
-  const std::size_t Size;
+  std::size_t Size;
 };
 
 // C++ wrapper over the _pi_device_binary_property_struct structure.

sycl/source/detail/device_image_impl.hpp

@@ -260,39 +260,33 @@ class device_image_impl {
         const char *SCName = (*SCIt)->Name;
 
         ByteArray Descriptors = DeviceBinaryProperty(*SCIt).asByteArray();
-        assert(Descriptors.size() > 8 && "Unexpected property size");
+        // First 8 bytes are consumed by the size of the property.
+        Descriptors.dropBytes(8);
 
         // Expected layout is vector of 3-component tuples (flattened into a
         // vector of scalars), where each tuple consists of: ID of a scalar spec
         // constant, (which might be a member of the composite); offset, which
         // is used to calculate location of scalar member within the composite
-        // or zero for scalar spec constants; size of a spec constant
-        constexpr size_t NumElements = 3;
-        assert(((Descriptors.size() - 8) / sizeof(std::uint32_t)) %
-                       NumElements ==
-                   0 &&
-               "unexpected layout of composite spec const descriptors");
-        auto *It = reinterpret_cast<const std::uint32_t *>(&Descriptors[8]);
-        auto *End = reinterpret_cast<const std::uint32_t *>(&Descriptors[0] +
-                                                            Descriptors.size());
+        // or zero for scalar spec constants; size of a spec constant.
         unsigned LocalOffset = 0;
-        while (It != End) {
-          // Make sure that alignment is correct in blob.
-          const unsigned OffsetFromLast = /*Offset*/ It[1] - LocalOffset;
+        while (!Descriptors.empty()) {
+          auto [Id, CompositeOffset, Size] =
+              Descriptors.consume<uint32_t, uint32_t, uint32_t>();
+
+          // Make sure that alignment is correct in the blob.
+          const unsigned OffsetFromLast = CompositeOffset - LocalOffset;
           BlobOffset += OffsetFromLast;
           // Composites may have a special padding element at the end which
           // should not have a descriptor. These padding elements all have max
           // ID value.
-          if (It[0] != std::numeric_limits<std::uint32_t>::max()) {
+          if (Id != std::numeric_limits<std::uint32_t>::max()) {
             // The map is not locked here because updateSpecConstSymMap() is
             // only supposed to be called from c'tor.
             MSpecConstSymMap[std::string{SCName}].push_back(
-                SpecConstDescT{/*ID*/ It[0], /*CompositeOffset*/ It[1],
-                               /*Size*/ It[2], BlobOffset});
+                SpecConstDescT{Id, CompositeOffset, Size, BlobOffset});
           }
-          LocalOffset += OffsetFromLast + /*Size*/ It[2];
-          BlobOffset += /*Size*/ It[2];
-          It += NumElements;
+          LocalOffset += OffsetFromLast + Size;
+          BlobOffset += Size;
         }
       }
       MSpecConstsBlob.resize(BlobOffset);

sycl/source/detail/program_impl.cpp

@@ -541,23 +541,21 @@ void program_impl::flush_spec_constants(const RTDeviceBinaryImage &Img,
     const spec_constant_impl &SC = SCEntry->second;
     assert(SC.isSet() && "uninitialized spec constant");
     ByteArray Descriptors = DeviceBinaryProperty(*SCIt).asByteArray();
-    // First 8 bytes are consumed by size of the property
-    assert(Descriptors.size() > 8 && "Unexpected property size");
-    // Expected layout is vector of 3-component tuples (flattened into a vector
-    // of scalars), where each tuple consists of: ID of a scalar spec constant,
-    // (which might be a member of the composite); offset, which is used to
-    // calculate location of scalar member within the composite or zero for
-    // scalar spec constants; size of a spec constant
-    assert(((Descriptors.size() - 8) / sizeof(std::uint32_t)) % 3 == 0 &&
-           "unexpected layout of composite spec const descriptors");
-    auto *It = reinterpret_cast<const std::uint32_t *>(&Descriptors[8]);
-    auto *End = reinterpret_cast<const std::uint32_t *>(&Descriptors[0] +
-                                                        Descriptors.size());
-    while (It != End) {
+
+    // First 8 bytes are consumed by the size of the property.
+    Descriptors.dropBytes(8);
+
+    // Expected layout is vector of 3-component tuples (flattened into a
+    // vector of scalars), where each tuple consists of: ID of a scalar spec
+    // constant, (which might be a member of the composite); offset, which
+    // is used to calculate location of scalar member within the composite
+    // or zero for scalar spec constants; size of a spec constant.
+    while (!Descriptors.empty()) {
+      auto [Id, Offset, Size] =
+          Descriptors.consume<uint32_t, uint32_t, uint32_t>();
+
       Ctx->getPlugin().call<PiApiKind::piextProgramSetSpecializationConstant>(
-          NativePrg, /* ID */ It[0], /* Size */ It[2],
-          SC.getValuePtr() + /* Offset */ It[1]);
-      It += 3;
+          NativePrg, Id, Size, SC.getValuePtr() + Offset);
     }
   }
 }

sycl/source/detail/program_manager/program_manager.cpp

@@ -1277,11 +1277,10 @@ void ProgramManager::addImages(pi_device_binaries DeviceBinary) {
           // * 4 bytes - Size of the underlying type in the device_global.
           // * 4 bytes - 0 if device_global has device_image_scope and any value
           //             otherwise.
-          assert(DeviceGlobalInfo.size() == 16 && "Unexpected property size");
-          const std::uint32_t TypeSize =
-              *reinterpret_cast<const std::uint32_t *>(&DeviceGlobalInfo[8]);
-          const std::uint32_t DeviceImageScopeDecorated =
-              *reinterpret_cast<const std::uint32_t *>(&DeviceGlobalInfo[12]);
+          DeviceGlobalInfo.dropBytes(8);
+          auto [TypeSize, DeviceImageScopeDecorated] =
+              DeviceGlobalInfo.consume<std::uint32_t, std::uint32_t>();
+          assert(DeviceGlobalInfo.empty() && "Extra data left!");
 
           auto ExistingDeviceGlobal = m_DeviceGlobals.find(DeviceGlobal->Name);
           if (ExistingDeviceGlobal != m_DeviceGlobals.end()) {