Francis Lord Francis Lord - 2 months ago 19
C++ Question

C++ Segmentation fault at std::map::insert

I'm trying to teach myself C++ (actually I should say re-learn, but I first learned it when I didn't know a thing about coding and year ago so it doesn't count) and I'm doing my first project after finishing the online tutorial. I figured since I had a good C# and VB.Net background I might as well try something a bit bigger, but not too big. Before I start, I'm using Code::Blocks as my IDE and the default compiler in that IDE (I believe it's MinGW). So here is my thing : I have a ChromaTest project (this is using the Razer Chroma SDK for those wondering about the name) which is a console app, and a ChromaTestDLL project, which is (you guessed it) a DLL (I decided to do a DLL to learn how to do so at the same time and because I might use some the code in a GUI project later on). Problem is I'm getting a Segmentation Fault error when trying to insert into a map. Here is the relevant code :

In the ChromaTestDLL Project

MyChroma.h (Header for the MyChroma class)

#ifndef MYCHROMA_H
#define MYCHROMA_H

#include <map>
#include <windef.h>
#include "RzChromaSDKTypes.h"
#include <string>
#include "Template.h"

#ifdef BUILD_DLL
#define DLL_EXPORT __declspec(dllexport)
#else
#define DLL_EXPORT __declspec(dllimport)
#endif

using namespace std;

#ifdef __cplusplus
extern "C"
{
#endif

class DLL_EXPORT MyChroma
{
public:
MyChroma();

bool Init();

std::map<char, COLORREF> GetColorMapping();

void SetColorMapping(char key, COLORREF color);

void AssignToKeyBoard();

void SetColorFromString(string s, COLORREF color);

~MyChroma();

protected:
std::map<char, COLORREF>* _ColorMapping;

ChromaSDK::Keyboard::RZKEY KeyFromChar(char keyChar);

My_Chroma_Implementation* Chroma;

private:
};

#ifdef __cplusplus
}
#endif

#endif // MYCHROMA_H


MyChroma.cpp (Relevant implementation for MyChroma class)

#include "MyChroma.h"
#include "Template.h"
#include <iostream>

MyChroma::MyChroma()
{
_ColorMapping = new std::map<char, COLORREF>();
}

std::map<char, COLORREF> MyChroma::GetColorMapping() { return *_ColorMapping; }

void MyChroma::SetColorMapping(char key, COLORREF color){

if (_ColorMapping->count(key) == 0)
_ColorMapping->insert(std::make_pair(key, color)); //This where the error happens
else
(*_ColorMapping)[key] = color;
}

MyChroma::~MyChroma() {
delete Chroma;
delete _ColorMapping;
}

//Other implementations omitted


In the ChromaTest project

MyChroma.h (Header to import MyChroma class, slightly different from the one in ChromaTestDll, basically it only contains public members)

#ifndef MYCHROMA_H
#define MYCHROMA_H

#include <map>
#include <windef.h>
#include <string>

#ifdef BUILD_DLL
#define DLL_EXPORT __declspec(dllexport)
#else
#define DLL_EXPORT __declspec(dllimport)
#endif

using namespace std;

#ifdef __cplusplus
extern "C"
{
#endif

class DLL_EXPORT MyChroma
{
public:
MyChroma();

bool Init();

std::map<char, COLORREF> GetColorMapping();

void SetColorMapping(char key, COLORREF color);

void AssignToKeyBoard();

void SetColorFromString(string s, COLORREF color);

~MyChroma();

};

#ifdef __cplusplus
}
#endif

#endif // MYCHROMA_H


Main.cpp (main app code)

#include <iostream>
#include "MyChroma.h"

#include <wingdi.h>

using namespace std;

int main()
{
MyChroma test = MyChroma();
bool result = test.Init();

cout << (result ? "Initialized\n" : "Failed to initialize Razer Chroma");

cout << "Setting color";

if (result){
test.SetColorMapping('a', RGB(255,0, 0)); //This call goes in the DLL where I said it failed earlier.
test.SetColorMapping('a', RGB(0,0,255));
}

return 0;
}


Sorry for the atrociously long code (please tell me if there are things I could have removed here). Can anybody spot any mistake in there I would not be surprised this would be linked to pointers, this is probably the concept which took me the most time to understand. At first I didn't put the map in a pointer and on the heap, but changing another variable to that earlier seemed to have fixed another problem so I figured I'd give it a try. Sadly I had pretty much the same errors when not putting the map on the heap too.

On a side note, can anybody explain to me what different between the heap and the stack, why would I want to go through the (risky) hassle of storing variables on the heap (with pointers and deletion and all) instead of on the stack, and when should I use the heap or when should I not.

Answer

Based on the information in your question:

  1. The compiled code in your DLL appears to declare a MyChroma class containing a bunch of internal class members, in its header file.

  2. Then your main application uses a completely different header file, that defines a class called MyChroma, stripped of its class members.

  3. Then, your main application instantiates the MyChroma class, based on what it sees in its header files.

That's not going to work. Since your main application knows nothing about those class members, the actual class that it instantiates is too small.

And it instantiates a class on the stack.

And then the constructor comes from the DLL, which thinks the class contains all those other class members.

And the constructor in the DLL attempts to initialize them.

On the stack.

Hello stack corruption.

The answer here is simply "don't do what you did". This is undefined behavior. Everything that you compile that references a particular class must see an identical declaration (and inline method definitions) of the class.

Full stop.

No exceptions.

Well, with sufficient experience, it's possible to do something like this safely, when targetting a specific C++ implementation, but this is not the case here.

Until then, there are ways to hide the internal implementation details of library-provided classes, but this is not how you do it. The safe way to do it is with the PIMPL design pattern.

A few other thing you should not do, as well. This doesn't relate directly to the problem at hand, but this will avoid several other common pitfalls that can, without advance warning, pull the rug from under your feet:

  1. Do not use use namespace std;. Especially in header files. Completely forget that something like that exists in the C++ language.

  2. All your classes should also follow the Rule Of Three.