Unaligned access with potential undefined behavior (with fix included)

[germandiagogomez]

Hello there.

I am using aseprite tga to convert files to tga for RmlUi consumption. When passing UB sanitizer I spot a potential bug (that probably does not manifest in x86, since unaligned access, even if slow, is allowed, but could in ARM). Since I was experimenting random times at which my file would not be saved, I suspect it could be related even in x86?

Anyway, the relevant line is here:

tga/tga.h

Line 195 in cd3420c

T value = *((T*)m_ptr);

This access is potentially misaligned. This code corrects it via a branchless implementation.

Since accessing *((T*)m_ptr) is potentially unaligned access and hence, illegal, a trick comes into play: every time we access the first result, the second result will be calculated aligned (with an incorrect value) but since the second result is not needed in that case, no undefined behavior access is done at all in that case. When the second var is accessed, then the access is aligned and legal (and correct).

template<typename T>
    T getPixel() {
      T value = *((T*)m_ptr);
      advance();
      return value;
    }

With this:

    template<typename T>
    T getPixel() {
      T value = alignedAccess<T>();
      advance();
      return value;
    }

    template<typename T>
    T alignedAccess() const {
      static_assert(std::is_unsigned<T>::value, "T is not unsigned");
      static_assert(alignof(std::uint8_t) == 1, "std::uint8_t does not have 1 byte alignment");
      constexpr auto t_alignment = alignof(T);
      auto alignment_remainder = reinterpret_cast<std::uintptr_t>(m_ptr) % t_alignment;
      T values[2] = {
          [&] {
            std::uint8_t buf[sizeof(T)];
            std::copy(m_ptr, m_ptr + sizeof(T), &buf[0]);
            return *reinterpret_cast<T*>(&buf[0]);
          }(),
          *(reinterpret_cast<T*>(m_ptr - alignment_remainder)) 
      };
      return values[static_cast<bool>(alignment_remainder == 0)];
    }

This would remove the unaligned access. Of course, equivalent and/or better optimized version can probably be done, but this one seems reasonable to me.

[dacap]

Hi @germandiagogomez, from what you've commented, it looks like the misalignment can happen only if the memory assigned to the Image::pixels member is misaligned and the Image::rowstride contains an invalid/unaligned value. Did you try allocating aligned memory? (_aligned_malloc / aligned_alloc)