Why does g++ optimize out a critical section of the following code?

Question

The following code causes a crash in my program, because

void fractalizeSegment() {
        // Assume next != NULL
        double deltaX = next->x - x;
        double deltaY = next->y - y;

        // Add 3 new points labeled a1, a2, a3 from this to next
        Point a3(x + 2.0*deltaX/3.0, y + 2.0*deltaY/3.0, next);
        double sqr3 = std::sqrt(3.0);
        Point a2(x + deltaX/2.0 - sqr3*deltaY/2.0,
             y + deltaY/2.0 + sqr3*deltaX/2.0,
             &a3);
        Point a1(x + deltaX/3.0, y + deltaY/3.0, &a2);

        next = &a1;
    }

Is somehow optimized to

void fractalizeSegment() {
    next = &a1;
}

The method is called on p0 = {x = 0, y = 0, next = 0x7fffffffe100}, which point to p1 = {x = 1, y = 0, next = 0x0}.

By analyzing the program in the debugger I found that when I'm in the method fractalizeSegment:

a1 = {x = 6.9533558075099091e-310, y = 6.9533558075098597e-310, next = 0x7fffffffe190}

In address a1.next there is

a2 = {x = 6.9533558074508189e-310, y = 4.9406564584124654e-324, next = 0x34}.

Trying to deference (*a2.next).next causes a segmentation fault.

Why does g++ optimize my code like this? How can I prevent it?

The current workaround I found was printing out the values of a1, a2, and a3, this prevented the optimization.

Perhaps it does *not* assume that `next != NULL`? GCC very aggressively exploits undefined behavior when optimizing code like this. — Cody Gray - on strike, Oct 01 '17 at 10:27
Adding a if(next != NULL) around the code doesn't change the behavior — Sven, Oct 01 '17 at 10:30

rodrigo · Accepted Answer · 2017-10-01T11:25:51.833

7

a1 is a local automatic variable that will be destroyed upon returning from the function, so any use of *next after that will be undefined. The compiler thinks that surely you will not use those values so why bother computing them.

You probably want to create a1 in the heap:

next = new Point(x + deltaX/3.0, y + deltaY/3.0, &a2); //no need of a1.

Maybe also a2 and a3 should be dynamically allocated, but that depends on the implementation of Point, that you did not show.

And remember to delete the allocated objects somewhere.

edited Oct 01 '17 at 11:25

answered Oct 01 '17 at 10:30

rodrigo

94,151
12
143
190

"in the heap"? Surely "on the heap". – tambre Oct 01 '17 at 10:31
2

You’re really quibbling on “in” vs “on”? The latter is far more idiomatic than the former, which is much more accurate. – Dúthomhas Oct 01 '17 at 10:33
@tambre: Well, I'm not a native speaker, and I have a difficulty with _in_ vs _on_... A quick search on google reveals that _on_ is used about twice more than _in_ on that context (in that context?), but both are regularly used. Nevertheless, I think that the meaning is clear anyway. – rodrigo Oct 01 '17 at 10:36
@StoryTeller Yes, it was directed at **tambre**, but your analysis is exactly opposite: “in” and ”on” are indeed “former” and “latter”, because “on” is more idiomatic than the more accurate “in”. I am **disagreeing** with **tambre**’s giving **rodrigo** a hard time about chosing “in” over “on” — both work, and **rodrigo**’s usage is not incorrect. – Dúthomhas Oct 01 '17 at 11:00
@Cheersandhth.-Alf: About the `&a2` maybe it is incorrect, maybe not. That depends on whether `Point` keeps a reference to that passed point (as a linked list). If that is the case, then `&a3` is also wrong and all of them need to by dynamically allocated. If that pointer is only used as a reference and not kept, then all is fine. – rodrigo Oct 01 '17 at 11:22
@Cheersandhth.-Alf: But from the comments, probably `Point` is a linked list of points, so all of them should be _newed_. I've added a note about that. – rodrigo Oct 01 '17 at 11:26
An entertaining read on what an optimizing compiler could do with undefined behavior: https://blogs.msdn.microsoft.com/oldnewthing/20140627-00/?p=633 – Employed Russian Oct 01 '17 at 15:31

Why does g++ optimize out a critical section of the following code?

1 Answers1