Posible comportamiento indefinido en la implementación primitiva static_vector

12

tl; dr: Creo que mi static_vector tiene un comportamiento indefinido, pero no puedo encontrarlo.

Este problema está en Microsoft Visual C ++ 17. Tengo esta implementación static_vector simple e inacabada, es decir, un vector con una capacidad fija que se puede asignar en pila. Este es un programa de C ++ 17, que usa std :: lined_storage y std :: launder. He tratado de reducirlo a las partes que creo que son relevantes para el problema:

template <typename T, size_t NCapacity>
class static_vector
{
public:
    typedef typename std::remove_cv<T>::type value_type;
    typedef size_t size_type;
    typedef T* pointer;
    typedef const T* const_pointer;
    typedef T& reference;
    typedef const T& const_reference;

    static_vector() noexcept
        : count()
    {
    }

    ~static_vector()
    {
        clear();
    }

    template <typename TIterator, typename = std::enable_if_t<
        is_iterator<TIterator>::value
    >>
    static_vector(TIterator in_begin, const TIterator in_end)
        : count()
    {
        for (; in_begin != in_end; ++in_begin)
        {
            push_back(*in_begin);
        }
    }

    static_vector(const static_vector& in_copy)
        : count(in_copy.count)
    {
        for (size_type i = 0; i < count; ++i)
        {
            new(std::addressof(storage[i])) value_type(in_copy[i]);
        }
    }

    static_vector& operator=(const static_vector& in_copy)
    {
        // destruct existing contents
        clear();

        count = in_copy.count;
        for (size_type i = 0; i < count; ++i)
        {
            new(std::addressof(storage[i])) value_type(in_copy[i]);
        }

        return *this;
    }

    static_vector(static_vector&& in_move)
        : count(in_move.count)
    {
        for (size_type i = 0; i < count; ++i)
        {
            new(std::addressof(storage[i])) value_type(move(in_move[i]));
        }
        in_move.clear();
    }

    static_vector& operator=(static_vector&& in_move)
    {
        // destruct existing contents
        clear();

        count = in_move.count;
        for (size_type i = 0; i < count; ++i)
        {
            new(std::addressof(storage[i])) value_type(move(in_move[i]));
        }

        in_move.clear();

        return *this;
    }

    constexpr pointer data() noexcept { return std::launder(reinterpret_cast<T*>(std::addressof(storage[0]))); }
    constexpr const_pointer data() const noexcept { return std::launder(reinterpret_cast<const T*>(std::addressof(storage[0]))); }
    constexpr size_type size() const noexcept { return count; }
    static constexpr size_type capacity() { return NCapacity; }
    constexpr bool empty() const noexcept { return count == 0; }

    constexpr reference operator[](size_type n) { return *std::launder(reinterpret_cast<T*>(std::addressof(storage[n]))); }
    constexpr const_reference operator[](size_type n) const { return *std::launder(reinterpret_cast<const T*>(std::addressof(storage[n]))); }

    void push_back(const value_type& in_value)
    {
        if (count >= capacity()) throw std::out_of_range("exceeded capacity of static_vector");
        new(std::addressof(storage[count])) value_type(in_value);
        count++;
    }

    void push_back(value_type&& in_moveValue)
    {
        if (count >= capacity()) throw std::out_of_range("exceeded capacity of static_vector");
        new(std::addressof(storage[count])) value_type(move(in_moveValue));
        count++;
    }

    template <typename... Arg>
    void emplace_back(Arg&&... in_args)
    {
        if (count >= capacity()) throw std::out_of_range("exceeded capacity of static_vector");
        new(std::addressof(storage[count])) value_type(forward<Arg>(in_args)...);
        count++;
    }

    void pop_back()
    {
        if (count == 0) throw std::out_of_range("popped empty static_vector");
        std::destroy_at(std::addressof((*this)[count - 1]));
        count--;
    }

    void resize(size_type in_newSize)
    {
        if (in_newSize > capacity()) throw std::out_of_range("exceeded capacity of static_vector");

        if (in_newSize < count)
        {
            for (size_type i = in_newSize; i < count; ++i)
            {
                std::destroy_at(std::addressof((*this)[i]));
            }
            count = in_newSize;
        }
        else if (in_newSize > count)
        {
            for (size_type i = count; i < in_newSize; ++i)
            {
                new(std::addressof(storage[i])) value_type();
            }
            count = in_newSize;
        }
    }

    void clear()
    {
        resize(0);
    }

private:
    typename std::aligned_storage<sizeof(T), alignof(T)>::type storage[NCapacity];
    size_type count;
};

Esto pareció funcionar bien por un tiempo. Luego, en un momento, estaba haciendo algo muy similar a esto: el código real es más largo, pero esto es esencial:

struct Foobar
{
    uint32_t Member1;
    uint16_t Member2;
    uint8_t Member3;
    uint8_t Member4;
}

void Bazbar(const std::vector<Foobar>& in_source)
{
    static_vector<Foobar, 8> valuesOnTheStack { in_source.begin(), in_source.end() };

    auto x = std::pair<static_vector<Foobar, 8>, uint64_t> { valuesOnTheStack, 0 };
}

En otras palabras, primero copiamos las estructuras Foobar de 8 bytes en un static_vector en la pila, luego hacemos un std :: par de static_vector de estructuras de 8 bytes como el primer miembro, y un uint64_t como el segundo. Puedo verificar que valuesOnTheStack contiene los valores correctos inmediatamente antes de construir el par. Y ... esto falla con la optimización habilitada dentro del constructor de copia static_vector (que se ha incluido en la función de llamada) al construir el par.

Larga historia corta, inspeccioné el desmontaje. Aquí donde las cosas se ponen un poco raras; el asm generado alrededor del constructor de copia en línea se muestra a continuación: tenga en cuenta que esto es del código real, no del ejemplo anterior, que está bastante cerca pero tiene algunas cosas más sobre la construcción del par:

00621E45  mov         eax,dword ptr [ebp-20h]  
00621E48  xor         edx,edx  
00621E4A  mov         dword ptr [ebp-70h],eax  
00621E4D  test        eax,eax  
00621E4F  je          <this function>+29Ah (0621E6Ah)  
00621E51  mov         eax,dword ptr [ecx]  
00621E53  mov         dword ptr [ebp+edx*8-0B0h],eax  
00621E5A  mov         eax,dword ptr [ecx+4]  
00621E5D  mov         dword ptr [ebp+edx*8-0ACh],eax  
00621E64  inc         edx  
00621E65  cmp         edx,dword ptr [ebp-70h]  
00621E68  jb          <this function>+281h (0621E51h)  

Bien, primero tenemos dos instrucciones mov que copian el miembro de conteo desde el origen al destino; Hasta aquí todo bien. edx se pone a cero porque es la variable del bucle. Luego tenemos una comprobación rápida si el recuento es cero; no es cero, así que procedemos al ciclo for donde copiamos la estructura de 8 bytes usando dos operaciones mov de 32 bits primero de la memoria para registrar, luego de registrar a la memoria. Pero hay algo sospechoso, en el que esperaríamos que un movimiento de algo como [ebp + edx * 8 +] se lea desde el objeto fuente, en su lugar solo hay ... [ecx]. Eso no suena bien. ¿Cuál es el valor de ecx?

Resulta que ecx solo contiene una dirección basura, la misma en la que estamos segfaulándose. ¿De dónde obtuvo este valor? Aquí está el asm inmediatamente arriba:

00621E1C  mov         eax,dword ptr [this]  
00621E22  push        ecx  
00621E23  push        0  
00621E25  lea         ecx,[<unrelated local variable on the stack, not the static_vector>]  
00621E2B  mov         eax,dword ptr [eax]  
00621E2D  push        ecx  
00621E2E  push        dword ptr [eax+4]  
00621E31  call        dword ptr [<external function>@16 (06AD6A0h)]  

Esto se parece a una llamada de función cdecl antigua normal. De hecho, la función tiene una llamada a una función C externa justo arriba. Pero tenga en cuenta lo que está sucediendo: ecx se está utilizando como un registro temporal para insertar argumentos en la pila, se invoca la función y ... luego, ecx nunca se vuelve a tocar hasta que se usa erróneamente a continuación para leer desde la fuente static_vector.

En la práctica, los contenidos de ecx se sobrescriben con la función llamada aquí, que por supuesto está permitido hacer. Pero incluso si no fuera así, no hay forma de que ecx contenga una dirección a lo correcto aquí; en el mejor de los casos, apuntaría a un miembro de la pila local que no sea el static_vector. Parece que el compilador ha emitido algún ensamblaje falso. Esta función nunca podría producir la salida correcta.

Entonces ahí es donde estoy ahora. El ensamblaje extraño cuando las optimizaciones están habilitadas mientras se juega en std :: launder huele a tierra como un comportamiento indefinido. Pero no puedo ver de dónde podría venir eso. Como información complementaria pero marginalmente útil, el sonido metálico con las banderas correctas produce un ensamblaje similar a este, excepto que usa correctamente ebp + edx en lugar de ecx para leer valores.

pjohansson
fuente
Solo una mirada superficial, pero ¿por qué recurres clear()a los recursos a los que has recurrido std::move?
Betsabé
No veo cómo eso es relevante. Claro, también sería legal dejar el static_vector con el mismo tamaño pero con un montón de objetos movidos. El contenido se destruirá cuando el destructor static_vector se ejecute de todos modos. Pero prefiero dejar el vector desplazado con un tamaño de cero.
pjohansson
Tararear. Más allá de mi salario entonces. Obtenga un voto positivo, ya que es muy solicitado y podría atraer la atención.
Betsabé
No se puede reproducir ningún bloqueo con su código (no ayudó porque no se compila debido a la falta de is_iterator) proporcione un ejemplo reproducible mínimo
Alan Birtles
1
por cierto, creo que mucho código es irrelevante aquí. Quiero decir, no llamas al operador de asignación en ningún lugar aquí, por lo que podría eliminarse del ejemplo
bartop

Respuestas:

6

Creo que tienes un error de compilación. Agregar __declspec( noinline )a operator[]parece solucionar el bloqueo:

__declspec( noinline ) constexpr const_reference operator[]( size_type n ) const { return *std::launder( reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ) ); }

Puede intentar informar el error a Microsoft, pero parece que el error ya está solucionado en Visual Studio 2019.

Eliminar std::laundertambién parece solucionar el bloqueo:

constexpr const_reference operator[]( size_type n ) const { return *reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ); }
Alan Birtles
fuente
Me estoy quedando sin otras explicaciones también. Por mucho que eso apesta dada nuestra situación actual, parece plausible que esto sea lo que está sucediendo, así que lo marcaré como la respuesta aceptada.
pjohansson
¿Quitar el lavado lo arregla? ¡Eliminar el lavado de ropa explícitamente sería un comportamiento indefinido! Extraño.
pjohansson
Se sabe que @pjohansson std::launderse implementó incorrectamente en algunas implementaciones. Tal vez su versión de MSVS se base en esa implementación incorrecta. No tengo las fuentes, desafortunadamente.
Fureeish