c++ dangerous casting code

Question

I'm pretty sure this is dangerous code. However, I wanted to check to see if anyone had an idea of what exactly would go wrong.

Suppose I have this class structure:

class A {
protected:
  int a;
public:
  A() { a = 0; }        
  int getA() { return a; }
  void setA(int v) { a = v; }
};

class B: public A {
protected:
  int b;
public:
  B() { b = 0; }
};

And then suppose I want to have a way of automatically extending the class like so:

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

template <typename T>
class Test: public T, public Base {};

One really important guarantee that I can make is that neither Base nor Test will have any other member variables or methods. They are essentially empty classes.

The (potentially) dangerous code is below:

int main() {
  B *b = new B();

  // dangerous part?
  // forcing Test<B> to point to to an address of type B
  Test<B> *test = static_cast<Test<B> *>(b);

  //
  A *a = dynamic_cast<A *>(test);
  a->setA(10);
  std::cout << "result: " << a->getA() << std::endl;
}

The rationale for doing something like this is I'm using a class similar to Test, but in order for it to work currently, a new instance T (i.e. Test) has necessarily be made, along with copying the instance passed. It would be really nice if I could just point Test to T's memory address.

If Base did not add a virtual destructor, and since nothing gets added by Test, I would think this code is actually okay. However, the addition of the virtual destructor makes me worried that the type info might get added to the class. If that's the case, then it would potentially cause memory access violations.

Finally, I can say this code works fine on my computer/compiler (clang), though this of course is no guarantee that it's not doing bad things to memory and/or won't completely fail on another compiler/machine.

why not just use a `void*` if you need to pass around opaque pointers. It would make the meaning clear and avoid the risk of someone trying to perform an operation on `Test*`. — Jeffery Thomas, Dec 10 '12 at 22:59
The reason for not using `void *` is that I use `dynamic_cast` later in the code (as is done in `main`). Also, `Test` is only ever used internally, and can't be accessed from the outside. By the time a user gets access to it, it will either be a `T *`, or a pointer to a parent of `T`. — Abe Schneider, Dec 10 '12 at 23:17

score 5 · Accepted Answer · answered Dec 10 '12 at 23:23

The virtual destructor Base::~Base will be called when you delete the pointer. Since B doesn't have the proper vtable (none at all in the code posted here) that won't end very well.

It only works in this case because you have a memory leak, you're never deleting test.

score 1 · Answer 2 · answered Dec 10 '12 at 23:26

1

Your code produces undefined behaviour, as it violates strict aliasing. Even if it did not, you are invoking UB, as neither B nor A are polymorphic classes, and the object pointed to is not a polymorphic class, therefore dynamic_cast cannot succeed. You're attempting to access a Base object that does not exist to determine the runtime type when using dynamic_cast.

One really important guarantee that I can make is that neither Base nor Test will have any other member variables or methods. They are essentially empty classes.

It's not important at all- it's utterly irrelevant. The Standard would have to mandate EBO for this to even begin to matter, and it doesn't.

answered Dec 10 '12 at 23:26

Puppy

144,682
38
256
465

I would think in the non-polymorphic case, the fact that `Test` and `Base` don't add any thing would be important. I thought the c++11 standard had a specific layout of objects for the simple cases (and thus the macros to calculate the offset of variables). – Abe Schneider Dec 10 '12 at 23:35
Also, the dynamic_cast does succeed for me. – Abe Schneider Dec 10 '12 at 23:36
It's not, and it doesn't. That stuff goes out the window as soon as virtual functions are involved. Also, `Base` does not add nothing, it adds a virtual function. The fact that the `dynamic_cast` works for you is irrelevant. It is UB. Also, the aliasing makes it UB. – Puppy Dec 10 '12 at 23:36

score -1 · Answer 3 · answered Dec 10 '12 at 23:07

-1

As long as you perform no operations with Test<B>* and avoid any magic like smart pointers or automatic memory management, you should be fine.

You should be sure to look for obscured code like debug prints or logging that will inspect the object. I've had debuggers crash on me for attempting to look into a value of a pointer set up like this. I will bet this will cause you some pain, but you should be able to make it work.

I think the real problem is maintenance. How long will it be before some developer does an operation on Test<B>*?

answered Dec 10 '12 at 23:07

Jeffery Thomas

42,202
8
92
117

It's for a library, and Test will be completely hidden. It's only ever used when someone wrap's their class T in my code, so the user won't even see the class used.. I was going to turn on memory guards to see what would happen, but the issue is that even if it works on my machine, I'm worried someone else's compiler could do something very different. – Abe Schneider Dec 10 '12 at 23:15

c++ dangerous casting code

3 Answers3