Google Sparsehash использует realloc () для типа, который нетривиально копируется - PullRequest
Новая
       41

Google Sparsehash использует realloc () для типа, который нетривиально копируется

0 голосов
/ 21 сентября 2018

Рассмотрим эту простую программу: 

#include <string>
#include <sparsehash/dense_hash_map>

int main()
{
    google::dense_hash_map<std::string, int> map;
    map["foo"] = 0;
}

Компиляция с GCC 8.2 и -Wclass-memaccess (или -Wall) выдает предупреждение: 

sparsehash/internal/libc_allocator_with_realloc.h:68:40: warning:
‘void* realloc(void*, size_t)’ moving an object of non-trivially copyable type
    ‘struct std::pair<const std::__cxx11::basic_string<char>, int>’;
use ‘new’ and ‘delete’ instead [-Wclass-memaccess]
    return static_cast<pointer>(realloc(p, n * sizeof(value_type)));
                                ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

Вопросы:

  1. Это неопределенное поведение?
  2. Можете ли вы предложить исправление или обходной путь, который можно применить к коду приложения (не изменяя Sparsehash или избегая его использования)?
  3. (Бонусные баллы) Можете ли вы создать программу, которая на самом деле ведет себя неправильно из-за этого (используя std :: string или ваш собственный нетривиальный тип)?До сих пор я не видел никаких проблем в коде с использованием std :: string в качестве типа ключа, несмотря на тот факт, что std :: string должен быть довольно распространенным типом ключа.

подал вопрос здесь: https://github.com/sparsehash/sparsehash/issues/149

Ответы [ 3 ]

0 голосов
/ 25 сентября 2018

  1. Да, это неопределенное поведение.
    Но пока не отчаивайтесь, если std::string не хранит никаких внутренних указателей в вашей реализации и нигде не регистрирует их, это будет "работать"" тем не мение;создание побитовой копии будет эквивалентно построению перемещения в месте назначения и уничтожению источника.
    Так обстоит дело с большинством ( не всех) реализаций строк, SSO или нет.

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

  3. Создание программы из-за неправильного перемещения по битовой копии тривиально.
    Используйте этот тип с google::dense_hash_map: 

    class bang {
    bang* p;
public:
    bang() : p(this) {}
    bang(bang const&) : bang() {}
    bang& operator=(bang const&) { return *this; }
    ~bang() { if (p != this) std::abort(); }
};
0 голосов
/ 27 сентября 2018

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

Например,этот стандарт не может быть указан как «неопределенное поведение» стандартом C ++ 20: 

alignas(string) unsigned char buffer[sizeof(string)];
auto p = new(buffer) string{"test"};
alignas(string) unsigned char buffer2[sizeof(string)];
memcpy(buffer,buffer2,sizeof(string));//create a new string object copy of *p;
auto np = reinterpret_cast<string*>(buffer2);
(*np)[0]="r";
// the object at p shall not be accessed, not even destroyed.

Тип не должен быть тривиально перемещаемым, если он имеет элемент не статических данных, который ссылается на любую частьсамо по себе: 

struct fail{
    int a;
    int b;
    int* selected;
    fail(bool is_a,int i){
       if (is_a){ a=i; selected=&a;}
       else { b=i; selected=&b;}
       }
     };

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

0 голосов
/ 25 сентября 2018

1.Это неопределенное поведение? Да.Вы никогда не должны копировать объекты, используя realloc (), потому что иногда они имеют внутренние указатели, которые указывают на ресурс.Проблема возникает позже, когда на двух разных объектах работают деструкторы.Теперь происходит двойное освобождение одного и того же ресурса , полный номер нет.

2.Можете ли вы предложить исправление или обходной путь, который можно применить к коду приложения (не изменяя Sparsehash или избегая его использования)?

Попробуйте 

#include <memory>

и измените строку 

google::dense_hash_map<std::string, int> map;

до 

google::dense_hash_map<std::string, int, std::hash<std::string>, std::equal_to<std::string>, std::allocator> map;

Теперь он не будет использовать распределитель Google libc_allocator_with_realloc

3.(Бонусные баллы) Можете ли вы создать программу, которая на самом деле ведет себя неправильно из-за этого (используя std :: string или ваш собственный нетривиальный тип)?До сих пор я не видел никаких проблем в коде с использованием std :: string в качестве типа ключа, несмотря на тот факт, что std :: string должен быть довольно распространенным типом ключа.

Не так просто.Потому что вы пытаетесь вызвать неопределенное поведение.В вашей тестовой программе я бы передавал строки длиной не менее 32 символов, поэтому оптимизация небольших строк не запускается. И в куче gcc можно выполнить тесты, чтобы проверить, не испортилась ли она.См 1

...