Possível comportamento indefinido na implementação primitiva static_vector

12

tl; dr: Eu acho que meu static_vector tem um comportamento indefinido, mas não consigo encontrá-lo.

Esse problema está no Microsoft Visual C ++ 17. Eu tenho essa implementação static_vector simples e inacabada, ou seja, um vetor com uma capacidade fixa que pode ser alocada por pilha. Este é um programa C ++ 17, usando std :: align_storage e std :: washder. Tentei resumir abaixo as partes que considero relevantes para o 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;
};

Isso pareceu funcionar bem por um tempo. Então, em um ponto, eu estava fazendo algo muito semelhante a isso - o código real é mais longo, mas isso chega ao essencial:

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 };
}

Em outras palavras, primeiro copiamos estruturas Foobar de 8 bytes em um static_vector na pilha e, em seguida, criamos um std :: pair de um static_vector de estruturas de 8 bytes como o primeiro membro e uint64_t como o segundo. Posso verificar se o valuesOnTheStack contém os valores corretos imediatamente antes da construção do par. E ... esse segfaults com otimização ativada dentro do construtor de cópia do static_vector (que foi incorporado na função de chamada) ao construir o par.

Para encurtar a história, inspecionei a desmontagem. É aqui que as coisas ficam um pouco estranhas; o asm gerado em torno do construtor de cópia embutido é mostrado abaixo - observe que isso é do código real, não da amostra acima, que é bem próxima, mas tem mais algumas coisas acima da construção do 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)  

Ok, primeiro temos duas instruções mov que copiam o membro da contagem da origem para o destino; Por enquanto, tudo bem. edx é zerado porque é a variável de loop. Depois, verificamos rapidamente se a contagem é zero; como não é zero, prosseguimos para o loop for onde copiamos a estrutura de 8 bytes usando duas operações mov de 32 bits, primeiro da memória para registrar e depois do registro para a memória. Mas há algo suspeito - onde esperamos que um mov de algo como [ebp + edx * 8 +] leia do objeto de origem, há apenas ... [ecx]. Isso não parece certo. Qual é o valor de ecx?

Acontece que ecx contém apenas um endereço de lixo, o mesmo em que estamos segmentando. De onde ele tirou esse valor? Aqui está o asm imediatamente acima:

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)]  

Parece uma chamada de função cdecl antiga comum. De fato, a função tem uma chamada para uma função C externa logo acima. Mas observe o que está acontecendo: ecx está sendo usado como um registro temporário para enviar argumentos para a pilha, a função é invocada e ... então ecx nunca é tocado novamente até que seja usado erroneamente abaixo para ler a partir do código de fonte static_vector.

Na prática, o conteúdo de ecx é sobrescrito pela função chamada aqui, o que obviamente é permitido. Mas, mesmo que isso não aconteça, não há como o ecx conter um endereço para a coisa correta aqui - na melhor das hipóteses, apontaria para um membro da pilha local que não é o static_vector. Parece que o compilador emitiu algum conjunto falso. Esta função nunca pode produzir a saída correta.

Então é onde estou agora. Uma montagem estranha quando as otimizações são ativadas enquanto se brinca na terra std :: washder cheira a mim como um comportamento indefinido. Mas não consigo ver de onde isso pode estar vindo. Como informações suplementares, mas marginalmente úteis, o clang com os sinalizadores corretos produz um conjunto semelhante a esse, exceto que ele usa corretamente ebp + edx em vez de ecx para ler valores.

pjohansson
fonte
Apenas uma aparência superficial, mas por que você está recorrendo clear()aos recursos para os quais chamou std::move?
Bathsheba
Não vejo como isso é relevante. Claro, também seria legal deixar o static_vector com o mesmo tamanho, mas vários objetos movidos para fora. O conteúdo será destruído quando o destruidor static_vector for executado de qualquer maneira. Mas eu prefiro deixar o vetor movido para fora com um tamanho de zero.
pjohansson 10/03
Cantarolar. Além da minha nota de pagamento então. Tenha um voto positivo, pois isso é bem solicitado e pode atrair atenção.
Bathsheba
Não é possível reproduzir nenhuma falha com o seu código (não é ajudado por ele não ser compilado devido à falta de is_iterator) por favor, forneça um exemplo reproduzível mínimo
Alan Birtles
11
Aliás, acho que muito código é irrelevante aqui. Quero dizer, você não chama o operador de atribuição em nenhum lugar aqui para que possa ser removido do exemplo
bartop em

Respostas:

6

Eu acho que você tem um bug do compilador. A adição __declspec( noinline )de operator[]parece corrigir a falha:

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

Você pode tentar relatar o bug à Microsoft, mas o bug parece já estar corrigido no Visual Studio 2019.

A remoção std::laundertambém parece corrigir a falha:

constexpr const_reference operator[]( size_type n ) const { return *reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ); }
Alan Birtles
fonte
Também estou ficando sem outras explicações. Por mais que isso seja péssimo, dada a nossa situação atual, parece plausível que seja isso que está acontecendo, então vou marcar isso como a resposta aceita.
pjohansson 10/03
Remover a lavagem corrige isso? Remover a lavagem seria explicitamente um comportamento indefinido! Estranho.
pjohansson 10/03
O @pjohansson std::launderé / foi conhecido por ter sido implementado incorretamente por algumas implementações. Talvez sua versão do MSVS seja baseada nessa implementação incorreta. Infelizmente não tenho as fontes.
Fureeish 10/03