1

Lately I have had a constant battle with a function causing a SEGFAULT randomly. After doing some extra work in trying to find out the problem, I have come up with the following:

All code posted via pastebin:

BUILD 1:This is the original code, it causes the following SEGFAULT (given after link) http://pastebin.com/huzcqnDA

SEGFAULT:

#0 6FC657AC libstdc++-6!_ZNKSs4_Rep12_M_is_leakedEv() (Z:\CPP Programming\CodeBlocks\MinGW\bin\libstdc++-6.dll:??)
#1 6FC89FDB libstdc++-6!_ZNSs4_Rep7_M_grabERKSaIcES2_() (Z:\CPP Programming\CodeBlocks\MinGW\bin\libstdc++-6.dll:??)
#2 6FC8C0E7 libstdc++-6!_ZNSsC1ERKSs() (Z:\CPP Programming\CodeBlocks\MinGW\bin\libstdc++-6.dll:??)
#3 0094A470 Carp::Sprite::Sprite(this=0x27fae8, s=...) (Z:/CPP Programming/Carperon/Source/Carp/Engine/StructL2.hpp:28)
#4 00944E98 Carp::Item::_spr(this=0x1277eb80) (Z:/CPP Programming/Carperon/Source/Carp/../Carp/Classes.hpp:59)
#5 00416219 Carp::WinBag::update(this=0x2857f8, o=false) (Z:\CPP Programming\Carperon\Source\Carp\Interface.cpp:60)
#6 00419304 Carp::GameUI::checkUpdate(this=0x2857e4) (Z:\CPP Programming\Carperon\Source\Carp\Interface.cpp:240)
#7 00401B7D Carp::GameApp::loopGame(this=0x2801ac) (Z:\CPP Programming\Carperon\main.cpp:35)
#8 00402145 _fu2041___ZSt4cout() (Z:\CPP Programming\Carperon\Source\Application.cpp:25)
#9 004017A9 main() (Z:\CPP Programming\Carperon\main.cpp:6)

BUILD 2:This is the current build, currently causes a compiler error, which gives me the idea that this might be the cause of the problem. http://pastebin.com/89gCjH5P

Error:

Z:\CPP Programming\Carperon\Source\Carp\Interface.cpp|57|error: no match for 'operator<<' in 'std::operator<< <std::char_traits<char> >((* &(& std::operator<< <std::char_traits<char> >((* &(& std::operator<< <std::char_traits<char> >((* &(& std::operator<< <std::char_traits<char> >((* & std::cout), ((const char*)"Item Info: ")))->std::basic_ostream<_CharT, _Traits>::operator<< <char, std::char_traits<char> >(a)), ((const char*)"\012ItemContainer: ")))->std::basic_ostream<_CharT, _Traits>::operator<< <char, std::char_traits<char> >(((const void*)((Carp::WinBag*)t|

When I call the getter function given in Character, What is actually happening? I am failing to see what the problem is here, and previous Q&As that I have found do not solve the problem, they only cause it to break another random function later on.

The best I can call this situation is a Heisenbug, since this only occurs when I am in DEBUG mode for an unrelated SEGFAULT somewhere else in the program.

The only possible help I have found is using const-correctness with my getters, only to bring the same exact SEGFAULT to the board again (wasting time into compiling).

P.S. My program has static linkage to Ogre3D, which causes me to have an average compiling time of 5 minutes (more than 7 if I change specific headers). So it will take a long time for me to post edits/results.

P.S. Carp::WinBag is the same as Carp::Interface given in the sample code (gave the wrong class name)

Extra Note: I have had this problem occur for 5 days straight on and off. My sanity can only take so much more of this...

SOLUTION: My situation has been caused from my own laziness somewhere else in the code: 

ItemPtr temp(new Item(*listItem[1].get()));
temp->spawnDrop(Coord3(fRAND(-1,1),2,fRAND(-1,1)));
dropList.push_back(temp);
temp.reset(new Item(*listItem[2].get()));
temp->spawnDrop(Coord3(fRAND(-1,1),2,fRAND(-1,1)));
dropList.push_back(temp);
temp.reset(new Item(*listItem[3].get()));
temp->spawnDrop(Coord3(fRAND(-1,1),2,fRAND(-1,1)));
dropList.push_back(temp);

With this, I created a pointer to a new object, BUT at the same time caused the old one to be lost (Memory leak anyone?). This caused all the problems I had later in the code, and writing this the RIGHT way will fix it.

I cannot believe that I did this again after such a long time, and worse NOT realize it... For anyone else that is unfortunate enough to assume this works please DON'T. It will cause endless and confusing stress for you :*

Molma
  • 171
  • 8

1 Answers1

2

Character doesn't make any guarantee that Items has been initialized to a pointer. When you call it, it simply returns a pointer. If that pointer has not been initialized (or has been initialized to a bad memory location), trying to access that pointer may cause a seg fault:

Character c;
Intem* items = c._items();//get uninitialized ptr in c
items[foo];//seg fault (maybe)

Of course, this isn't the only way that you can get a seg-fault in the call.

What is actually happening in your getter call is that you are taking a "this" pointer, applying an offset to the "this" pointer to find the "items" pointer, and returning that value. However, if your "this" pointer is invalid, then you can get a seg fault. So:

Character* c;//not inititialized
c->_items();//seg fault (maybe)

Can cause a seg fault all on its own.

However, seg faults don't always happen: if the pointer location happens to be to good memory you'll not see the seg fault, you'll just continue down into undefined behavior mode.

SO how on earth do you debug these things? Gotta admit, its a pain in the ass. It is one of the primary reasons people dislike C and C++, and I don't think most people here are going to go looking for it for you.

Most compilers in Debug mode will force uninitialized pointers to a value. Sometimes the value is in hexspeak (My favorite being 0xBADF00D). So look at your pointer values. Visual studio initializes pointers to 0xccccccccccccc.

However, the BEST way to avoid this type of problem is to make having uninitialized pointers impossible. Use vectors and references. When you have to use pointers, stick to smart pointers. Use an RAIIdesign patterns with your constructors and destructors. Follow the Rule of 3 (or Rule of 3-5 in c++11). You'll never (ok you'll "rarely") need to look for invalid values because you've made them hard to exist.

IdeaHat
  • 7,192
  • 18
  • 47
  • Alright. So with what you stated, and according to [this question](http://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11), It would make sense to either give all the constructors AND a destructor, or not have any of these? If so then what would I do with primitive types that have not been declared? Also would declaring the ctor/dtor as `=default` actually prevent that from happening? – Molma Dec 05 '13 at 20:07
  • If the class is in charge of any memory, it should have a destructor to clean up (aka the default destructor aint good enough). If it has a destructor to clean up, then copying the value is probably non-trivial, so you need to define the copy constructor. If the copy constructor is non-trivial, then copy assignment is non-trivial. And thus the rule of 3. Resource Aquisition is Allocation says that your constructor should allocate all nessisary data. Defining "ctor/dtor as =default" just uses the default constructor, but as stated above, the default aint good enough. – IdeaHat Dec 05 '13 at 20:14
  • I'm not sure what you mean by "primative types that have not been declared". Can you elaborate? – IdeaHat Dec 05 '13 at 20:15
  • I remember a while back before I rewritten my code, that I did not have any ctors/dtors. Instead I made a "defaultize" function, which took any variables (such as primitive types like `int` and `float`) and set them to a default value, usually like: `int x=0`. I did that because if I made my own ctors/dtors, I felt I would encounter later problems due to something not being properly initialized OR not being "defaultized" before use. This caused stress for bigger classes because I would have a completely sloppy mess of code just to make sure everything was being made correctly... – Molma Dec 05 '13 at 20:22
  • This also caused a ton of memory leaks throughout the program, which caused me to rewrite it to said version while also adding smart pointers. I also stated in the code that I am using smart pointers. – Molma Dec 05 '13 at 20:24
  • There are extremely rare occurences where I use a regular pointer. In the entire program, the only non-smart pointers I provided are `*Character::ItemContainer::items` like shown, a `SkillContainer` and `CommonStat` – Molma Dec 05 '13 at 20:25
  • All `Carp::Item`s are smart pointers, and the `Gorilla::Sprite` in the code is from a library therefore I cannot change it's initialization. (I also am not allocating it via `new`, the library does what it needs to in the background.) – Molma Dec 05 '13 at 20:26
  • Also I realized after re-reading your comment that there are in fact structures that do not mess with memory directly. Would at least having a parameter contructor be viable? Example: `Item::Item(string name,int amount,etc...)`? – Molma Dec 05 '13 at 20:35
  • Yes, having parametrized constructors are extremely viable, and having no default constructor can make sense if a non-inizialzed object wouldn't make sense. The rule of 3 only talks about the destructor, copy constructor, and copy assignment, it doesn't say anything about the constructor. RAII just says that you need to have all the resources to make a valid object when you instantiate it, but you can certainly parametrize it. And having a default constructor can work too, providing the idea of a "NULL" object makes sense (A NULL car doesn't make sense, but an empty list does, for example) – IdeaHat Dec 05 '13 at 20:43
  • However, I don't see how having a parametrized constructor for Item will initialize the pointer in Character. – IdeaHat Dec 05 '13 at 20:44
  • It won't. I only ask about the parameterized constructor for other classes/structures. I am going to need to run through my code and make adjutments accordingly then, this will take some time. – Molma Dec 05 '13 at 20:53
  • Also would Item being a pointer cause any problems since it is private and I am using a getter to get the Item's information? For example: would calling `myCharacter->getItem()->doSomething()` or `myCharacter->getItem()->getSprite().variable` cause any problems, where the Item's Sprite is private and the Character's Item is private? – Molma Dec 05 '13 at 20:55
  • @Molma The biggest problem (IMHO) with using a raw pointer as a member is that it is ambiguous to who owns it (AKA who should clean this up?). If Character owns it, you gotta remember to clean it up, and if you forget: memory leak city. OR you could use a std::unique_ptr and be done with it. – IdeaHat Dec 05 '13 at 21:02
  • Alright, I will take this into consideration when revamping the code. – Molma Dec 05 '13 at 21:09
  • WOW! Just...wow... I found the problem after extremely deep debugging with `cout`. I found that I was initializing the SAME smart pointer to 3 different objects. I will update my question with more info. – Molma Dec 05 '13 at 22:53