Возможное неопределенное поведение в примитивной реализации static_vector

12

tl; dr: я думаю, что мой static_vector имеет неопределенное поведение, но я не могу его найти.

Эта проблема на Microsoft Visual C ++ 17. У меня есть эта простая и незавершенная реализация static_vector, то есть вектор с фиксированной емкостью, который может быть выделен в стеке. Это программа C ++ 17, использующая std :: align_storage и std :: launder. Я попытался свести это ниже к частям, которые я думаю, имеют отношение к проблеме:

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

Некоторое время это работало нормально. Затем, в какой-то момент, я делал нечто очень похожее на это - реальный код длиннее, но это доходит до сути:

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

Другими словами, мы сначала копируем 8-байтовые структуры Foobar в static_vector в стеке, затем мы создаем std :: pair static_vector из 8-байтовых структур в качестве первого члена и uint64_t в качестве второго. Я могу проверить, что valuesOnTheStack содержит правильные значения непосредственно перед созданием пары. И ... это происходит с оптимизацией, включенной в конструкторе копирования static_vector (который был встроен в вызывающую функцию) при создании пары.

Короче говоря, я осмотрел разборки. Это где вещи становятся немного странными; сгенерированный asm вокруг встроенного конструктора копирования показан ниже - обратите внимание, что это из фактического кода, а не из приведенного выше примера, который довольно близок, но имеет еще кое-что над конструкцией пары:

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)  

Итак, сначала у нас есть две инструкции mov, копирующие элемент count из источника в место назначения; Все идет нормально. edx обнуляется, потому что это переменная цикла. Затем у нас есть быстрая проверка, если счет равен нулю; это не ноль, поэтому мы переходим к циклу for, где мы копируем 8-байтовую структуру, используя две 32-битные операции mov сначала из памяти в регистр, затем из регистра в память. Но есть кое-что подозрительное - где мы ожидаем, что mov из чего-то вроде [ebp + edx * 8 +] будет читать из исходного объекта, вместо этого есть просто ... [ecx]. Это не звучит правильно. Какова ценность ecx?

Оказывается, ecx просто содержит адрес мусора, тот же, на котором мы segfaulting. Откуда он взял это значение? Вот асм сразу выше:

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

Это похоже на обычный старый вызов функции cdecl. Действительно, у функции есть вызов внешней функции C чуть выше. Но обратите внимание на то, что происходит: ecx используется как временный регистр для передачи аргументов в стек, функция вызывается, и ... затем ecx никогда не затрагивается до тех пор, пока он не будет ошибочно использован ниже для чтения из источника static_vector.

На практике содержимое ecx перезаписывается вызываемой здесь функцией, что, конечно, разрешено делать. Но даже если это не так, ecx никогда не будет содержать здесь адрес для правильной вещи - в лучшем случае это будет указывать на локальный элемент стека, который не является static_vector. Кажется, что компилятор испустил некоторую поддельную сборку. Эта функция никогда не выдаст правильный вывод.

Так вот где я сейчас. Странная сборка, когда оптимизация включена во время игры в std ::андер, пахнет для меня как неопределенное поведение. Но я не вижу, откуда это может исходить. В качестве дополнительной, но мало полезной информации, clang с правильными флагами производит аналогичную сборку, за исключением того, что он правильно использует ebp + edx вместо ecx для чтения значений.

pjohansson
источник
Только поверхностный взгляд, но почему вы обращаетесь clear()к ресурсам, на которые звонили std::move?
Вирсавия
Я не понимаю, насколько это актуально. Конечно, было бы также законно оставить static_vector с тем же размером, но с кучей перемещенных объектов. Содержимое будет уничтожено, когда деструктор static_vector будет запущен в любом случае. Но я предпочитаю оставлять вытесненный вектор размером с ноль.
pjohansson
Hum. Помимо моей зарплаты тогда. Имейте в виду, что это хорошо спросили, и может привлечь внимание.
Вирсавия
Невозможно воспроизвести сбой в вашем коде (не помогает, если он не компилируется из-за отсутствия is_iterator), приведите минимальный воспроизводимый пример
Алан Биртлз,
1
Кстати, я думаю, что много кода здесь не имеет значения. Я имею в виду, вы не вызываете оператор присваивания где - нибудь здесь , так что он может быть удален из примера
bartop

Ответы:

6

Я думаю, что у вас есть ошибка компилятора. Добавление __declspec( noinline )к, operator[]кажется, исправить сбой:

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

Вы можете попробовать сообщить об ошибке в Microsoft, но, похоже, ошибка уже исправлена ​​в Visual Studio 2019.

Удаление std::launderтакже, кажется, исправляет сбой:

constexpr const_reference operator[]( size_type n ) const { return *reinterpret_cast<const T*>( std::addressof( storage[ n ] ) ); }
Алан Биртлз
источник
У меня тоже мало объяснений. Столько, сколько это отстой, учитывая нашу текущую ситуацию, кажется правдоподобным, что это то, что происходит, поэтому я отмечу это как принятый ответ.
pjohansson
Удаление отмыва исправляет это? Удаление отмывания явно было бы неопределенным поведением! Странный.
pjohansson
Известно, что @pjohansson std::launderнеправильно реализован некоторыми реализациями. Возможно, ваша версия MSVS основана на этой неправильной реализации. У меня нет источников, к сожалению.
Fureeish