Mauravan Mauravan - 3 months ago 30
C++ Question

c++ can't find detected memory leaks

I'm fairly new to c++ and i'm having some difficulties.
I'm trying to build a String class that wrapps around c-strings

Now after some time i realized that there are some memory leaks in the heap, but i can't seem to find the mistake.

This is my Header:

#include <iostream>
#include <memory>
using namespace std;

class String final {
size_t m_len;
size_t m_start;
shared_ptr<char> m_string;

public:
String(); // Standard Constr
String(const String& s); // Copy Constr
String(const char* s); // Typconvert Constr

// Deconstr
~String();
}


These are the constructors in the .cpp:

#include "stdafx.h"
#include "MyString.h"
#include <iostream>

using namespace std;

String::String() : m_len(0), m_start(0), m_string(nullptr)
{
}

String::String(const String &obj) : m_len(obj.m_len), m_start(obj.m_start), m_string(obj.m_string)
{
}

String::String(const char* obj) : m_len(strlen(obj))
{
shared_ptr<char> ptr(unique_ptr<char[]>(new char[m_len]));
m_string = ptr;

for (size_t i = 0; i <= m_len; i++) {
*(m_string.get() + i*sizeof(char)) = obj[i];
if (i == m_len)
{
*(m_string.get() + i*sizeof(char)) = '\0';
}
}
}

String::~String()
{
m_string.reset();
}


And this is my main:

#include "stdafx.h"
#include "MyString.h"
#include <iostream>

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>

using namespace std;

int main()
{
_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
String s0;
String s1("");
String s2("abc");
String s3("ab");

String s00(s0);
String s11(s1);
String s22(s2);

_CrtDumpMemoryLeaks();
return 0;
}


Now when i run this i get an error:

Detected memory leaks!
Dumping objects ->
{66} normal block at 0x00668458, 16 bytes long.
Data: <X f > 58 AB 0E 01 01 00 00 00 01 00 00 00 D8 8D 66 00
{65} normal block at 0x00668DD8, 2 bytes long.
Data: <ab> 61 62
{64} normal block at 0x00668D98, 16 bytes long.
Data: <X f > 58 AB 0E 01 02 00 00 00 01 00 00 00 F8 91 66 00
{63} normal block at 0x006691F8, 3 bytes long.
Data: <abc> 61 62 63
{62} normal block at 0x00668C60, 16 bytes long.
Data: <X f > 58 AB 0E 01 02 00 00 00 01 00 00 00 D8 8F 66 00
{61} normal block at 0x00668FD8, 0 bytes long.
Data: <> öß³‘@þÿÿÿ÷O
Object dump complete.
Debug Error!

HEAP CORRUPTION DETECTED: after Normal block (#65) at 0x00668DD8.
CRT detected that the application wrote to memory after end of heap buffer.


I can't figure out where that is coming from. I thought about the deconstructor and tried to take it out, so that it would write one for me, but that didn't work either. Also, the code block that seems to break the program is this one:

void* __CRTDECL operator new(size_t const size)
{
for (;;)
{
if (void* const block = malloc(size))
{
return block;
}

if (_callnewh(size) == 0)
{
if (size == SIZE_MAX)
{
__scrt_throw_std_bad_array_new_length();
}
else
{
__scrt_throw_std_bad_alloc();
}
}

// The new handler was successful; try to allocate again...
}
}


from new_scalar.cpp.

Thank you very much for any help.

Answer

Using valgrind you see your problems:

==20490== Memcheck, a memory error detector
==20490== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==20490== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==20490== Command: go
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C76: String::String(char const*) (main.cpp:38)
==20490==    by 0x80487E6: main (main.cpp:59)
==20490==  Address 0x443fa58 is 0 bytes after a block of size 0 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x80487E6: main (main.cpp:59)
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C82: String::String(char const*) (main.cpp:41)
==20490==    by 0x80487E6: main (main.cpp:59)
==20490==  Address 0x443fa58 is 0 bytes after a block of size 0 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x80487E6: main (main.cpp:59)
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C76: String::String(char const*) (main.cpp:38)
==20490==    by 0x80487F6: main (main.cpp:60)
==20490==  Address 0x443facb is 0 bytes after a block of size 3 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x80487F6: main (main.cpp:60)
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C82: String::String(char const*) (main.cpp:41)
==20490==    by 0x80487F6: main (main.cpp:60)
==20490==  Address 0x443facb is 0 bytes after a block of size 3 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x80487F6: main (main.cpp:60)
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C76: String::String(char const*) (main.cpp:38)
==20490==    by 0x8048806: main (main.cpp:61)
==20490==  Address 0x443fb42 is 0 bytes after a block of size 2 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x8048806: main (main.cpp:61)
==20490== 
==20490== Invalid write of size 1
==20490==    at 0x8048C82: String::String(char const*) (main.cpp:41)
==20490==    by 0x8048806: main (main.cpp:61)
==20490==  Address 0x443fb42 is 0 bytes after a block of size 2 alloc'd
==20490==    at 0x402AF41: operator new[](unsigned int) (vg_replace_malloc.c:417)
==20490==    by 0x8048BDC: String::String(char const*) (main.cpp:34)
==20490==    by 0x8048806: main (main.cpp:61)
==20490== 
==20490== 
==20490== HEAP SUMMARY:
==20490==     in use at exit: 18,944 bytes in 1 blocks
==20490==   total heap usage: 7 allocs, 6 frees, 18,997 bytes allocated
==20490== 
==20490== LEAK SUMMARY:
==20490==    definitely lost: 0 bytes in 0 blocks
==20490==    indirectly lost: 0 bytes in 0 blocks
==20490==      possibly lost: 0 bytes in 0 blocks
==20490==    still reachable: 18,944 bytes in 1 blocks
==20490==         suppressed: 0 bytes in 0 blocks
==20490== Rerun with --leak-check=full to see details of leaked memory
==20490== 
==20490== For counts of detected and suppressed errors, rerun with: -v
==20490== ERROR SUMMARY: 6 errors from 6 contexts (suppressed: 0 from 0)

After fixing new[strlen] to new[strlen+1] ->

==21539== Memcheck, a memory error detector
==21539== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==21539== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==21539== Command: go
==21539== 
==21539== 
==21539== HEAP SUMMARY:
==21539==     in use at exit: 18,944 bytes in 1 blocks
==21539==   total heap usage: 7 allocs, 6 frees, 19,000 bytes allocated
==21539== 
==21539== LEAK SUMMARY:
==21539==    definitely lost: 0 bytes in 0 blocks
==21539==    indirectly lost: 0 bytes in 0 blocks
==21539==      possibly lost: 0 bytes in 0 blocks
==21539==    still reachable: 18,944 bytes in 1 blocks
==21539==         suppressed: 0 bytes in 0 blocks
==21539== Rerun with --leak-check=full to see details of leaked memory
==21539== 
==21539== For counts of detected and suppressed errors, rerun with: -v
==21539== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

But some remarks on your code:

String::~String() 
{
    m_string.reset();
}

Why??? If class is destructed the pointer will destructed which automatically results in freeing the pointer itself. It is stupid to do it twice! Leave the destructor empty, use the automatic one!

for (size_t i = 0; i <= m_len; i++) {
    *(m_string.get() + i*sizeof(char)) = obj[i];
    if (i == m_len)
    {
        *(m_string.get() + i*sizeof(char)) = '\0';
    }
}

Is the same as strcpy! If you use strlen to get the size, you can also use the copy from the libs.

String::String(const char* obj) : m_len(strlen(obj))
{
    shared_ptr<char> ptr(unique_ptr<char[]>(new char[m_len]));
    m_string = ptr;

Should be replaced with:

String::String(const char* obj) : m_len(strlen(obj)), m_string{ new char[m_len+1], delArray<char>() } { ... }

where delArray is:

template< typename T >
struct delArray { void operator ()( T const * p) { delete[] p; } };

There are some more unusual code fragments...

Comments