Turning on g++ optimization causes segfault - I don't get it

Question

I've been working on my program, and I decided to turn on some optimizations using g++ -O3. Suddenly, my program started segfaulting. I've hunted the problematic code down, and minimized my program to something that still segfaults (only when using level 3 optimizations). I was hoping someone could take a quick peek at the code (I tried minimizing it as much as possible):

// src/main.cpp
#include "rt/lights/point.hpp"
int main(int argc, char **argv)
{
    rt::Light *light = new rt::light::Point(alg::vector(.0f, 5.0f, 5.0f), rt::Color(1.0f), .5f);
    return 0;
}


// include/rt/lights/point.hpp
#ifndef RT_LIGHT_POINT_HPP_
#define RT_LIGHT_POINT_HPP_

#include "rt/accelerator.hpp"
#include "rt/color.hpp"
#include "rt/intersection.hpp"
#include "rt/light.hpp" // abstract

namespace rt {
namespace light {

class Point : public Light
{
  public:
    Point(alg::vector pos, Color color, float intensity) : Light(intensity * color), pos(pos) {}

    Color get_contrib(const Intersection&, const Accelerator&, const alg::vector& toViewer) const;

  private:
    alg::vector pos;
};

} // namespace light
} // namespace rt

#endif


// include/rt/light.hpp
#ifndef RT_LIGHT_HPP_
#define RT_LIGHT_HPP_

#include "algebra/vector.hpp"
#include "rt/color.hpp"

namespace rt {

class Intersection;
class Accelerator;

class Light
{
  public:
    Light(Color intensity) : intensity(intensity) {}

    virtual Color get_contrib(const Intersection&, const Accelerator&, const alg::vector& toViewer) const = 0;

    Color get_intensity() const {return intensity;}

  protected:
    Color intensity;
};

} // namespace rt

#endif

I would love some insight on why this code only segfaults when using optimizations, and how to stop it from doing so. Thanks!

$ find src/ -name "*.cpp" | xargs g++ -I include/ -O3
$ ./a.out
Segmentation fault

Edit: By request, the constructors for alg::vector

struct vector
{
    float x, y, z;
    vector() : x(.0f), y(.0f), z(.0f) {}
    explicit vector(float f) : x(f), y(f), z(f) {}
    vector(float x, float y, float z) : x(x), y(y), z(z) {}
    // ...

Edit2: Adding gdb output when compiling with -g

(gdb) file a.out
Reading symbols from /home/rob/devel/gbug/a.out...done.
(gdb) run
Starting program: /home/rob/devel/gbug/a.out 

Program received signal SIGSEGV, Segmentation fault.
rt::light::Point::Point (this=0x804b008, pos=..., color=..., intensity=0.5)
    at src/rt/lights/point.cpp:13
13  Point::Point(alg::vector pos, Color color, float intensity) : Light(intensity * color), pos(pos)
(gdb) bt
#0  rt::light::Point::Point (this=0x804b008, pos=..., color=..., intensity=0.5)
    at src/rt/lights/point.cpp:13
#1  0x08048898 in main (argc=1, argv=0xbffff3e4) at src/main.cpp:5

Edit3: Sources for rt::Color.

// include/rt/color.hpp
#ifndef RT_COLOR_HPP_
#define RT_COLOR_HPP_

#include "algebra/vector.hpp"

namespace rt {

/*******************************************************************************
 * CLASS DEFINITION
 */

struct Color
{
    float r, g, b;
    Color() : r(.0f), g(.0f), b(.0f) {}
    explicit Color(float f) : r(f), g(f), b(f) {}
    Color(float r, float g, float b) : r(r), g(g), b(b) {}

    Color& operator+= (const Color&);
    Color& operator*= (const Color&);
    Color& operator*= (float);
};

/*******************************************************************************
 * MEMBER OPERATORS
 */

inline Color& Color::operator+= (const Color& other)
{
    r += other.r;
    g += other.g;
    b += other.b;
    return *this;
}

inline Color& Color::operator*= (const Color& other)
{
    r *= other.r;
    g *= other.g;
    b *= other.b;
    return *this;
}

inline Color& Color::operator*= (float f)
{
    r *= f;
    g *= f;
    b *= f;
}

/*******************************************************************************
 * ADDITIONAL OPERATORS
 */

inline Color operator+ (Color lhs, const Color& rhs)
{
    return lhs += rhs;
}

inline Color operator* (Color lhs, const Color& rhs)
{
    return lhs *= rhs;
}

inline Color operator* (Color c, float f)
{
    return c *= f;
}

inline Color operator* (float f, Color c)
{
    return c *= f;
}

} // namespace rt

#endif

Answer 1

When calculating intensity * color, indirectly this operator is called:

inline Color& Color::operator*= (float f)
{
    r *= f;
    g *= f;
    b *= f;
}

It claims to return a reference to a Color, but doesn't. It should return a reference to *this, like the other operators do:

return *this;

Answer 2

Time to learn how to debug with gdb!

Recompile all the source code with -g:

find src/ -name "*.cpp" | xargs g++ -I include/ -O3 -g

Then run gdb, load your file, and execute it:

gdb
file a.out
run

gdb will bring you to a command prompt when your program hits the segfault. At the (gdb) prompt type "bt" and hit enter. It will give you the stack trace. The first frame of the stack will be the line of code that caused the segmentation fault.

From there if it's obvious fix it otherwise add the output to your question. Sometimes gdb isn't great at debugging code that's in constructors but try it first and see what it says.

Answer 3

You must realize that it's not the optimization that's breaking your code, the code is already broken. I can't quite see what is going on just by looking at those chunks but seeing as though you are segfaulting on a new statement, I would try and direct my efforts toward checking the input parameters of your new functions. Amongst the many things that happens during an 03 optimization is that the compiler will try to inline function calls, unroll your loops, and create new variables as well as get rid of ones in order to accelerate your execution. As a first step, double check anywhere you have loops and make sure if you're doing something like i < strlen(str) that the statement is not crazy and assert that the input parameters that should not be NULL are in fact not NULL.

Hope this helps.

Answer 4

I'm not sure if this is the issue you are seeing, but if you have a class with virtual functions that is used polymorphically, it should have a virtual destructor:

class Light {
   ...
   virtual ~Light {}
   ...
};

Currently, if you would delete the light variable in your main, the ~Light destructor would be called (since the variable has type Light*), instead of the correct ~Point one. Making the destructor virtual fixes this.

Answer 5

There might be some confusion using the member variable pos and the passed in variable pos, try naming them different variable names.

Point(alg::vector pos, Color color, float intensity) : Light(intensity * color), pos(pos) {}