новый оператор перезаписывает существующий объект

Question

У меня есть собственный класс FastStack, реализованный в виде массива с фиксированным размером, и индекс для этого массива.

В моем конструкторе копирования я выделяю массив, а затем назначаю каждый объект из массива копии в новыймассив.В объектах стека происходит некоторый пересчет, поэтому вместо простого копирования используется присваивание.

Проблема в том, что при выделении массива он иногда перезаписывает часть массива другого стека.Как и следовало ожидать, это приводит к возможным ошибкам сегментации при разыменовании этих данных.

class FastStack {
private:
    int m_size, m_ptr;
    ObjectRef* m_stack;

public:
    FastStack(int size) : m_size(size), m_ptr(-1) {
        m_stack = new ObjectRef[m_size];
    }

    FastStack(const FastStack& copy) : m_size(copy.m_size), m_ptr(copy.m_ptr) {
        long a = (long)copy.m_stack[0];

        m_stack = new ObjectRef[m_size];

        if ((long)copy.m_stack[0] != a)
            fprintf(stderr, "\nWe have a serious problem!\n\n");

        for (int i = 0; i <= m_ptr; i++)
            m_stack[i] = copy.m_stack[i];
    }

    ~FastStack() {
        delete[] m_stack;
    }
};

class ObjectRef {
private:
    DataObj* m_obj;

public:
    ObjectRef() : m_obj(0) { }

    ObjectRef(DataObj* obj) : m_obj(obj) {
        if (m_obj) m_obj->addRef();
    }

    ObjectRef(const ObjectRef& obj) : m_obj(obj.m_obj) {
        if (m_obj) m_obj->addRef();
    }

    ~ObjectRef() {
        if (m_obj) m_obj->delRef();
    }

    ObjectRef& operator=(DataObj* obj) {
        if (obj) obj->addRef();
        if (m_obj) m_obj->delRef();
        m_obj = obj;
        return *this;
    }

    ObjectRef& operator=(const ObjectRef& obj) {
        if (obj.m_obj) obj.m_obj->addRef();
        if (m_obj) m_obj->delRef();
        m_obj = obj.m_obj;
        return *this;
    }
};

Я вижу, что "У нас серьезная проблема!"Строка незадолго до segfault, и, шагая по ней с помощью gdb, я вижу, что один из ObjectRef, созданных new, имеет тот же адрес, что и массив другого стека.

Мой первый инстинкт - сказать, что new никогда не долженвыделение памяти, которая уже используется, но это, очевидно, имеет место в данном случае, и я в полной растерянности относительно того, что можно сделать.

Добавлено: В тот момент, когда я вижу, что это происходит, m_size =2 и m_ptr = 0.

Полный код этой программы находится на github по адресу https://github.com/dvpdiner2/pycdc, но довольно запутан и труден для понимания.

Answer 1

Иногда известно, что new выделяет память, которая уже была выделена другим объектам, и когда это происходит, хороший код C ++ должен повторить выделение в надежде на то, что память не будет использоваться совместно с другими ...

НЕТ!

new все в порядке, ваша программа глючит. Напишите это сто раз на доске.

new никогда не даст вам память, которая была передана кому-то другому НО , если вы сделали что-то плохое раньше (то есть доступ за пределы, разыменование указателя, как только объект, на который он указывал, имеет был освобожден, использовал недействительный итератор и миллионы других возможных нарушений "UB"), а затем new получает специальное разрешение и может делать все, что захочет, включая создание демонов из ваших ноздрей.

Ваш код выглядит весьма подозрительно, но я не вижу ничего, что наверняка ошибка (слишком много кода скрыто, например, что такое ObjRef и как он объявлен). Однако из того, что я вижу, я почти уверен, что вы сделали что-то не так, прежде чем перейти к этому новому распределению, потому что есть много нарушений хороших практик C ++ (например, показанный класс имеет конструктор и конструктор копирования, но нет оператора присваивания или деструктора) .

Самая большая проблема с показанным кодом заключается в том, что он выглядит как полуобеспеченная и ошибочная попытка имитировать подмножество того, что будет делать обычный std :: vector. Чтобы точно сказать, что является (являются) проблемой (ами), требуется больше контекста ... это плохой код, но может быть законным в зависимости от того, как был написан другой код. Также очевидно, что даже этот небольшой фрагмент кода был изменен и уменьшен, поскольку нет методов для доступа к чему-либо, нет друзей и членов данных, которые являются частными (поэтому в принципе невозможно что-либо сделать с этими объектами).

Answer 2

Что вам нужно сделать, это использовать std::vector для управления памятью и просто сохранить индекс (m_ptr), управляемый вами явно. Это решит все эти проблемы.

Однако я серьезно не понимаю, что, черт возьми, не так с вашим кодом. Считай себя оптимизатором и сделай быстрый проход, сосредоточившись только на a и copy.m_stack[0].

long a = (long)copy.m_stack[0];

m_stack = new ObjectRef[m_size];

if ((long)copy.m_stack[0] != a)



long a = (long)copy.m_stack[0];

if ((long)copy.m_stack[0] != a)


if (copy.m_stack[0] != copy.m_stack[0])

Если это условие истинно, значит, вы испортили кучу и возможно , но менее вероятно, стек. Повреждение кучи может привести к появлению совершенно несвязанного кода с ошибочным кодом. Либо так, либо ты шутишь с нитками и не говоришь нам об этом. (и ошибаюсь).

Конечно, вам не хватает нескольких других важных функций, таких как operator = и destructor, но вам не придется реализовывать себя самостоятельно, если бы вы использовали вектор.

Answer 3

В цикле вы перебираете от 0 до m_ptr:

for (int i = 0; i <= m_ptr; i++)
    m_stack[i] = copy.m_stack[i];

Но ваш массив m_stack содержит m_size элементов (при условии, что вы их инициализировали).

EDIT: единственный способ (я мог видеть) перекрывать m_stack и copy.m_stack, если используется оператор размещения new. Но, согласно опубликованному источнику, вы этого не сделали.

Answer 4

Может ли быть так, что ваш код работает с несколькими потоками?

Поскольку new никогда не возвращает память, которая уже выделена, единственный способ получить сообщение о проверке состоит в том, что другие потоки что-то делают с экземпляром копии.

Это подозрение также вызвано тем фактом, что это происходит только время от времени.Как правило, это для многопоточных ошибок (поскольку им требуется особая синхронизация, которая происходит только время от времени).

Answer 5

Вы можете заменить весь код, отправленный этим:

class FastStack {
public:

protected:
    std::vector<boost::shared_ptr<DataObj> > m_Stack;
};

(и если вам нужно добавить конструктор, который гарантирует, что вектор предварительно выделил свою память).

Когда вы выполняете ненужную работу, как вы делаете здесь, реализуя классы, которые кто-то другой уже сделал лучше («кто-то другой» в данном случае умный человек, умнее большинства из нас), вероятно, вы добавите больше проблем чем вы решаете.

std :: vector будет гораздо лучшей коллекцией, чем выделенный вами массив, и boost :: shared_ptr (или tr1 :: shared_ptr или std :: shared_ptr, если вы используете tr1 / c + + 0x-совместимый компилятор) делает именно то, что делает ваш класс ObjectRef (но лучше). Из вашего кода это выглядит так, как будто DataObj содержит свой собственный refcount, который вы также можете удалить, если вместо этого используете shared_ptrs.

Используйте доступные инструменты - чем меньше кода вы должны написать, тем меньше у вас шансов на написание ошибок ... Если вы замените свой код на вышеприведенный и все еще видите проблемы, ваша ошибка где-то еще.

Answer 6

Мне кажется, что наиболее вероятным виновником на самом деле является ввод в конструктор копирования.

Тем не менее, я скажу, что этот код не выполняет достаточной проверки, и то, как он сконструирован, усложнит обнаружение ошибки или неправильного использования до тех пор, пока не произойдет сбой в будущем. Класс, написанный таким образом, будет трудно найти ошибки, как вы видите сейчас.

Простые вещи, которые вы должны сделать как минимум: 1. Обработка нового броска исключения 2. Проверьте границы перед индексированием в массив, даже если вы просто используете m_stack [0] 3. Убедитесь, что вход в конструктор копирования действителен (он же инициализирован)

О, и самый простой способ отследить ошибки памяти, подобные этой, - это просто проверить их. Он удивительно хорош в быстром устранении проблем с памятью, даже если подумал, что это не заявленная цель.

Также было бы очень просто отлаживать, если бы вы включили работающий кусок кода, который изолировал проблему в ее простейшей воспроизводимой форме.