Критика моего кучи отладчика

Question

Я написал следующий отладчик кучи для демонстрации утечек памяти, двойного удаления и неправильных форм удалений (то есть, пытаясь удалить массив с delete p вместо delete[] p) для начинающих программистов.

Я хотел бы получить отзывы об этом от сильных программистов на C ++, потому что я никогда не делал этого раньше и уверен, что совершил несколько глупых ошибок. Спасибо!

#include <cstdlib>
#include <iostream>
#include <new>

namespace
{
    const int ALIGNMENT = 16;
    const char* const ERR = "*** ERROR: ";
    int counter = 0;

    struct heap_debugger
    {
        heap_debugger()
        {
            std::cerr << "*** heap debugger started\n";
        }

        ~heap_debugger()
        {
            std::cerr << "*** heap debugger shutting down\n";
            if (counter > 0)
            {
                std::cerr << ERR << "failed to release memory " << counter << " times\n";
            }
            else if (counter < 0)
            {
                std::cerr << ERR << (-counter) << " double deletes detected\n";
            }
        }
    } instance;

    void* allocate(size_t size, const char* kind_of_memory, size_t token) throw (std::bad_alloc)
    {
        void* raw = malloc(size + ALIGNMENT);
        if (raw == 0) throw std::bad_alloc();

        *static_cast<size_t*>(raw) = token;
        void* payload = static_cast<char*>(raw) + ALIGNMENT;

        ++counter;
        std::cerr << "*** allocated " << kind_of_memory << " at " << payload << " (" << size << " bytes)\n";
        return payload;
    }

    void release(void* payload, const char* kind_of_memory, size_t correct_token, size_t wrong_token) throw ()
    {
        if (payload == 0) return;

        std::cerr << "*** releasing " << kind_of_memory << " at " << payload << '\n';
        --counter;

        void* raw = static_cast<char*>(payload) - ALIGNMENT;
        size_t* token = static_cast<size_t*>(raw);

        if (*token == correct_token)
        {
            *token = 0xDEADBEEF;
            free(raw);
        }
        else if (*token == wrong_token)
        {
            *token = 0x177E6A7;
            std::cerr << ERR << "wrong form of delete\n";
        }
        else
        {
            std::cerr << ERR << "double delete\n";
        }
    }
}

void* operator new(size_t size) throw (std::bad_alloc)
{
    return allocate(size, "non-array memory", 0x5AFE6A8D);
}

void* operator new[](size_t size) throw (std::bad_alloc)
{
    return allocate(size, "    array memory", 0x5AFE6A8E);
}

void operator delete(void* payload) throw ()
{
    release(payload, "non-array memory", 0x5AFE6A8D, 0x5AFE6A8E);
}

void operator delete[](void* payload) throw ()
{
    release(payload, "    array memory", 0x5AFE6A8E, 0x5AFE6A8D);
}

Answer 1

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

Answer 2

Объясните, почему вы выбрали «ALIGNMENT» в качестве идентификатора. Объясните, почему вы выбрали 16. Спорьте, как ваш алгоритм отлавливает наиболее распространенные ошибки, такие как переполнение конца выделенного в куче блока или забывание освободить память.

Answer 3

Это действительно отличное начало. Вот мои 2 цента, как вы просили оставить отзыв:

1. Код записывает информацию трассировки в cerr, которая действительно предназначена для ошибок. Используйте cout для информационных журналов.
2. Сумма выравнивания является произвольной. Если бы код пытался выделить 4090 байт, вы бы выделили 4106, что выльется в следующий блок 4k, который является размером страницы памяти. Рассчитанное значение выравнивания будет лучше ... или переименуйте ALIGNMENT в HEADER_SIZE или что-то подобное.
3. Учитывая заголовок, который вы создаете, вы можете сохранить размер и флаг для «вида памяти» во время выделения и сравнить его во время выпуска.
4. Маркер, вероятно, должен называться «часовой» или «канарейка».
5. Почему токен является size_t? Почему бы не пустота *?
6. Ваша проверка на null в выпуске, вероятно, должна вместо этого выдать исключение - не будет ли ошибкой, если код удалит нулевой указатель?
7. Ваши значения 'correct_token' и 'false_token' слишком похожи. Я должен был перечитать код, чтобы быть уверенным.
8. Учитывая пункт (3), вы можете удвоить сумму, которую вы дополнительно выделяете и имеете до и после часовых / охранных блоков. Это позволит обнаружить переполнение и переполнение памяти.

Answer 4

Я не очень хорошо следую вашему использованию жестко закодированных констант / константных строк - поместите их в перечисление? И я действительно не совсем понимаю идею токена. Нужно больше комментариев

Answer 5

void* raw = static_cast<char*>(payload) - ALIGNMENT;

Если payload уже удален, разве это не приведет к неопределенному поведению?