2

Is this code causes undefined behavior? Or can i run into a problem with this? (copy the full class without functions, just variables with public modifier and modify the private memebers throw this pointer) example:

#include <iostream>

using namespace std;

class Point {

private:

    int x;
    int y;

public:

    Point(int x, int y) {
        this->x = x;
        this->y = y;
    }

    void Print() {
        cout << "(" << x << ", " << y << ")" << endl;
    }

};

struct PointHack {

    int x;
    int y;

};

int main() {
    Point a(4, 5);
    a.Print();
    ((PointHack *) (&a))->x = 1;
    ((PointHack *) (&a))->y = 2;
    a.Print();

    return 0;
}

output:

(4, 5)
(1, 2)

(with the original member order, of course)

fred
  • 23
  • 1
  • 4
  • 10
    That's the most undefined behaviour I have seen in `[c++]` questions today. – us2012 Oct 06 '13 at 13:45
  • I'm not sure, it may fall under the "standard layout classes" umbrella (or however they are called), and in that case "common initial subsequence" and stuff may be valid... – Matteo Italia Oct 06 '13 at 13:47
  • 1
    If you want to modify private members, then make a "setter" function to set the values. Or don't make them private to begin with. – Some programmer dude Oct 06 '13 at 13:48
  • @MatteoItalia, common initial subsequence applies only to structs inside an union. Not to pointer casting. – zch Oct 06 '13 at 13:50
  • 1
    The whole point of OOD is to have data encapsulation. If that does not float your boat then do not do it. – Ed Heal Oct 06 '13 at 13:52
  • @zch: correct, as for §9.2 ¶20; I was thinking that maybe there was some extension to ¶21, but it doesn't seem to be the case (and type punning via pointer casts is UB anyway due to aliasing problems). – Matteo Italia Oct 06 '13 at 13:56
  • 1
    @Joachim Pileborg, in this example i can't access the private members because they are read only, they get their value at the constructor and later i can't modify them. by the way this is not a real problem, it just came to my mind :) – fred Oct 06 '13 at 14:11

3 Answers3

5

Despite your classes being layout compatible (see below), your code exhibits undefined behavior due to the fact that such pointer casts are prohibited by the C++ strict aliasing rules1.

But: replacing the casts with a union makes the code standard-compliant; this is actually guaranteed to work in C++11:

#include <iostream>

using namespace std;

class Point {

private:

    int x;
    int y;

public:

    Point(int x, int y) {
        this->x = x;
        this->y = y;
    }

    void Print() {
        cout << "(" << x << ", " << y << ")" << endl;
    }

};

struct PointHack {
    int x;
    int y;
};

union pu
{
    Point p;
    PointHack ph;
    pu(int x, int y) : p(x, y) {}
};

int main() {
    pu u(4,5);
    u.p.Print();
    u.ph.x=1;
    u.ph.y=2;
    u.p.Print();
    return 0;
}

This comes from the fact that Point and PointHack are standard-layout classes2 (C++11, §9 ¶7), and share a "common initial subsequence" (§9.2, ¶20); as such, if they both are stored in the same union (here pu) it's permitted to "inspect the common initial part of any of them"3.

Still, this answer is mostly an exercise of style; don't exploit such tricks unless you are really forced to. C++ provides better means to access private members if necessary without brutally breaking the encapsulation - you have getters/setters, protected inheritance, friend classes, ... And in general, if you access private class members in ways not intended by your target class, you are potentially violating the assumptions of that class about how its data is modified, which can lead to erratic behavior of the code of its methods.

Notes:

  1. In C++ you can't have two pointers of unrelated types pointing to the same object; this restriction is mostly used to help optimizers with assumptions about aliasing.
  2. Notice that the requirements for this are quite stringent; typically most classes that aren't basically C structs don't qualify for this. Even having different access qualifiers can break the magic.
  3. The idea is that assigning to ph makes it the "active object" of the union, and then p.Print() is the one "inspecting" the "inactive" object.
Community
  • 1
  • 1
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
2

Yes, the posted code invokes undefined behavior. Don't do that.

If you want to see a really insane way that some people accomplish this unsavory goal, here you go: access private member using template trick

But really, don't do that.

P.S.: never use C-style casts in C++. Use static_cast, dynamic_cast, reinterpret_cast, and const_cast, as appropriate. C-style casts are needlessly unsafe.

Community
  • 1
  • 1
John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • But why? The two classes are not has the same memory map? – fred Oct 06 '13 at 13:56
  • 1
    @fred: the problem is not the memory layout (that indeed is guaranteed to be the same, since they are standard layout classes as for §9 ¶7), but that doing that kind of pointer casts violates the C++ no-aliasing requirements. – Matteo Italia Oct 06 '13 at 13:58
  • 1
    Because C++. If you do it, it may work on some compilers and not others, or with certain compiler options, or versions, or operating systems, or whatever. It may be more productive at this point to start a separate question about the actual goal you are trying to achieve, and we can try to find a solution that is valid C++ and won't make future maintenance programmers curse your name. – John Zwinck Oct 06 '13 at 13:59
0

i found the problem

/*code:*/
#include <stdio.h>

void check(short *h,long *k)
{
    *h=5;
    *k=6;
    if (*h == 5)
        printf("strict aliasing problem\n");
}

int main(void)
{
    long      k[1];
    check((short *)k,k);
    return 0;
}
/*
$ gcc -Wall -ansi -pedantic -O0 -o o0 asd.c
$ ./o0
/output: nothing/
$ gcc -Wall -ansi -pedantic -O2 -o o2 asd.c
$ ./o2
/output: strict aliasing problem/
*/

(same as in c++)

fred
  • 23
  • 1
  • 4