Mögliches undefiniertes Verhalten bei der Implementierung des primitiven static_vector

12

tl; dr: Ich denke, mein static_vector hat ein undefiniertes Verhalten, aber ich kann es nicht finden.

Dieses Problem tritt unter Microsoft Visual C ++ 17 auf. Ich habe diese einfache und unvollendete static_vector-Implementierung, dh einen Vektor mit einer festen Kapazität, der gestapelt werden kann. Dies ist ein C ++ 17-Programm, das std :: align_storage und std :: launder verwendet. Ich habe versucht, es unten auf die Teile zu reduzieren, die meiner Meinung nach für das Problem relevant sind:

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

Dies schien für eine Weile gut zu funktionieren. Dann habe ich irgendwann etwas sehr Ähnliches gemacht - der eigentliche Code ist länger, aber das bringt es auf den Punkt:

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

Mit anderen Worten, wir kopieren zuerst 8-Byte-Foobar-Strukturen in einen static_vector auf dem Stapel, dann erstellen wir ein std :: pair eines static_vector aus 8-Byte-Strukturen als erstes Element und ein uint64_t als zweites. Ich kann überprüfen, ob valuesOnTheStack unmittelbar vor der Erstellung des Paares die richtigen Werte enthält. Und ... dies führt zu Fehlern bei der Optimierung, die im Kopierkonstruktor von static_vector (der in die aufrufende Funktion integriert wurde) beim Erstellen des Paares aktiviert ist.

Kurz gesagt, ich habe die Demontage inspiziert. Hier wird es etwas komisch; Der generierte Asm um den Inline-Kopierkonstruktor wird unten gezeigt. Beachten Sie, dass dies aus dem tatsächlichen Code stammt, nicht aus dem obigen Beispiel, das ziemlich nahe beieinander liegt, aber einige weitere Dinge über der Paarkonstruktion enthält:

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)  

Okay, zuerst haben wir zwei mov-Anweisungen, die das Zählelement von der Quelle zum Ziel kopieren. So weit, ist es gut. edx wird auf Null gesetzt, da es sich um die Schleifenvariable handelt. Wir haben dann eine schnelle Überprüfung, ob die Anzahl Null ist; Da es nicht Null ist, fahren wir mit der for-Schleife fort, in der wir die 8-Byte-Struktur mit zwei 32-Bit-Mov-Operationen zuerst von Speicher zu Register und dann von Register zu Speicher kopieren. Aber es gibt etwas faul - wo wir erwarten würden, dass ein Mov von etwas wie [ebp + edx * 8 +] aus dem Quellobjekt liest, gibt es stattdessen nur ... [ecx]. Das klingt nicht richtig. Was ist der Wert von ecx?

Es stellt sich heraus, dass ecx nur eine Mülladresse enthält, die gleiche, auf der wir Fehler machen. Woher hat es diesen Wert? Hier ist der Asm direkt oben:

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

Dies sieht aus wie ein normaler alter cdecl-Funktionsaufruf. In der Tat hat die Funktion einen Aufruf einer externen C-Funktion direkt darüber. Beachten Sie jedoch, was passiert: ecx wird als temporäres Register verwendet, um Argumente auf den Stapel zu übertragen, die Funktion wird aufgerufen und ... dann wird ecx nie wieder berührt, bis es unten fälschlicherweise zum Lesen aus der Quelle static_vector verwendet wird.

In der Praxis wird der Inhalt von ecx durch die hier aufgerufene Funktion überschrieben, was natürlich erlaubt ist. Aber selbst wenn dies nicht der Fall wäre, würde ecx auf keinen Fall jemals eine Adresse für das Richtige enthalten - bestenfalls würde es auf ein lokales Stack-Mitglied verweisen, das nicht der static_vector ist. Es scheint, als hätte der Compiler eine falsche Assembly ausgegeben. Diese Funktion könnte niemals die richtige Ausgabe erzeugen.

Dort bin ich jetzt. Seltsame Montage, wenn Optimierungen beim Herumspielen in std :: launder land aktiviert sind, riecht für mich nach undefiniertem Verhalten. Aber ich kann nicht sehen, woher das kommen könnte. Als ergänzende, aber nur geringfügig nützliche Information erzeugt das Klirren mit den richtigen Flags eine ähnliche Zusammenstellung wie diese, außer dass ebp + edx anstelle von ecx zum Lesen von Werten korrekt verwendet wird.

Pjohansson
quelle
Nur ein flüchtiger Blick, aber warum rufen Sie clear()die Ressourcen auf, auf denen Sie angerufen haben std::move?
Bathseba
Ich sehe nicht, wie das relevant ist. Sicher, es wäre auch legal, den static_vector mit der gleichen Größe, aber einer Reihe von ausgezogenen Objekten zu belassen. Der Inhalt wird zerstört, wenn der Destruktor static_vector trotzdem ausgeführt wird. Aber ich ziehe es vor, den ausgezogenen Vektor mit einer Größe von Null zu belassen.
Pjohansson
Summen. Dann jenseits meiner Gehaltsstufe. Haben Sie eine positive Bewertung, da dies gut gefragt ist und Aufmerksamkeit erregen könnte.
Bathseba
Sie können keinen Absturz mit Ihrem Code reproduzieren (nicht geholfen, weil er aufgrund fehlenden Codes nicht kompiliert wurde is_iterator). Bitte geben Sie ein minimal reproduzierbares Beispiel an
Alan Birtles
1
Übrigens denke ich, dass viel Code hier irrelevant ist. Ich meine, Sie rufen hier nirgendwo einen Zuweisungsoperator an, damit er aus dem Beispiel entfernt werden kann
Bartop

Antworten:

6

Ich denke du hast einen Compiler Bug. Das Hinzufügen __declspec( noinline )zu operator[]scheint den Absturz zu beheben:

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

Sie können versuchen, den Fehler an Microsoft zu melden, aber der Fehler scheint bereits in Visual Studio 2019 behoben zu sein.

Das Entfernen std::launderscheint auch den Absturz zu beheben:

constexpr const_reference operator[]( size_type n ) const { return *reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ); }
Alan Birtles
quelle
Mir gehen auch andere Erklärungen aus. So sehr das angesichts unserer aktuellen Situation auch scheiße ist, es scheint plausibel, dass dies der Fall ist, also werde ich dies als akzeptierte Antwort markieren.
Pjohansson
Das Entfernen von Wäscherei behebt das Problem? Das Entfernen von Wäsche wäre ausdrücklich ein undefiniertes Verhalten! Seltsam.
Pjohansson
@pjohansson std::launderist / war bekanntermaßen von einigen Implementierungen falsch implementiert. Möglicherweise basiert Ihre Version von MSVS auf dieser falschen Implementierung. Ich habe leider keine Quellen.
Fureeish