8

Good evening everyone.
A code snippet will be worth a thousand words :

// Storage suitable for any of the listed instances
alignas(MaxAlign<Base, Derived1, Derived2>::value)
char storage[MaxSize<Base, Derived1, Derived2>::value];

// Instanciate one of the derived classes using placement new
new (storage) Derived2(3.14);


// Later...

// Recover a pointer to the base class
Base &ref = *reinterpret_cast<Base*> (storage);

// Use its (virtual) functions
ref.print();

// Destroy it when we're done.
ref.~Base();

As you can see, I want to access the instance only through its base class, and without actually storing the base class pointer. Note that in the second part, the Derived2 type information will be lost, so I'm left with only storage and the guarantee that one derived instance is in it.

As placement new never adjusts the destination pointer, this boils down to using reinterpret_cast to upcast to a base class. Now I know that this is dangerous, since the more appropriate static_cast adjusts the pointer in some cases. [1]

And indeed, it triggers Undefined Behaviour. You'll find the full code here on Coliru (g++ 4.9.0), where it promptly crashes at runtime. Meanwhile on my PC (g++ 4.8.2), everything is fine. Note that on g++ 4.9, outputting both pointers before calling the functions shows identical values... and works.

So I tried to take the problem backwards : nudging the derived instance so that a pointer to Base will be equal to storage.

void *ptr = static_cast<Derived2*>(reinterpret_cast<Base*>(storage));
new (ptr) Derived2(3.14);

No luck. The thing still runs fine on g++ 4.8, and falls in flames on g++ 4.9.

Edit : come to think of it, the above isn't that smart. Because, what if the derived instance ends up before storage... Ouch.

So my question is : is there a solution to what I'm trying to achieve ? Or, are the cases mentioned at [1] sufficiently well-defined that I can write code that will work with a subset of classes (like, no virtual inheritance) ?

Quentin
  • 62,093
  • 7
  • 131
  • 191
  • The parts you label as dangerous indeed are completely fine, I can't figure out why the virtual call is crashing http://coliru.stacked-crooked.com/a/87e0f6f4138f8083 – Mooing Duck Jul 24 '14 at 18:13
  • 3
    You could just retrieve the `Base*` from the `new` expression, and store it for later use - `Base *ref = new (storage) Derived2(3.14);` - this avoids explicit casts. The code crashes even with this change, which I think might be due to [this bug](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60965). – Praetorian Jul 24 '14 at 18:17
  • @Praetorian: I checked, crash remains even using that pointer, with only one derived class, no reference casts... – Mooing Duck Jul 24 '14 at 18:20
  • @Praetorian I specifically don't want to store the base pointer (see first paragraph under my code snippet). I already tried it, and of course it works fine, but doesn't satisfy me. Thanks for the link though, I'll dig that way. – Quentin Jul 24 '14 at 18:20
  • 1
    @Praetorian: Good call, that bug looks like it matches this behavior perfectly. – Mooing Duck Jul 24 '14 at 18:23
  • It does, but the bug is marked as fixed in g++ 4.9.0. Seems it's not... – Quentin Jul 24 '14 at 18:24
  • @Quentin I don't understand the version numbers listed there either. The last few entries with the code commits, and the "Fixed" comment are from early May, while gcc-4.9.0 was released at the end of April [according to Wikipedia](https://en.wikipedia.org/wiki/GNU_Compiler_Collection). – Praetorian Jul 24 '14 at 18:31
  • @Praetorian I guess I'll try to set a g++ snapshot up on my PC and see if the crash subsists. – Quentin Jul 24 '14 at 18:40
  • Tested this out on MinGW with gcc4.9.1 and both your code and mine (where you store `Base *`) work. Your version does emit a type-punning warning. – Praetorian Jul 24 '14 at 18:59
  • @Praetorian that confirms the bug. The type-punning warning is mandatory, since I don't cast to `void*` in-between. Any advice about upcasting with `reinterpret_cast` (my [1]) ? – Quentin Jul 24 '14 at 19:03
  • Not really, other than don't do it :) I think the `reinterpret_cast` could fail when dealing with multiple inheritance, and even a C-style cast might be better for this case, since it'll do any necessary pointer adjustment. But I'm not sure of that last part. Best option is to store the `Base *`. – Praetorian Jul 24 '14 at 19:07
  • @Praetorian C-style cast won't do either, since I won't have the derived type. I guess I'll drop in some `static_assert`s to ensure the pointers are identical, and roll with it. You should post an answer about the g++ bug so I can accept it. – Quentin Jul 24 '14 at 19:10
  • I would've if I knew more about the dos and don'ts of the `reinterpret_cast` part. You should consider adding the `c++` tag, it'll get you a wider audience. – Praetorian Jul 24 '14 at 19:17
  • Even if your particular class hierarchy never needs pointer adjustment, an implementation is free to detect UB and make your program crash, just because it can. – n. m. could be an AI Jul 24 '14 at 21:08
  • @n.m. it's free to do it, however I don't think any implementer will purposely design code to break technically correct programs. And have people using his compiler. – Quentin Jul 24 '14 at 21:14
  • Programs with UB are not "technically correct" – M.M Jul 25 '14 at 02:00
  • UB never happens in a correct C++ program. Compilers can and do optimize away entire paths that provably always lead to UB. – n. m. could be an AI Jul 25 '14 at 04:52
  • The program is "technically correct" in the sense that the pointer does point to the right place, even though it was obtained by "unorthodox" means. That's not really different from casting to and from `void*`, or even `std::uintptr_t` : you pop a type out of your sleeve, cast the pointer and use it happily ever after. If I `static_assert` that the `Base` pointer is indeed equal to `storage`, it will always be the case, and even though it is UB according to the standard, it would be a great deal of work for the compiler to track it down and break it. – Quentin Jul 25 '14 at 07:53
  • Why not just `reinterpret_cast` (to the type you actually created) and then `static_cast` to take care of any pointer adjustment that may be required? The code that calls `new` must know the specific derived type it's constructing, so it shouldn't be a problem for that code to `reinterpret_cast` to that type immediately afterward. – Wyzard Jul 25 '14 at 14:07
  • @Wyzard this is just an example, in my real class code before and after the `// Later...` comment are in different methods. By then, I don't have the derived type anymore, just the guarantee that there is _some_ derived instance in there (see first paragraph after snippet). – Quentin Jul 25 '14 at 14:10
  • Store an extra offset (from buffer to base*)? The issue is that you do not in general have a way to reconstruct base* from a pointer to any derived class. You could rely on such offsets being fixed (do not see where standard guarantees this, but in practice...), and either hard code them or cheat to generate them at compile time. – Yakk - Adam Nevraumont Jul 26 '14 at 15:14
  • @Yakk that makes sense. Were the offsets fiwed, a little TMP could calculate them, and hard-code them automatically. Hmm. – Quentin Jul 26 '14 at 16:16

1 Answers1

3

I've just modified your code a bit, it seems that reinterpret_cast is not the problem. The minimum code that could replicate this error is as such

Coliru link: http://coliru.stacked-crooked.com/a/dd9a633511a3d08d

#include <iostream>

struct Base { 
    virtual void print() {}
};

int main(int, char**) {
   Base storage[1];
   storage[0].print();

   std::cout <<"Succeed";
   return 0;
}

The sufficient conditions to trigger this error are

  1. the "storage" variable is allocated on the stack as an array

  2. the print() must be a virtual method

  3. the compiler option should be -O2

If you use -O1, the program compiles and runs without a problem.

Besides, this error seems to only show up when compiled with g++ 4.9.0. It runs fine if compiled with VS2012/2013 or g++ 4.7.2 (you can test it on http://www.compileonline.com/compile_cpp_online.php)

Judging from the above, I think this may be a compiler specific problem.

Note: The actual output of the program given in this answer is different from the OP's. It does not show Segmentation Fault. However, when run successfully, it should print "Succeed", which is not shown when it is run on the Coliru.

Edit: Modified the code to replicate the error. No derived class needed.

Hsi-Hung Shih
  • 233
  • 1
  • 7
  • 4
    Even if you forget `Derived` and replace it by `Base` in `main`, you still get a segmentation fault with `g++ -O2` (http://coliru.stacked-crooked.com/a/839f941954a7edad). Not with `clang++ -O2`. Seems like a g++ bug. – Marc van Leeuwen Jul 27 '14 at 22:02