As the title says, when the unordered_map uses the [] operator to insert elements, there will be a Segmentation fault, and the most confusing place for me is that this error occurs when I use resize() on vec, and when I use push_back() The program has no problem . I don't understand what is causing this.
#include<iostream>
#include<unordered_map>
#include<vector>
#include<cstdlib>
#include<ctime>
using namespace std;
struct Element
{
int key;
vector<int> vec;
int flag;
};
class Test
{
private:
unordered_map<int,Element *> map;
public:
Element *getElement(int key,int flag)
{
Element *element;
auto temp = map.find(key);
if(temp==map.end()||flag == temp->second->flag)
{
element = new Element();
element->key = key;
element->flag = flag;
if(temp != map.end())
{
delete temp->second;
map.erase(key);
}
map[key] = element;
}
else
{
element = map[key];
}
return element;
}
};
int main()
{
srand((unsigned)time(nullptr));
Test test;
for (size_t i = 0; i < 100000; i++)
{
/* code */
int vecSize = rand()%100;
int key = rand()%5000;
int flag = rand()%5000;
Element *element = test.getElement(key,flag);
if(element->vec.size()==0)
{
element->vec.resize(vecSize,0);
}
for (size_t i = 0; i < vecSize; i++)
{
element->vec[i] = rand()%10000;
}
}
return 0;
}
output:
Program received signal SIGSEGV, Segmentation fault.
0x000055555555716e in std::__detail::_Hash_node<std::pair<int const, Element*>, false>::_M_next (this=0x8470000046a) at /usr/include/c++/7/bits/hashtable_policy.h:298
298 { return static_cast<_Hash_node*>(this->_M_nxt); }
(gdb) bt
#0 0x000055555555716e in std::__detail::_Hash_node<std::pair<int const, Element*>, false>::_M_next (this=0x8470000046a) at /usr/include/c++/7/bits/hashtable_policy.h:298
#1 0x000055555555782c in std::_Hashtable<int, std::pair<int const, Element*>, std::allocator<std::pair<int const, Element*> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_rehash_aux (this=0x7fffffffe370, __n=337) at /usr/include/c++/7/bits/hashtable.h:2098
#2 0x0000555555556ef2 in std::_Hashtable<int, std::pair<int const, Element*>, std::allocator<std::pair<int const, Element*> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_rehash
(this=0x7fffffffe370, __n=337, __state=@0x7fffffffe220: 167) at /usr/include/c++/7/bits/hashtable.h:2071
#3 0x00005555555564e0 in std::_Hashtable<int, std::pair<int const, Element*>, std::allocator<std::pair<int const, Element*> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_insert_unique_node (this=0x7fffffffe370, __bkt=88, __code=1424, __node=0x55555577a330) at /usr/include/c++/7/bits/hashtable.h:1718
#4 0x0000555555555992 in std::__detail::_Map_base<int, std::pair<int const, Element*>, std::allocator<std::pair<int const, Element*> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true>, true>::operator[] (this=0x7fffffffe370, __k=@0x7fffffffe2f4: 1424) at /usr/include/c++/7/bits/hashtable_policy.h:728
#5 0x00005555555554db in std::unordered_map<int, Element*, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<int const, Element*> > >::operator[] (this=0x7fffffffe370,
__k=@0x7fffffffe2f4: 1424) at /usr/include/c++/7/bits/unordered_map.h:973
#6 0x00005555555551ca in Test::getElement (this=0x7fffffffe370, key=1424, flag=318) at main.cpp:35
#7 0x0000555555554e23 in main () at main.cpp:56
Let's take a look at your major design decision:
unordered_map<int,Element *> map;
You are using a "bare pointer", which should be reserved for cases where the data structure, meaning your map
, needs to refer to something, but it doesn't own it. This is not the case in your program: no one else owns the Element
objects. So you should store them explicitly, and return references:
unordered_map<int,Element> map;
Element& getElement(int key,int flag);
As other people have also indicated: your program can be both simpler, and correct. You're making life too hard for yourself by not writing proper C++.
EDIT because you indicate that you're stuck with a legacy API:
It would still be possible to preserve the API but redo the implementation. If the API requires you to return a *
pointer to internals (rather than a reference as I suggested), store the elements internally as smart pointers, and return the get()
result from that.