0

As a learning/testing-my-limits exercise, I'm trying to make a DLL that can take a given value, serialize it, and de-serialize it for a program compiled with another compiler. So far everything's gone better than expected. However, I'm running into what appears to be a memory corruption issue.

Here's the code I'm using:

//POD_base.h: contains the base template POD (plain old data) class. Also contains memory allocation and deallocation functions.
namespace pod_helpers
{
  void* pod_malloc(size_t size)
  {
    HANDLE heapHandle = GetProcessHeap();
    HANDLE storageHandle = nullptr;

    if (heapHandle == nullptr)
    {
      return nullptr;
    }

    storageHandle = HeapAlloc(heapHandle, 0, size);

    return storageHandle;
  }

  void pod_free(void* ptr)
  {
    HANDLE heapHandle = GetProcessHeap();
    if (heapHandle == nullptr)
    {
      return;
    }

    if (ptr == nullptr)
    {
      return;
    }

    HeapFree(heapHandle, 0, ptr);
  }
}

template<typename T>
class pod
{
protected:
  pod();
  pod(const T& value);
  pod(const pod& copy);                   // no copy ctor in any pod
  ~pod();

  pod<T>& operator=(pod<T> value);
  operator T() const;

  T get() const;
  void swap(pod<T>& first, pod<T>& second);
};

A pod specialization for int:

//POD_basic_types.h: contains pod specializations for basic datatypes
template<>
class pod<int>
{
  typedef int original_type; //these typedefs are meant to make the specializations easier to write and understand, since they're all the same except for the underlying datatypes.
  typedef std::int32_t safe_type;

public:
  pod() : data(nullptr) {}

  pod(const original_type& value)
  {
    set_from(value);
  }

  pod(const pod<original_type>& copyVal)
  {
    original_type copyData = copyVal.get();
    set_from(copyData);
  }

  ~pod()
  {
    release();
  }

  pod<original_type>& operator=(pod<original_type> value)
  {
    swap(*this, value);

    return *this;
  }

  operator original_type() const
  {
    return get();
  }

protected:
  safe_type* data;

  original_type get() const
  {
    original_type result;

    result = static_cast<original_type>(*data);

    return result;
  }

  void set_from(const original_type& value)
  {
    data = reinterpret_cast<safe_type*>(pod_helpers::pod_malloc(sizeof(safe_type)));

    if (data == nullptr)
    {
      return;
    }

    new(data) safe_type (value);
  }

  void release()
  {
    if (data)
    {
      pod_helpers::pod_free(data);
      data = nullptr;
    }
  }

  void swap(pod<original_type>& first, pod<original_type>& second)
  {
    using std::swap;

    swap(first.data, second.data);
  }
};

My DLL takes advantage of this behind-the-scenes type conversion like this:

virtual pod<int> Add(const pod<int> number1, const pod<int> number2);

pod<int> CCDLL_v1_implementation::Add(const pod<int> number1, const pod<int> number2)
{
  int workingNum1, workingNum2;

  workingNum1 = number1;
  workingNum2 = number2;

  return workingNum1 + workingNum2;
}

Then my test program loads the DLL via LoadLibrary/GetProcAddress. So far, so good; I've confirmed that the DLL is actually loaded and the Add function called. I've also verified that pod<int>'s set_from is being called with the correct values. However, this is where things break down.

I expect set_from to allocate enough space for one safe_type (in this case, std::int32_t) value, then store the passed value within the allocated memory. When I check the value of *data within set_from, this seems to be the case. However, when I'm retrieving the pod<int>'s value via get, the pod's data appears to be garbage. It no longer points to the value the pod was passed during set_from.

I know set_from is called on the EXE's side and get is called on the DLL's side. My understanding of the process is as follows:

EXE -> creates pod<int> (allocating memory via GetProcessHeap/HeapAlloc) -> constructs the pod with a given value, which is passed to set_from.
The DLL's Add function is called, with the given pod passed to it.
DLL -> accesses pod<int> -> gets the pod's stored value via get -> accesses the memory the EXE allocated
DLL does its calculations (here, a simple addition) with the pod's value
DLL -> creates pod<int> (allocating memory via GetProcessHeap/HeapAlloc) -> constructs the pod with a given value, which is passed to set_from.
The DLL's newly-constructed pod is returned to the EXE.
EXE -> accesses the pod's internal value via get -> accesses the memory the DLL allocated

This seems to be breaking down where either the DLL or the EXE has to access memory the other allocated. I've seen elsewhere on SO that using this combination of GetProcessHeap/HeapAlloc should work across DLL boundaries. HeapCreate's documentation is also suggestive of the same idea:

The memory of a private heap object is accessible only to the process that created it. If a dynamic-link library (DLL) creates a private heap, the heap is created in the address space of the process that calls the DLL, and it is accessible only to that process.

as is the documentation for GetProcessHeap (emphasis mine):

The GetProcessHeap function obtains a handle to the default heap for the calling process.

Oddly enough, however, GlobalAlloc suffered the same problems GetProcessHeap/HeapAlloc did, making me further question what's going wrong here. Even odder, when I compile both the EXE and the DLL with the same compiler, everything works as expected.

Am I making incorrect assumptions about the way this allocation/deallocation process works? Should I be using something other than GetProcessHeap/HeapAlloc? Or am I simply trying to do the impossible?


Update with information gained from the comments:

Passing a pod by reference (CCDLL_v1_implementation::Add(const pod<int>& number1, const pod<int>& number2) works correctly. Only passing by value does not.

It doesn't appear to matter whether I pass "unwrapped" arguments to a pod-taking function or whether I wrap the arguments in pods first:

pod<int> a = 9;
pod<int> b = 2;

CCDLL_lib->Add(a, b);

produces the same corrupted data as

CCDLL_lib->Add(9, 2);

The only difference seems to be that wrapping arguments in pods first will call the copy c'tor when Add is called, and leaving arguments unwrapped will just call the regular c'tor.

This also doesn't seem to be a class layout issue:

if (reinterpret_cast<const void*>(&data) == reinterpret_cast<const void*>(this))
{
  //simple debug messagebox here
}

evaluates to true on both sides of the EXE/DLL boundary.


I've managed to prove the issue lies on the EXE/DLL boundary, although I still don't know why.

I added a new method to the DLL so I could focus on one argument instead of two:

void Square(pod<int> number);

Then I called it like this:

pod<int> a = 9;
CCDLL_lib->Square(a);

From a series of debug messageboxes, the following sequence occurs:

Regular c'tor called (EXE)
set_from(9) called (EXE)
copy c'tor called (EXE)
get returns 9 to the copy c'tor (EXE)
set_from(9) called (EXE)
Square called (DLL) with an invalid pod (Visual Studio shows the data pointer pointing to garbage at this point)

If I change Square to accept a reference instead (virtual void Square(pod<int>& number) = 0;), the following sequence occurs:

Regular c'tor called (EXE)
set_from(9) called (EXE)
Square called (DLL) with a valid pod (Visual Studio shows the data pointer holding correct data)
genpfault
  • 51,148
  • 11
  • 85
  • 139
cf-
  • 8,598
  • 9
  • 36
  • 58
  • There's nothing wrong with the way you're using the heap. In fact, from your description, things are going wrong before any of the heap allocations are freed? However: I don't use C++ much, but it looks to me as if some of the code in the `pod` class might be compiled directly into the executable rather than calling the DLL. If a single `pod` object is being accessed by both the DLLs version and the EXEs version of the code, that would explain your problem. – Harry Johnston Mar 23 '14 at 21:19
  • @Harry Yep, things are going wrong before any memory is freed. The way problems occur is quite consistent: `set_from` is called, during which `data` has expected values; then `get` is called, during which `data` is corrupted; then `release` is called. Also, I'm actually expecting the EXE and DLL to operate on the same `pod`; the EXE sets up the pod and the DLL pulls it apart, then vice versa. I'm just not sure where `data` goes bad. – cf- Mar 23 '14 at 22:38
  • 1
    You can't operate on a class object from multiple compilers, because (for a start) there's no standard for the way the members are laid out in memory. If the EXE writes `data` starting at the fourth byte of the object and the DLL expects it to start at the eighth byte, boom! That's almost certainly why you're getting the wrong value for `data`; you're not reading it from where it was written. – Harry Johnston Mar 23 '14 at 22:54
  • 1
    @Harry As far as I'm aware, the class's layout should be standard. [The `offsetof` docs](http://www.cplusplus.com/reference/cstddef/offsetof/) state it works on a standard-layout class, and `std::is_standard_layout` returned true when I tested it on a `pod`. – cf- Mar 23 '14 at 23:10
  • Also see my discussion with @deGoot in the comments of [this related question](http://stackoverflow.com/questions/22306541/repackaging-binary-data-in-a-dll-how-does-this-work) – cf- Mar 23 '14 at 23:14
  • Do both compilers support C++11? I'd suggest you double-check whether the layout is actually the same or not: for example, you could create a static `std::int32_t **` and set it to the address of the `data` pointer in set_from(), then in get() you can explicitly compare it to what it thinks the address is. – Harry Johnston Mar 23 '14 at 23:58
  • If the value of `data` is actually being overwritten, it may be possible to set a data checkpoint in the debugger to locate the point at which it changes. – Harry Johnston Mar 23 '14 at 23:59
  • @Harry Both compilers support C++11. As of right now, I'm building the DLL with Visual Studio 2013 and the test EXE with GCC 4.8.1. Using the `std::int32 **` test you suggested, `dataCheckPtr == &data` in `set_from`, but `dataCheckPtr != &data` in `get`. But oddly enough `offsetof(pod, data);` returns 0 from both the EXE side and DLL side. This seems self-contradictory, but I'm probably missing something obvious... – cf- Mar 24 '14 at 02:35
  • It should be easy to find out on which side (if either) `offsetof` is behaving as expected, just check whether `&data == &pod` or not. I don't think VS2013 is fully C++11 compatible. In particular, see this: http://msdn.microsoft.com/en-us/library/hh438475.aspx – Harry Johnston Mar 24 '14 at 20:06
  • 1
    @computerfreaker: You're passing `pod` by value to the `Add` function, which will invoke the copy constructor of your `pod`. Maybe passing the data by reference will help (i.e. `virtual pod Add(const pod& number1, const pod& number2);`). Otherwise, the problem may be with the data copy workflow. Also, for the sake of analysis, I suggest you try the approach with an inputs-only function first before trying inputs and outputs to determine where the problem is. – deGoot Mar 24 '14 at 23:32
  • @Harry `if (reinterpret_cast(&data) == reinterpret_cast(this))` comes out true no matter where I check it. That's a good sanity check, though, since the contradictory results from the pointer check and the `offsetof` check were making me question everything. – cf- Mar 25 '14 at 03:57
  • @deGoot _Maybe passing the data by reference will help_ That's actually it. Wow. I adjusted my method signature as you suggested and the method gets inputs successfully now. Apparently my copy c'tor needs some love. (This is also confirmed by the method getting *inputs* successfully when passed by reference, but crashing when trying to *return* a `pod` value.) – cf- Mar 25 '14 at 04:31
  • And now that I'm testing more, the copy c'tor never actually gets called on either side. This gets odder and odder. When passing arguments to `Add`, the regular c'tor (`pod(const original_type& value)`) is called for both arguments, then the `()` operator is called for both arguments, and spits out a bad `data` pointer. – cf- Mar 25 '14 at 13:50
  • Keep in mind that (if I've understood correctly) now that you've added a virtual function it is no longer a standard-layout class. – Harry Johnston Mar 25 '14 at 19:58
  • @Harry Nope, the virtual function is part of the DLL. I left out most of the DLL's structure because it's not relevant to the question, but think of `Square` as a a parallel of `Add`. I'll edit my question again to clear that up (edit: done). The `pod` class hasn't changed, I just wanted an extra test method in the DLL so I could focus entirely on one input parameter instead of two inputs and an output. – cf- Mar 25 '14 at 20:01
  • Is the binary protocol for passing a class object by value standardized? To be honest at this point I don't think you're going to work out what's happening without stepping through the assembly code for `CCDLL_lib->Square(a)`. – Harry Johnston Mar 25 '14 at 22:53
  • @Harry I'm not sure if there's a _standard_ protocol for passing a class object by value, but attempting to logic it out seems sound enough. `&data == &pod`, so there's no debug information at the start of the class. `#pragma pack(push, 1)` guarantees no extra padding in the class. And the class is standard-layout, so the class is always laid out the same way. Therefore, the class is fixed in size and all class members (should just be `data` anyway) are fixed in position. The only abnormality I could see would be `&pod` being incorrectly placed; I'll have to check the asm as you suggest. – cf- Mar 26 '14 at 02:02
  • 1
    Yes, at this point I'd hazard a guess that the object is being read from the wrong address. (You could confirm by getting the constructor to report the value of `this` and Square to report the value of `&number`.) Another thought: if this is 64-bit code, by any chance does the problem go away if you have more than one pointer stored in `pod` objects? – Harry Johnston Mar 26 '14 at 03:53
  • @Harry Your guess seems to be correct. I'm still working on figuring out exactly how far off the class pointer ends up, but `this` in the copy c'tor and `&number` in `Square` are definitely different. I guess it's time to try and figure out some asm. And while this is 32-bit code, a second pointer does in fact come out with the correct value. (The `dataCheckPtr` I used for checking `data`'s consistency earlier pointed to the correct location of `data` even after `data` itself was corrupted.) – cf- Mar 26 '14 at 15:00

0 Answers0