Bug 200438 – Undefined behavior in Vector::append

• Component: Web Template Framework
• Status: NEW

[Michael Catanzaro]

I believe Coverity has discovered some undefined behavior here:

template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity>
template<typename U>
ALWAYS_INLINE void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::append(const U* data, size_t dataSize)
{
    size_t newSize = m_size + dataSize;
    if (newSize > capacity()) {
        data = expandCapacity(newSize, data);
        ASSERT(begin());
    }
    if (newSize < m_size)
        CRASH();
    asanBufferSizeWillChangeTo(newSize);
    T* dest = end();
    VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, std::addressof(data[dataSize]), dest);
    m_size = newSize;
}

The full report here is a bit complicated, but I believe the crux of the problem is std::addressof(data[dataSize]). data[dataSize] is, of course, one element past the end of the array. Per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91357, I understand that's already UB, and applying std::addressof after UB occurs doesn't make it OK (even though it apparently works with today's compilers).

I think it could be fixed by changing VectorCopy::uninitializedCopy to take a count indicating how much to copy and pass that directly to memcpy, rather than accepting the end pointer const T* srcEnd.

[Michael Catanzaro]

Although the example in that GCC bug is a bit different, actually not at all comparable, because there the question is whether operator[] can be used in this way on a std::vector, whereas here in WebKit we are working with a builtin array.

Still, although I'm not a standards expert, I suspect it's not OK.

[Alexey Proskuryakov]

A pointer pointing to the element after the last one is fine, but dereferencing it is not. 

Now, I’m not sure what std::addressof gives us here that (data + dataSize) would not. What kind of overload are we protecting against?