1

I have the following class.

Header:

#include <iostream>
#include <vector>
#include <regex>

#ifndef GENERATION_H
#define GENERATION_H
class Generation {
public:
    Generation(int x, int y);
    ~Generation() {
    }

    void setCell(int x, int y, bool alive);

    bool getCell(int x, int y);

    int surroundingLivingCells(int x, int y);

    int getX();

    int getY();

    static std::shared_ptr<Generation> parseRil(int x_i, int y_i, const std::string& ril);

private:
    int _x;
    int _y;

    std::vector<int> _born = {3};
    std::vector<int> _starve = {3, 5};

    std::vector<std::vector<int>> _data;    
};

#endif // GENERATION_H

Cpp:

#include <regex>

#include "generation.h"

Generation::Generation(int x, int y) : _x(x), _y(y) {
    _data.resize(y, std::vector<int>(x, 0));
}

void Generation::setCell(int x, int y, bool alive) {
    _data[y][x] = alive ? 1 : 0;
}

bool Generation::getCell(int x, int y) {
    return _data[y][x] == 1;
}

int Generation::surroundingLivingCells(int x, int y) {
    int c = 0;
    for (int yy=y-1;yy<=y+1;++yy) {
        for (int xx=x-1;xx<=x+1;++xx) {
            if (xx == x && yy == y)
                continue;

            if (xx>=0 && yy>=0 && xx < _x && yy < _y) {
                c += _data[yy][xx];
            }
        }       
    }
    return c;
}

int Generation::getX() {
    return _x;
}

int Generation::getY() {
    return _y;
}

std::shared_ptr<Generation> Generation::parseRil(int x_i, int y_i, const std::string& ril) {
    std::shared_ptr<Generation> g(new Generation(x_i, y_i));

    std::regex ril_regex("((\\d*)(\\w))([!$]*)");
    auto ril_begin = std::sregex_iterator(ril.begin(), ril.end(), ril_regex);
    auto ril_end = std::sregex_iterator();

    int x = 0;
    int y = 0;

    for (auto i = ril_begin; i != ril_end; ++i) {
        std::smatch ril_match = *i;

        int amount = ril_match[2] != "" ? std::stoi(ril_match[1]) : 1;
        bool alive = ril_match[3] == "o";

        for (int j=x; j<x+amount;++j) {
            g->setCell(j, y, alive);
        }
        x += amount;

        if (ril_match[4] == "$") {
            y += 1;
            x = 0;
        }
    }

    return g;
}

When I create the class with x=213, y=142 and ril and destruct the object, the program crashes:

std::shared_ptr<Generation> g(Generation::parseRil(x, y, ril));
g.reset();

I use the default destructor, no custom code there. It seems in parseRil something is going wrong, since creating and destructing the object with the contructor works just fine.

The stack:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6b6fe36 in _int_free () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff6b6fe36 in _int_free () from /usr/lib/libc.so.6
#1  0x000000000046175e in __gnu_cxx::new_allocator<int>::deallocate (this=0x6dd800, __p=0x6f3410)
    at /usr/include/c++/6.1.1/ext/new_allocator.h:110
#2  0x00000000004602f7 in std::allocator_traits<std::allocator<int> >::deallocate (__a=..., __p=0x6f3410, 
    __n=213) at /usr/include/c++/6.1.1/bits/alloc_traits.h:442
#3  0x000000000045eb0a in std::_Vector_base<int, std::allocator<int> >::_M_deallocate (this=0x6dd800, 
    __p=0x6f3410, __n=213) at /usr/include/c++/6.1.1/bits/stl_vector.h:178
#4  0x000000000045da87 in std::_Vector_base<int, std::allocator<int> >::~_Vector_base (this=0x6dd800, 
    __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/stl_vector.h:160
#5  0x000000000045cd35 in std::vector<int, std::allocator<int> >::~vector (this=0x6dd800, 
    __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/stl_vector.h:427
#6  0x000000000046181b in std::_Destroy<std::vector<int, std::allocator<int> > > (__pointer=0x6dd800)
    at /usr/include/c++/6.1.1/bits/stl_construct.h:93
#7  0x00000000004603f5 in std::_Destroy_aux<false>::__destroy<std::vector<int, std::allocator<int> >*> (
    __first=0x6dd800, __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:103
#8  0x000000000045ec4a in std::_Destroy<std::vector<int, std::allocator<int> >*> (__first=0x6dce70, 
    __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:126
#9  0x000000000045dc17 in std::_Destroy<std::vector<int, std::allocator<int> >*, std::vector<int, std::allocator<int> > > (__first=0x6dce70, __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:151
#10 0x000000000045cd6d in std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >::~vector (this=0x6bc668, __in_chrg=<optimized out>)
    at /usr/include/c++/6.1.1/bits/stl_vector.h:426
#11 0x000000000045c8f0 in Generation::~Generation (this=0x6bc630, __in_chrg=<optimized out>)
    at /home/amu/Code/github/GoMtL/src/generation.h:10
#12 0x0000000000484bfc in std::_Sp_counted_ptr<Generation*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (
    this=0x6bbe50) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:372
#13 0x000000000045dee4 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x6bbe50)
    at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:150
#14 0x000000000045cf67 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (
    this=0x7fffffffe408, __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:662
#15 0x000000000045c930 in std::__shared_ptr<Generation, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (
    this=0x7fffffffe400, __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:928
#16 0x0000000000485e55 in std::__shared_ptr<Generation, (__gnu_cxx::_Lock_policy)2>::reset (
    this=0x6a6bb0 <generation_current>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:1025
---Type <return> to continue, or q <return> to quit---
#17 0x0000000000485107 in init_game () at /home/amProgram received signal SIGSEGV, Segmentation fault.
0x00007ffff6b6fe36 in _int_free () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff6b6fe36 in _int_free () from /usr/lib/libc.so.6
#1  0x000000000046175e in __gnu_cxx::new_allocator<int>::deallocate (this=0x6dd800, __p=0x6f3410)
    at /usr/include/c++/6.1.1/ext/new_allocator.h:110
#2  0x00000000004602f7 in std::allocator_traits<std::allocator<int> >::deallocate (__a=..., __p=0x6f3410, 
    __n=213) at /usr/include/c++/6.1.1/bits/alloc_traits.h:442
#3  0x000000000045eb0a in std::_Vector_base<int, std::allocator<int> >::_M_deallocate (this=0x6dd800, 
    __p=0x6f3410, __n=213) at /usr/include/c++/6.1.1/bits/stl_vector.h:178
#4  0x000000000045da87 in std::_Vector_base<int, std::allocator<int> >::~_Vector_base (this=0x6dd800, 
    __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/stl_vector.h:160
#5  0x000000000045cd35 in std::vector<int, std::allocator<int> >::~vector (this=0x6dd800, 
    __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/stl_vector.h:427
#6  0x000000000046181b in std::_Destroy<std::vector<int, std::allocator<int> > > (__pointer=0x6dd800)
    at /usr/include/c++/6.1.1/bits/stl_construct.h:93
#7  0x00000000004603f5 in std::_Destroy_aux<false>::__destroy<std::vector<int, std::allocator<int> >*> (
    __first=0x6dd800, __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:103
#8  0x000000000045ec4a in std::_Destroy<std::vector<int, std::allocator<int> >*> (__first=0x6dce70, 
    __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:126
#9  0x000000000045dc17 in std::_Destroy<std::vector<int, std::allocator<int> >*, std::vector<int, std::allocator<int> > > (__first=0x6dce70, __last=0x6ddbc0) at /usr/include/c++/6.1.1/bits/stl_construct.h:151
#10 0x000000000045cd6d in std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >::~vector (this=0x6bc668, __in_chrg=<optimized out>)
    at /usr/include/c++/6.1.1/bits/stl_vector.h:426
#11 0x000000000045c8f0 in Generation::~Generation (this=0x6bc630, __in_chrg=<optimized out>)
    at /home/amu/Code/github/GoMtL/src/generation.h:10
#12 0x0000000000484bfc in std::_Sp_counted_ptr<Generation*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (
    this=0x6bbe50) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:372
#13 0x000000000045dee4 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x6bbe50)
    at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:150
#14 0x000000000045cf67 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (
    this=0x7fffffffe408, __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:662
#15 0x000000000045c930 in std::__shared_ptr<Generation, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (
    this=0x7fffffffe400, __in_chrg=<optimized out>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:928
#16 0x0000000000485e55 in std::__shared_ptr<Generation, (__gnu_cxx::_Lock_policy)2>::reset (
    this=0x6a6bb0 <generation_current>) at /usr/include/c++/6.1.1/bits/shared_ptr_base.h:1025
---Type <return> to continue, or q <return> to quit---
#17 0x0000000000485107 in init_game () at /home/amu/Code/github/GoMtL/src/main.cpp:65
#18 0x0000000000485577 in main (argc=1, argv=0x7fffffffe928)
    at /home/amu/Code/github/GoMtL/src/main.cpp:118

However, when I choose small x and y, like 18 and 12, the program works just fine. What is the problem here?

Thanks!

amuttsch
  • 1,254
  • 14
  • 24
  • 1
    You forgot the most interesting part: `Generation`'s destructor code. – TerraPass Jul 25 '16 at 17:14
  • ... and, I don't think that the constructor *actually* does what you *think* it does! ## I assure you, "small X and Y" is a *red herring.* – Mike Robinson Jul 25 '16 at 17:14
  • @TerraPass: I don't have destructor code. It's the default one. – amuttsch Jul 25 '16 at 17:15
  • @amuttsch I see, you should put this piece of information into the question then. – TerraPass Jul 25 '16 at 17:16
  • @MikeRobinson The data inside the `_data` container looks good. The `size()` of `_data` is correct too. – amuttsch Jul 25 '16 at 17:17
  • http://stackoverflow.com/help/mcve – melpomene Jul 25 '16 at 17:21
  • @melpomene I added some more code. It seems `parseRil` breaks something. – amuttsch Jul 25 '16 at 17:27
  • 2
    I notice neither `parseRil` nor `setCell` seems to check for an out of bounds condition. – aschepler Jul 25 '16 at 17:36
  • @aschepler yes you are right, but an out of bounds condition would crash the problem prior to the destructor, right? – amuttsch Jul 25 '16 at 17:39
  • 1
    Nope, you can't count on Undefined Behavior crashing your program immediately. It could also seem to run fine for hours and then give a wrong answer, or crash much later. – aschepler Jul 25 '16 at 17:42
  • @aschepler So I added a simple `if (x > _x || y > _y) { return; }` to `setCell` and it doesn't crash anymore. Can you elaborate why the program doesn't crash when I try to access an element that is out of bounds? – amuttsch Jul 25 '16 at 17:48
  • 1
    probably garbage ints from a previous allocation block – ascotan Jul 25 '16 at 17:59
  • 1
    @amuttsch Speed. You aren't supposed to go out of range with a vector and the vast majority of times you won't, so why penalize all of the well-formed programs with a test they don't need? More here: http://stackoverflow.com/questions/30181568/why-doesnt-c-detect-when-out-of-range-vector-elements-are-accessed-with-the and here: http://stackoverflow.com/questions/1239938/accessing-an-array-out-of-bounds-gives-no-error-why. If you want a crash, [use `std::vector::at`](http://en.cppreference.com/w/cpp/container/vector/at) and do not catch the thrown exception. – user4581301 Jul 25 '16 at 18:10
  • Thanks for your comments and explanation. With them I was able to find the initial issue and fixed it, the solution is in the original post. – amuttsch Jul 25 '16 at 18:15
  • Excellent! Maybe you should "Answer" the question also, so that it will now be flagged as "having an answer." (Future souls who face a similar problem might be looking for ... "answers.") – Mike Robinson Jul 25 '16 at 18:53
  • amuttsch Putting an answer in the question is frowned on here. I recommend moving your answer to an... Err... What @MikeRobinson just said. – user4581301 Jul 25 '16 at 18:54
  • Okay, I moved it. It was a special problem indeed, but sometimes you need ideas from the outside...especially if you are starting to code in a new language. Thanks again for your help! :) – amuttsch Jul 25 '16 at 19:06

1 Answers1

0

Okay, I had multiple issues with the code. First, setCell didn't check the bounds, which were violated. A simple if (x > _x || y > _y) return fixed the problem. (Thanks @ascotan)

But why did we have out of bounds issues? It turns out the ril parameter had newline characters in it \r. This caused the regex to malfunction and didn't recognize some new row $ characters if they are the first one after a new line. That caused x to overflow. After removing them, the out of bounds condition didn't occur again and the program doesn't crash anymore.

amuttsch
  • 1,254
  • 14
  • 24