On 03/10/11 15:52, David Chapman wrote: > On 10/3/2011 7:23 AM, Andrew Cooper wrote: >> Hello, >> >> I am using valgrind-3.6.0.SVN-Debian, and cant work out why I am getting >> Invalid read errors. >> >> The routine is one which is adding symbols to a std::map, indexed by >> their virtual address. As multiple symbols may share the same virtual >> address, I need to concatenate the symbol names in the already present >> symbol entry. >> >> I have the following (cut down, debugging) code which demonstrates the >> problem: >> >> Symbol * prev = prev_itt->second; >> char * prev_name = prev->name; >> >> size_t newsize = strlen(prev_name); >> newsize += strlen(sym->name); >> newsize += 2; >> >> >> prev->name = new char[newsize]; >> //Should concat and delete prev_name, but for debugging >> leave like this >> delete [] prev->name; >> >> //For debugging, put this back, so no leaks >> prev->name = prev_name; > There is a bug here: prev_name is a copy of a pointer to the memory you > just deleted. prev->name now points to invalid memory. valgrind tries > to keep track of these but if the memory is not referenced for a long > time (and many new allocations), it may not be able to determine that > the reference is to just-freed memory (as opposed to as-yet unallocated > memory).
prev_name is a copy of the original prev->name. However, the next thing which happens is to prev->name is pointed to a new heap location, without deleting the original. Now prev_name and prev->name point to different, valid heap locations. For debugging purposes, I then deleted that new heap location, turning prev->name into an invalid pointer. Finally, assigned prev_name (which is still valid) back to prev->name. Therefore it is a nop, whether the new/delete are present or not. Anyway - Tom Huges already identified the issue and valgrind is now happy with my proper concat code. >> With the new char[newsize] and delete [] prev->name commented out, >> valgrind is perfectly happy with the code (which is effectively a nop) > It is not a no-op unless those two lines are commented out; see above. > >> However, with the new and delete present as above, valigrind complains with: >> >> ==10046== Invalid read of size 1 >> ==10046== at 0x4C286E4: __GI_strlen (mc_replace_strmem.c:284) >> ==10046== by 0x40CB88: SymbolTable::insert(Symbol*) (symbol-table.cpp:37) >> ==10046== by 0x406536: parse_symbol_file(std::string) (main.cpp:44) >> ==10046== by 0x406DA4: main (main.cpp:70) >> ==10046== Address 0x7ae7d36 is 0 bytes after a block of size 6 alloc'd >> ==10046== at 0x4C27939: operator new[](unsigned long) >> (vg_replace_malloc.c:305) >> ==10046== by 0x40C969: Symbol::Symbol(unsigned long long, char, char >> const*) (symbol.cpp:9) >> ==10046== by 0x406526: parse_symbol_file(std::string) (main.cpp:44) >> ==10046== by 0x406DA4: main (main.cpp:70) >> ==10046== >> ==10046== Invalid read of size 1 >> ==10046== at 0x4C286E4: __GI_strlen (mc_replace_strmem.c:284) >> ==10046== by 0x40CB94: SymbolTable::insert(Symbol*) (symbol-table.cpp:38) >> ==10046== by 0x406536: parse_symbol_file(std::string) (main.cpp:44) >> ==10046== by 0x406DA4: main (main.cpp:70) >> ==10046== Address 0x7ae7fe6 is 0 bytes after a block of size 6 alloc'd >> ==10046== at 0x4C27939: operator new[](unsigned long) >> (vg_replace_malloc.c:305) >> ==10046== by 0x40C969: Symbol::Symbol(unsigned long long, char, char >> const*) (symbol.cpp:9) >> ==10046== by 0x406526: parse_symbol_file(std::string) (main.cpp:44) >> ==10046== by 0x406DA4: main (main.cpp:70) >> >> Where the two errors are referring to the two strlen() calls when >> calculating newsize. >> >> Are these errors indicating a supposed bug in my code, or are they >> complaining about something in the __GI_strlen replaced code. If so, >> does this mean there is a bug in __GI_strlen ? >> >> Thanks in advance, >> > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2dcopy1 _______________________________________________ Valgrind-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/valgrind-users
