Comportement indéfini possible dans l'implémentation primitive de static_vector

12

tl; dr: Je pense que mon static_vector a un comportement indéfini, mais je ne le trouve pas.

Ce problème est sur Microsoft Visual C ++ 17. J'ai cette implémentation static_vector simple et inachevée, c'est-à-dire un vecteur avec une capacité fixe qui peut être allouée en pile. Il s'agit d'un programme C ++ 17, utilisant std :: aligné_storage et std :: launder. J'ai essayé de résumer ci-dessous les parties qui, selon moi, sont pertinentes pour le problème:

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

Cela semblait bien fonctionner pendant un certain temps. Puis, à un moment donné, je faisais quelque chose de très similaire à cela - le code réel est plus long, mais cela va à l'essentiel:

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 d'autres termes, nous copions d'abord les structures Foobar de 8 octets dans un static_vector sur la pile, puis nous créons une paire std :: d'un static_vector de structures de 8 octets en tant que premier membre, et un uint64_t en tant que second. Je peux vérifier que valuesOnTheStack contient les bonnes valeurs immédiatement avant la construction de la paire. Et ... ce segfaults avec optimisation activée dans le constructeur de copie de static_vector (qui a été inséré dans la fonction appelante) lors de la construction de la paire.

Pour faire court, j'ai inspecté le démontage. C'est là que les choses deviennent un peu bizarres; l'asm généré autour du constructeur de copie en ligne est illustré ci-dessous - notez que cela vient du code réel, pas de l'exemple ci-dessus, qui est assez proche mais a plus de choses au-dessus de la construction de la paire:

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)  

D'accord, nous avons donc d'abord deux instructions mov copiant le membre count de la source vers la destination; jusqu'ici tout va bien. edx est mis à zéro car c'est la variable de boucle. Nous vérifions ensuite rapidement si le nombre est nul; il n'est pas nul, nous passons donc à la boucle for où nous copions la structure de 8 octets en utilisant deux opérations mov 32 bits d'abord de la mémoire au registre, puis du registre à la mémoire. Mais il y a quelque chose de louche - où nous nous attendrions à ce qu'un mov de quelque chose comme [ebp + edx * 8 +] soit lu à partir de l'objet source, il y a à la place juste ... [ecx]. Cela ne semble pas juste. Quelle est la valeur d'ecx?

Il s'avère que ecx contient juste une adresse poubelle, la même que celle sur laquelle nous nous trompons. D'où a-t-il obtenu cette valeur? Voici l'asm immédiatement ci-dessus:

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

Cela ressemble à un ancien appel de fonction cdecl classique. En effet, la fonction a un appel à une fonction C externe juste au-dessus. Mais notez ce qui se passe: ecx est utilisé comme un registre temporaire pour pousser les arguments sur la pile, la fonction est invoquée, et ... alors ecx n'est plus touché jusqu'à ce qu'il soit utilisé à tort ci-dessous pour lire à partir du source static_vector.

En pratique, le contenu d'ecx est écrasé par la fonction appelée ici, ce qu'il est bien sûr autorisé à faire. Mais même si ce n'est pas le cas, il n'y a aucun moyen que ecx contienne jamais une adresse à la bonne chose ici - au mieux, il pointerait vers un membre de pile local qui n'est pas le static_vector. Il semble que le compilateur ait émis un faux assemblage. Cette fonction n'a jamais pu produire la sortie correcte.

Voilà où j'en suis maintenant. L'assemblage étrange lorsque les optimisations sont activées tout en jouant dans std :: launder land me sent comme un comportement indéfini. Mais je ne vois pas d'où cela pourrait venir. En tant qu'informations supplémentaires mais légèrement utiles, clang avec les indicateurs de droite produit un assemblage similaire à celui-ci, sauf qu'il utilise correctement ebp + edx au lieu d'ecx pour lire les valeurs.

pjohansson
la source
Un regard superficiel mais pourquoi faites-vous appel clear()aux ressources auxquelles vous avez fait appel std::move?
Bathsheba
Je ne vois pas en quoi c'est pertinent. Bien sûr, il serait également légal de laisser le static_vector avec la même taille mais un tas d'objets déplacés. Le contenu sera détruit quand le destructeur static_vector s'exécutera de toute façon. Mais je préfère laisser le vecteur déplacé avec une taille nulle.
pjohansson
Fredonner. Au-delà de mon salaire, alors. Ayez un vote positif car cela est bien demandé et pourrait attirer l'attention.
Bethsabée
Impossible de reproduire un plantage avec votre code (cela ne facilite pas la compilation en raison du manque de is_iterator), veuillez fournir un exemple reproductible minimal
Alan Birtles
1
btw, je pense que beaucoup de code n'est pas pertinent ici. Je veux dire, vous n'appelez pas d'opérateur d'affectation n'importe où ici, donc il pourrait être supprimé de l'exemple
Bart

Réponses:

6

Je pense que vous avez un bug de compilation. L'ajout __declspec( noinline )de operator[]semble résoudre le crash:

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

Vous pouvez essayer de signaler le bogue à Microsoft, mais le bogue semble déjà être corrigé dans Visual Studio 2019.

La suppression std::laundersemble également résoudre le crash:

constexpr const_reference operator[]( size_type n ) const { return *reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ); }
Alan Birtles
la source
Je manque également d'autres explications. Autant que ça craint étant donné notre situation actuelle, il semble plausible que c'est ce qui se passe, alors je vais marquer cela comme la réponse acceptée.
pjohansson
Supprimer le blanchiment le résout-il? Supprimer le blanchiment serait explicitement un comportement indéfini! Étrange.
pjohansson
@pjohansson std::launderest / était connu pour être incorrectement implémenté par certaines implémentations. Peut-être que votre version de MSVS est basée sur cette implémentation incorrecte. Je n'ai malheureusement pas les sources.
Fureeish