Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
naalit opened this issue Dec 12, 2022 · 1 comment · May be fixed by #477
Open

Comments

@naalit
Copy link

naalit commented Dec 12, 2022

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:

  • OS: Arch Linux
  • Compiler: GCC 12.2.0
  • Empirical Version: current master, but I originally found this problem in Symbulation with 1fe7f9008c
@naalit
Copy link
Author

naalit commented Dec 19, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant