Member Avatar for Labdabeta

I have made a header file for SDL GUI applications. I was testing the SDLGame::Draw() function when I got a SigSegV fault. The fault occurs in different locations each time I run the program. Also sometimes instead of a SigSegV fault I get a SigTrap fault. Here is the code involved with the SigSegV.
SDLGame class constructor:

SDLGame::SDLGame(int w, int h, int bpp, Uint32 flags)
{
    if (SDL_Init(SDL_INIT_EVERYTHING)==-1)
        exit(-1);
    Window=SDL_SetVideoMode(w, h, bpp, flags);
    if (Window==NULL)
        exit(-2);
}

SDLGame class Draw():

void SDLGame::Draw(Sprite img, int x, int y)
{
    SDL_Rect offset;
    offset.x=x;
    offset.y=y;
    SDL_BlitSurface((SDL_Surface*)img, NULL, Window, &offset);
}

Sprite class constructor (from file):

Sprite::Sprite(char *filename)
{
    SDL_Surface *Temp=IMG_Load(filename);
    if (Temp!=NULL)
    {
        SDL_Surface *Good=SDL_DisplayFormatAlpha(Temp);
        pixels=new Pixel32[Good->w*Good->h];
        w=Good->w;
        h=Good->h;
        pixels=(Pixel32*)Good->pixels;
        SDL_FreeSurface(Temp);
        //SDL_FreeSurface(Good);
    }
    else
    {
        pixels=NULL;
        w=0;
        h=0;
        SDL_FreeSurface(Temp);
    }
}

Sprite class SDL_Surface typecast operator:

Sprite::operator SDL_Surface*()const
{
    SDL_Surface *ret=NULL;
    ret=SDL_CreateRGBSurface(SDL_SWSURFACE, w, h, 32, 0x00FF0000, 0x0000FF00, 0x000000FF, 0xFF000000);
    Pixel32 *surface=(Pixel32*)ret->pixels;
    for (int i=0; i<(w*h); i++)
    {
        if (!pixels[i]||!surface[i])
        {
            i=(w*h);
            break;
        }
        surface[i]=pixels[i];
    }
    SDL_Surface *dup=ret;
    ret->refcount++;
    return dup;
}

Pixel32 is essentially typedef'd as Uint32. This is the basis of my main() function (but not it entirely, I think the problem lies in the fact that I draw a surface twice):

int main()
{
SDL_Game TTT(600, 600, 32, SDL_SWSURFACE);
Sprite Board("BOARD.png");
TTT.Draw(Board, 0, 0);
TTT.Step();
TTT.Draw(Board, 0, 0);
TTT.Step();
return 0;
}

If I need to post the whole code I can. I just don't wish to right now. Could anybody solve this problem without more code?

  • 2 Contributors
  • 4 Replies
  • 894 Views
  • 4 Days Discussion Span
  • Latest Post Latest Post by Labdabeta

Dani AI

Generated

This is a classic ownership/copying bug: the Sprite ends up pointing at memory it does not own (or that gets freed), and passing the Sprite by value into Draw makes a temporary copy whose destructor invalidates that shared memory. That explains the nondeterministic SIGSEGV/SIGTRAP after the blit. was correct that copying is part of the problem; ’s idea of removing the destructor is unsafe and will only hide undefined behavior.

Fix checklist (apply one ownership model and be consistent):

  • Stop passing Sprite by value. Change the draw prototype to accept a reference:

    void SDLGame::Draw(const Sprite& sprite, int x, int y);

  • Decide ownership:

    • If Sprite should own raw pixels, allocate your own buffer and copy the pixels out of any temporary SDL_Surface. Do not set your pixels pointer to surface->pixels unless the Sprite also owns that SDL_Surface for its full lifetime.
    • If Sprite should own an SDL_Surface, store an SDL_Surface* surf in the Sprite and free it in the destructor. Do not manually poke at surf->refcount.

  • Implement correct copy semantics (Rule of Three/Five) or disable copying. Example deep-copy skeleton (replace names to match your class):

    struct Sprite {
  Pixel32* data;
  int w, h;

  ~Sprite() { delete [] data; }

  Sprite(const Sprite& o) : w(o.w), h(o.h) {
    size_t n = (size_t)w * h;
    data = new Pixel32[n];
    memcpy(data, o.data, n * sizeof(Pixel32));
  }

  Sprite& operator=(const Sprite& o) {
    if (this == &o) return *this;
    delete [] data;
    w = o.w; h = o.h;
    size_t n = (size_t)w * h;
    data = new Pixel32[n];
    memcpy(data, o.data, n * sizeof(Pixel32));
    return *this;
  }
};

  • Avoid manually incrementing SDL_Surface internals (like refcount++). If you create temporary SDL_Surfaces for blitting, free them after use. If you cache a converted surface in Sprite, reuse and free it in the destructor.

Troubleshooting tips: run under Valgrind or AddressSanitizer to find double-free or use-after-free, and reproduce with a debug build. Fixing ownership/copying will remove the intermittent crashes; deleting the destructor only postpones a hard-to-find memory corruption.

Recommended Answers

All 4 Replies

Member Avatar for rxlim

Do you get the same problem if you debug the application?

Member Avatar for Labdabeta

Yes, it usually happens at the end of the conversion from Sprite to SDL_Surface *, although occasionally it happens at the Draw function. I think that somehow the Sprite is getting delete[]d after it's pixel surface is blitted, I just can't see why.

Member Avatar for rxlim

Are you deleting anything in the destructor of Sprite? If so, then you will invalidate the original Sprite object's corresponding pointer, unless you have a copy constructor that takes this into account? You are passing your sprite by value, so if this is your problem, then perhaps you should pass it by reference instead.

Member Avatar for Labdabeta

THANK YOU! I knew it would be something stupid with the pointers, come to think of it, it fails after the blitting, but that is also the last line, where the sprite goes out of scope! I'm gonna just try deleting the deconstructor, so what if I have some deallocated memory, it will be overwritten later.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.