6

Assume that in my code I have to store a void* as data member and typecast it back to the original class pointer when needed. To test its reliability, I wrote a test program (linux ubuntu 4.4.1 g++ -04 -Wall) and I was shocked to see the behavior.

struct A
{
  int i;
  static int c;
  A () : i(c++) { cout<<"A() : i("<<i<<")\n"; }
};
int A::c;

int main ()
{
  void *p = new A[3];  // good behavior for A* p = new A[3];
  cout<<"p->i = "<<((A*)p)->i<<endl;
  ((A*&)p)++;
  cout<<"p->i = "<<((A*)p)->i<<endl;
  ((A*&)p)++;
  cout<<"p->i = "<<((A*)p)->i<<endl;
}

This is just a test program; in actual for my case, it's mandatory to store any pointer as void* and then cast it back to the actual pointer (with help of template). So let's not worry about that part. The output of the above code is,

p->i = 0
p->i = 0 // ?? why not 1
p->i = 1

However if you change the void* p; to A* p; it gives expected behavior. WHY ?

Another question, I cannot get away with (A*&) otherwise I cannot use operator ++; but it also gives warning as, dereferencing type-punned pointer will break strict-aliasing rules. Is there any decent way to overcome warning ?

Community
  • 1
  • 1
iammilind
  • 68,093
  • 33
  • 169
  • 336
  • Using g++ 4.5.1, clang 3.0.127530, and Visual C++ 2010 SP1, the program outputs { 0, 1, 2 }. What compiler are you using that yields the unexpected result? – James McNellis Jun 11 '11 at 04:34
  • I can corroborate James McNellis' results. I get `0, 1, 2` on VC++ 2005 and VC++ 2008 compilers. As far as I can see there's nothing obviously wrong with your code. What compiler are you using? – In silico Jun 11 '11 at 04:35
  • I am using g++ 4.4.1 and have also linked results on ideone (which also I guess uses g++). I didn't tagged as g++ because of other question (type punned warning). – iammilind Jun 11 '11 at 04:36
  • 1
    @trutheality: No. You cannot perform arithmetic with a `void*`. – James McNellis Jun 11 '11 at 04:49
  • @trutheality Can't do arithmetic on a void pointer but this works when taking a trip to char pointer land (pointers to char types are allowed to alias, too). – Luc Danton Jun 11 '11 at 04:50
  • yeah I should have tested it before asking. – trutheality Jun 11 '11 at 04:51
  • @Luc: The pointed-to data can be aliased, yes, but the bytes underlying a `void*` cannot be reinterpreted as a `char*` without violating the strict aliasing rule. However, the OP could use a `char*` instead of a `void*` and get the correct result; it would just take a few more `static_cast`s to get right. – James McNellis Jun 11 '11 at 04:52
  • 1
    indeed `p=sizeof(A)+(char*)p;` works as expected and with no warnings [on ideone](http://ideone.com/AbvqF). – trutheality Jun 11 '11 at 04:53
  • @trutheality: Well, presumably `p = (A*)p + 1;` would also yield no errors. – James McNellis Jun 11 '11 at 04:58
  • @trutheality, should have been an answer :) – iammilind Jun 11 '11 at 04:59
  • @James I suppose I should have explained that a correct trip to char pointer land takes proper precautions. – Luc Danton Jun 11 '11 at 05:00
  • @James, is your method as safe as, the `template` one you suggested in your answer ? – iammilind Jun 11 '11 at 05:00
  • @James: indeed it does work. Would that make it the best solution for the OP? – trutheality Jun 11 '11 at 05:01
  • @iammilind: I actually completed my test after DXM already posted it as an answer. – trutheality Jun 11 '11 at 05:03
  • @iammilind: The code in my last comment is exactly what the code in my `advance_pointer_as` function template does. The advantage of the function template is that it is readable and (somewhat) less easy to screw up. – James McNellis Jun 11 '11 at 05:03
  • 2
    Using g++ 4.3, 4.4, or 4.5, this code outputs `0 0 1` when compiled at `-O2` or higher; when compiled at `-O1` or lower, it outputs `0 1 2`. – Adam Rosenfield Jun 11 '11 at 05:09
  • @Adam, correct I always do compilation at -04. But don't know how it will affect the results !! – iammilind Jun 11 '11 at 05:14
  • 1
    Apparently the platform matters too; on Win32, g++ 4.5.1 with `-O4` yields the expected { 0, 1, 2 } @Adam. – James McNellis Jun 11 '11 at 05:17
  • "it's mandatory to store any pointer as void* and then cast it back to the actual pointer (with help of template)" - possibly then the problem is that in your code you used `(A*&)`, that is you cast it back *without* the help of the template, in violation of your coding rules. Or possibly you used the template to perform that cast, and the template's documentation permits it, in which case the problem is that it shouldn't because it leads to type-punning. – Steve Jessop Jun 11 '11 at 08:52
  • @Steve, all these problems are happening inside a `template` code only. I showed here just to demonstrate the possible minimum example. However, I **was** doing the same thing inside the code; i.e. somewhat like, `template T& operator ++ () { ((T*&)p)++ ; return *this; }` – iammilind Jun 11 '11 at 09:01
  • @iammilind: OK, so the problem is just that your `operator++` should do what James's `advance_pointer_as` does. – Steve Jessop Jun 11 '11 at 09:04

3 Answers3

11

Well, as the compiler warns you, you are violating the strict aliasing rule, which formally means that the results are undefined.

You can eliminate the strict aliasing violation by using a function template for the increment:

template<typename T>
void advance_pointer_as(void*& p, int n = 1) {
    T* p_a(static_cast<T*>(p));
    p_a += n;
    p = p_a;
}

With this function template, the following definition of main() yields the expected results on the Ideone compiler (and emits no warnings):

int main()
{
    void* p = new A[3];
    std::cout << "p->i = " << static_cast<A*>(p)->i << std::endl;
    advance_pointer_as<A>(p);
    std::cout << "p->i = " << static_cast<A*>(p)->i << std::endl;
    advance_pointer_as<A>(p);
    std::cout << "p->i = " << static_cast<A*>(p)->i << std::endl;
}
James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • wouldn't it increase the machine instruction for simple `++` operation (or we have to rely on optimizations happening) ? – iammilind Jun 11 '11 at 04:46
  • 7
    A slow, correct program is far superior to a fast, incorrect program. – James McNellis Jun 11 '11 at 04:47
  • @iammilind There's no way to know unless you measure & compare for yourself. – Luc Danton Jun 11 '11 at 04:48
  • 2
    My sarcastic comment notwithstanding, I would expect a compiler to inline this even at low optimization levels, so I would not expect it to have significant performance costs. – James McNellis Jun 11 '11 at 04:57
  • have you checked MSVC with maximum optimization flags on ? Because I have kept `-O4`. As @Adam suggested in comments, even `g++` outputs correctly for lower optimizations. – iammilind Jun 11 '11 at 05:16
  • Visual C++ has no `-O4` (from my limited understanding, Visual C++ does not heavily take advantage of the strict aliasing rule for optimization purposes, at least not in current versions). This should yield the expected result regardless of optimization settings because it does not violate the strict aliasing rule (nor does it violate any other rule of which I am aware, disregarding the fact that the dynamically allocated `A[3]` is leaked, but that is not of concern here). – James McNellis Jun 11 '11 at 05:20
6

You have already received the correct answer and it is indeed the violation of the strict aliasing rule that leads to the unpredictable behavior of the code. I'd just note that the title of your question makes reference to "casting back pointer to the original class". In reality your code does not have anything to do with casting anything "back". Your code performs reinterpretation of raw memory content occupied by a void * pointer as a A * pointer. This is not "casting back". This is reinterpretation. Not even remotely the same thing.

A good way to illustrate the difference would be to use and int and float example. A float value declared and initialized as

float f = 2.0;

cab be cast (explicitly or implicitly converted) to int type

int i = (int) f;

with the expected result

assert(i == 2);

This is indeed a cast (a conversion).

Alternatively, the same float value can be also reinterpreted as an int value

int i = (int &) f;

However, in this case the value of i will be totally meaningless and generally unpredictable. I hope it is easy to see the difference between a conversion and a memory reinterpretation from these examples.

Reinterpretation is exactly what you are doing in your code. The (A *&) p expression is nothing else than a reinterpretation of raw memory occupied by pointer void *p as pointer of type A *. The language does not guarantee that these two pointer types have the same representation and even the same size. So, expecting the predictable behavior from your code is like expecting the above (int &) f expression to evaluate to 2.

The proper way to really "cast back" your void * pointer would be to do (A *) p, not (A *&) p. The result of (A *) p would indeed be the original pointer value, that can be safely manipulated by pointer arithmetic. The only proper way to obtain the original value as an lvalue would be to use an additional variable

A *pa = (A *) p;
...
pa++;
...

And there's no legal way to create an lvalue "in place", as you attempted to by your (A *&) p cast. The behavior of your code is an illustration of that.

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • This is a nice explanation. I'm not sure about "no legal way to create an lvalue in place," though: `(char&)p` is equivalent to `*reinterpret_cast(&p)`, which is valid and doesn't violate strict aliasing, and effectively "creates an lvalue in place." – James McNellis Jun 11 '11 at 06:00
  • @James: I read that as meaning no legal way to create an lvalue "in place" *with type `A*`*, which is what the questioner attempted and hence what AndreyT is talking about. Types `char`, `unsigned char`, and anything else that's compatible with the effective type of the object referenced, sure. And I suppose for another nitpick, I think it's valid to create the lvalue, it's just not valid to assign to it, convert it to rvalue, or otherwise access it ;-) – Steve Jessop Jun 11 '11 at 08:46
2

As others have commented, your code appears like it should work. Only once (in 17+ years of coding in C++) I ran across something where I was looking straight at the code and the behavior, like in your case, just didn't make sense. I ended up running the code through debugger and opening a disassembly window. I found what could only be explained as a bug in VS2003 compiler because it was missing exactly one instruction. Simply rearranging local variables at the top of the function (30 lines or so from the error) made the compiler put the correct instruction back in. So try debugger with disassembly and follow memory/registers to see what it's actually doing?

As far as advancing the pointer, you should be able to advance it by doing:

p = (char*)p + sizeof( A );

VS2003 through VS2010 never give you complaints about that, not sure about g++

DXM
  • 4,413
  • 1
  • 19
  • 29