-2

Story: I'm making a shared library exposing C code as a wrapper to a C++ static library compiled in VS 2008, so I'm tied to that compiler. In my library I'm trying to do a very simple dictionary to pass data through the shared library boundaries. I'll not pass the actual dictionary, just the handler an provide functions to access its members. The dictionary is simply a std::vector of key-value pairs.

Problem: My program crashes every time I try to push_back a key-value pair into the dictionary. Here is a self-contained demo code (main.cpp of a fresh Win32 console project):

#include "stdafx.h"

#include <cstdlib>
#include <vector>

typedef enum KeyType
{
    KeyType_CHAR = 0,
    KeyType_INT,
    KeyType_CHAR_PTR_AS_STRING
};

typedef enum ValueType
{
    ValueType_CHAR = 0,
    ValueType_INT,
    ValueType_CHAR_PTR_AS_STRING
};

struct DictNode
{
    ValueType value_type;
    void* value_unsafe_ptr;
    void* key_unsafe_ptr;
};

struct Dict
{
    KeyType key_type;
    std::vector<DictNode> nodes;
};

int _tmain(int argc, _TCHAR* argv[])
{
    /* Create Dict */
    Dict* test = (Dict*)malloc(sizeof(Dict));
    test->key_type = KeyType_INT;

    /* Add (0, "Zero") */
    int* key0 = (int*)malloc(sizeof(int));
    *key0 = 0;

    char* content0 = (char*)malloc(sizeof(char)*5);
    content0[0] = 'Z';
    content0[1] = 'e';
    content0[2] = 'r';
    content0[3] = 'o';
    content0[4] = '\0';

    DictNode node0;
    node0.value_type = ValueType_CHAR;
    node0.key_unsafe_ptr = key0;
    node0.value_unsafe_ptr = content0;

    test->nodes.push_back(node0); // BOOM

    /* Release memory */
    test->nodes.clear();
    free(key0);
    free(content0);
    free(test);

    return 0;
}

At line test->nodes.push_back(node0); I get 0xC0000005: Access violation reading location 0xcdcdcdc1. By setting a breakpoint at that line, I can see that all key, content0, node0 and test are defined and have the correct values.

jabozzo
  • 591
  • 1
  • 6
  • 17
  • 3
    Why are you using `malloc` if this is a C++ program? – lcs Mar 02 '16 at 20:29
  • Because I'm storing values as void*, type safety is ensured using the set/get functions (not in sample code) which sets/checks the type using the `ValueType` or `KeyType` enumeration. – jabozzo Mar 02 '16 at 20:35
  • 1
    If you really need `void*` pointers (note: you shouldn't need them), you can obtain them from pointers allocated with `new` by using `reinterpret_cast` – lcs Mar 02 '16 at 20:39

2 Answers2

3

Dict* test = (Dict*)malloc(sizeof(Dict)); does not do what you think it does.

malloc allocates a block of memory, but does not initialize it. So later on, when you call test->nodes.push_back, you're calling into uninitialized memory. Undefined behavior. In your case, it crashes.

The solution here is allocate test with new, which will initialize both test and test->nodes.

Dict* test = new Dict;

The even better solution is to ask why test is dynamically allocated to begin with.

lcs
  • 4,227
  • 17
  • 36
1

Since you are using C for allocating stuff, you do not get C++ initialization behavior.

So when you allocate your Dict, the std::vector is still completely uninitialized. You can't use it without initializing it first. You should use placement new for this as long as you do not want to actually use C++.

The proper solution is to program in C++ => use new instead of malloc. Also, it's a good idea to not store stuff on the heap unless you really need to.

So the quick fix is to add:

new (&(test->nodes)) std::vector<DictNodes>;

after your allocate test.

A better way would be to stop using malloc alltogether and to use

Dict *test = new Dict; // this automatically initializes the vector

or possibly

Dict test; // no need to allocate on the heap if you don't need to
villintehaspam
  • 8,540
  • 6
  • 45
  • 76