Re: hash_set core dump on memory free

[Rupert Kittinger]

{ Note: this article is cross-posted to comp.lang.c++, 
microsoft.public.vc.stl, gnu.g++.help and comp.lang.c++.moderated. -mod }

Rakesh schrieb:

> Hi -
>
>   What is wrong this implementation? I get a core dump at the free()
> statement? Thanks
>
> Rakesh
>
> #include <ext/hash_map>
> #include <iostream.h>
> #include <ext/hash_set>
>
>
> using namespace std;
> using namespace __gnu_cxx;
>
> struct eqstr
> {
>   bool operator()(char* s1, char* s2) const
>   {Variou
>     return strcmp(s1, s2) == 0;
>   }
> };
>
> int main()
> {
>
>   char *s, *s1, *temp;
>   hash_set<char *, hash<char *>, eqstr> AddedPhrases;
>   hash_set<char*, hash<char*>, eqstr> ::iterator Iter1;
>
>
>   s = (char *)malloc(sizeof(char)*100);
>   strcpy(s, "apple");
>   AddedPhrases.insert(s);
>
>   s1 = (char *)malloc(sizeof(char)*100);
>   strcpy(s1, "absent");
>   AddedPhrases.insert(s1);
>   for (Iter1 = AddedPhrases.begin(); Iter1 != AddedPhrases.end();
> Iter1++)
>   {
>          temp = *Iter1;
>          //printf("\nDeleting:%s:%d", temp, strlen(temp));
>          free(temp);
>   }
> }
>
> $ g++ test3.cpp
> $ ./a.out
>
> Deleting:apple:5
> Deleting:absent:6
> *** glibc detected *** ./a.out: double free or corruption (!prev):
> 0x09fa2310 ***
> ======= Backtrace: =========
> /lib/libc.so.6[0x6b9f18]
> /lib/libc.so.6(__libc_free+0x79)[0x6bd41d]
> ./a.out(__gxx_personality_v0+0x274)[0x804892c]
> /lib/libc.so.6(__libc_start_main+0xdc)[0x66b7e4]
> ./a.out(__gxx_personality_v0+0x69)[0x8048721]
> ======= Memory map: ========
   {
> 0053b000-00546000 r-xp 00000000 fd:00 38110218
> /lib/libgcc_s-4.1.0-20060304.so.1
> 00546000-00547000 rwxp 0000a000 fd:00 38110218
> /lib/libgcc_s-4.1.0-20060304.so.1
> 00549000-0062b000 r-xp 00000000 fd:00 19344300
> /usr/lib/libstdc++.so.6.0.8
> 0062b000-0062f000 r-xp 000e2000 fd:00 19344300
> /usr/lib/libstdc++.so.6.0.8
> 0062f000-00630000 rwxp 000e6000 fd:00 19344300
> /usr/lib/libstdc++.so.6.0.8
> 00630000-00636000 rwxp 00630000 00:00 0
> 00639000-00652000 r-xp 00000000 fd:00 38110213   /lib/ld-2.4.so
> 00652000-00653000 r-xp 00018000 fd:00 38110213   /lib/ld-2.4.so
> 00653000-00654000 rwxp 00019000 fd:00 38110213   /lib/ld-2.4.so
> 00656000-00782000 r-xp 00000000 fd:00 38110214   /lib/libc-2.4.so
> 00782000-00785000 r-xp 0012b000 fd:00 38110214   /lib/libc-2.4.so
> 00785000-00786000 rwxp 0012e000 fd:00 38110214   /lib/libc-2.4.so
> 00786000-00789000 rwxp 00786000 00:00 0
> 0078b000-007ae000 r-xp 00000000 fd:00 38110217   /lib/libm-2.4.so
> 007ae000-007af000 r-xp 00022000 fd:00 38110217   /lib/libm-2.4.so
> 007af000-007b0000 rwxp 00023000 fd:00 38110217   /lib/libm-2.4.so
> 00974000-00975000 r-xp 00974000 00:00 0          [vdso]
> 08048000-0804c000 r-xp 00000000 fd:00 24674319   /home/oracle/a.out
> 0804c000-0804d000 rw-p 00003000 fd:00 24674319   /home/oracle/a.out
> 09fa2000-09fc3000 rw-p 09fa2000 00:00 0          [heap]
> b7e00000-b7e21000 rw-p b7e00000 00:00 0
> b7e21000-b7f00000 ---p b7e21000 00:00 0
> b7fa8000-b7fac000 rw-p b7fa8000 00:00 0
> bf897000-bf8ac000 rw-p bf897000 00:00 0          [stack]
> Deleting:Xax:3Aborted

Hi Rakesh,

the problem is that you are freeing an object that is still inside the
container, but the iterator still tries to access the object during to
call to operator++(). The reason is that the iterator does not store the
bucket number, so when the end of the bucket is reached, the hash
function is called to compute the bucket number. At this point,
hash<char*>() is called with a pointer that no longer points to valid
memory, so you encounter undefined behaviour.

So you have to erase() the string from the container before calling
free(), but after calling

AddedPhrases.erase(Iter1);

Iter1 is no longer a valid iterator. So the whole loop must be rewritten:

for (Iter1 = AddedPhrases.begin();
      Iter1 != AddedPhrases.end();
      /* do nothing */) { // increment is now performed inside the loop

          temp = *Iter1++; // increment iterator, then dereference
                                                  // original value value
          free(temp);
}


There are other issues with this code, e.g. missing checks for the size
of the string.

And if there is not a very good reason, it is much simpler to use
std::string which will take care of all the memory management for you.

cheers,


Rupert

--
     [ See http://www.gotw.ca/resources/clcm.htm for info about ]
     [ comp.lang.c++.moderated.    First time posters: Do this! ]