devosoft / Empirical

A library of tools for scientific software development, with emphasis on also being able to build web interfaces using Emscripten.
Other
86 stars 38 forks source link

Pointer tracking fails to properly account for object address changes with (multiple) inheritance #476

Open naalit opened 1 year ago

naalit commented 1 year ago

Describe the bug Empirical's pointer tracking infrastructure assumes that object addresses don't change, so when an object is deleted it looks up its address in a table of allocated addresses. But C++ sometimes does change object addresses, specifically with multiple inheritance but this should also happen sometimes with particularly large single inheritance situations although I haven't found an example where that actually happens with single inheritance. Specifically, in the example below the compiler moves the pointer to C forward a little bit to get the B pointer, and the virtual destructor knows how to get the original pointer back but Empirical doesn't.

To Reproduce Here's a minimal example that fails with -DEMP_TRACK_MEM, but is valid C++ and passes AddressSanitizer:

#include "emp/base/Ptr.hpp"

class A {
public:
  virtual ~A() {}
};

class B {
public:
  virtual ~B() {}
};

class C : public A, public B {
  size_t c;
};

void cleanup(emp::Ptr<B> b) { b.Delete(); }

int main() {
  emp::Ptr<C> c = emp::NewPtr<C>();
  cleanup(c);
}

Expected behavior The above program doesn't actually contain any memory management errors (as far as I know, and AddressSanitizer agrees), so pointer tracking should not produce any.

Toolchain:

naalit commented 1 year ago

I just found out that dynamic_cast<void*>(ptr) will obtain the base address of the real object allocation, so using that in Ptr::Delete() and similar methods should hopefully fix this problem. I've made that change in PR #477.