Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: sf::Image destructor causes SIGABRT crash  (Read 4694 times)

0 Members and 1 Guest are viewing this topic.

drummerp

  • Newbie
  • *
  • Posts: 18
    • View Profile
sf::Image destructor causes SIGABRT crash
« on: October 24, 2013, 07:36:35 pm »
Hello all! I'm trying to create a Skybox object, which when loaded loads a single image file and splits it into 5 or 6 (depending on if there's a bottom image) sf::Image objects, represented as a std::vector<sf::Image>. I then pass the image data (from image.getPixelsPtr() ) to glTexImage2D and call a number of other OpenGL functions to set up my textures for rendering.

My issue comes at the end of my function: when the vector is cleared off the stack, it results in a crash dump to stderr and then a SIGABRT crash. If I run it in my debugger, it becomes apparent it's resulting from in the sf::Image destructor. Here's my code (with some irrelevant portions removed):

bool Skybox::loadSkybox() {
    glGenTextures(6, ids);

    //Loads the image file, then splits into 5-6 images using sf::Image::copy, returning vector of images
    //returns a list of skybox images, in order: Top, Bottom?, Left, Right, Front, Back
    const std::vector<sf::Image> images = Image->getAsSkybox();
    if (images.empty() || images.at(0).getSize().x == 0) //Empty list or empty images is an error with loading
        return false;

    for (unsigned int i = 0; i < images.size(); i++) {

        glBindTexture(GL_TEXTURE_2D, ids[i]);

        GLuint width = images.at(i).getSize().x, height = images.at(i).getSize().y;
        unsigned char* image = const_cast<unsigned char*>(images.at(i).getPixelsPtr());
        if (image == NULL || width <= 0 || height <= 0) {
            std::err << "Error loading image file." << std::endl;
            glDeleteTextures(6, ids);
            return false;
        }

        glTexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);

        GLenum err;
        if ((err = glGetError()) != GL_NO_ERROR) {
            std::err << "OpenGL error in loading image number " << i << ": " << gluErrorString(err) << std::endl;
            glDeleteTextures(6, ids);
            return false;
        }

        //Call other OpenGL functions to set it up

        free(image); //Clear our temporary image data
    }

    return true;
}

I'm using SFML2,  Code::Blocks for my IDE and GCC 4.6 on a 32-bit on an Ubuntu 12.04 LTS environment.
« Last Edit: October 24, 2013, 07:51:57 pm by drummerp »

binary1248

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1405
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::Image destructor causes SIGABRT crash
« Reply #1 on: October 24, 2013, 07:57:06 pm »
unsigned char* image = const_cast<unsigned char*>(images.at(i).getPixelsPtr());
...
free(image); //Clear our temporary image data
What the... never ever use free in your own C++ code. Here you kind of asked for it by doing something really dumb. The pointer returned from getPixelsPtr() is const for a reason. If you cast away the constness, and free the memory allocated by the underlying object yourself, you can't complain you get all sorts of undefined behaviour. In this case, it results in a double free when the sf::Image destructors are called.

Rule #1 of C dynamic memory management: For every malloc() there must be a free(), and vice versa.
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: sf::Image destructor causes SIGABRT crash
« Reply #2 on: October 24, 2013, 07:57:32 pm »
Fix:
// free(image); //Clear our temporary image data

Quote
What the... never ever use free in your own C++ code.
What if I'm biding C to c++ and things might get allocated or freed in libs with cstdlib functions? :'( :'( :'( :'(
« Last Edit: October 24, 2013, 08:00:18 pm by FRex »
Back to C++ gamedev with SFML in May 2023

binary1248

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1405
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::Image destructor causes SIGABRT crash
« Reply #3 on: October 24, 2013, 08:02:49 pm »
What if I'm biding C to c++ and things might get allocated or free in libs with cstdlib functions? :'( :'( :'( :'(
I said in your own C++ code. If your code is C then there is no other way. If you want to use a library that makes you call free() on objects that malloc() themselves instead of providing a cleanup function for them, then... just use another library.
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: sf::Image destructor causes SIGABRT crash
« Reply #4 on: October 24, 2013, 08:11:12 pm »
The memory inside sf::Image doesn't belong to you. In modern C++, memory (and other resources) always has a clear owner -- if ownership is transferred, then this is done explicitly, usually through an encapsulated object or a smart pointer. In this respect, SFML is well-designed and uses the RAII idiom to relieve the user from the burden of memory management. That in turn implies that there are no implicit cases in SFML where you have to interfere with raw memory.

And as binary1248 mentioned, don't use const_cast unless you're absolutely sure what you're doing. It is almost only necessary when you interact with code that is not const-correct, i.e. bad code.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

drummerp

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: sf::Image destructor causes SIGABRT crash
« Reply #5 on: October 24, 2013, 08:13:01 pm »
unsigned char* image = const_cast<unsigned char*>(images.at(i).getPixelsPtr());
...
free(image); //Clear our temporary image data
What the... never ever use free in your own C++ code. Here you kind of asked for it by doing something really dumb. The pointer returned from getPixelsPtr() is const for a reason. If you cast away the constness, and free the memory allocated by the underlying object yourself, you can't complain you get all sorts of undefined behaviour. In this case, it results in a double free when the sf::Image destructors are called.

Rule #1 of C dynamic memory management: For every malloc() there must be a free(), and vice versa.

Ah, wow. I'd love to chalk that one up to a lack of sleep or something like that, but all I can really say is that I just pretty well screwed that up. But I understand my problem at least.

Anyway, I removed the call to free(), and that solved the problem. Behavior's now as expected. Thank you for the help.

drummerp

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: sf::Image destructor causes SIGABRT crash
« Reply #6 on: October 24, 2013, 08:15:13 pm »
And as binary1248 mentioned, don't use const_cast unless you're absolutely sure what you're doing. It is almost only necessary when you interact with code that is not const-correct, i.e. bad code.

I'm uncertain how else I'm supposed to approach this: glTexImage2D requires an unsigned char*, but getPixelsPtr() returns a const unsigned char*, and the two are not interchangeable -- trying to pass the return value of getPixelsPtr() directly to glTexImage2D without the cast results in an error. I'm probably going about this wrong, or otherwise not seeing something obvious, but I'm unsure what else I should do here.

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: sf::Image destructor causes SIGABRT crash
« Reply #7 on: October 24, 2013, 08:22:14 pm »
Quote
What the... never ever use free in your own C++ code.
/*
 * File:   main.cpp
 * Author: superprogrammer
 *
 * Created on October 24, 2013, 2:08 PM
 */


#include <iostream>
#include <cstdlib>
#include <cstring>

int main()
{
char * buffer=std::malloc(100);
std::strcpy(buffer,"Hello Modern C++");
std::cout<<buffer<<std::endl;
//std::free(buffer) // - commented cus binary1248 said to never use free in C++
}
;D

Quote
glTexImage2D requires an unsigned char*
Not for me on my Linux and not in docs of GL. Both take const void * and const pointer of any kind should be implicitly convertible to that(= what is the error message and how is that function declared for you??). Even if it wasn't const, casting away const doesn't mean that you can free pointers you didn't malloc...
« Last Edit: October 24, 2013, 08:24:24 pm by FRex »
Back to C++ gamedev with SFML in May 2023

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: sf::Image destructor causes SIGABRT crash
« Reply #8 on: October 24, 2013, 08:23:58 pm »
glTexImage2D requires an unsigned char*
No, it does not. It expects const GLvoid*:
void glTexImage2D( GLenum target,  
  GLint level,  
  GLint internalFormat,  
  GLsizei width,  
  GLsizei height,  
  GLint border,  
  GLenum format,  
  GLenum type,  
  const GLvoid * data);

binary1248's message concerning malloc/free was pretty clear: Don't use it in C++. Of course there are exceptional cases, but they are extremely rare, and completely irrelevant for this thread.
« Last Edit: October 24, 2013, 08:28:35 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: sf::Image destructor causes SIGABRT crash
« Reply #9 on: October 24, 2013, 09:04:47 pm »
For added comedic value I just wanted to run "image+freeing it's ptr" code with debugging on* to make a point of running debugger first and asking for help later :P and the call stack is more or less yelling profanities at me for touching that pointer that belongs to std::vector in sf::Image pointer, as seen on attached picture.
The added comedic value comes from the fact that free requires a cast to get rid of const because it takes a void * unlike GL that takes const void *... sooo.. does the error for const cast come from GL function or not? It's bug, mistake or very weird compiler config if it does.

*admittedly it's Fedora + CodeLite but since Ubuntu/Mint and C::B are apparently 'beginner friendly' in popular opinion I can only assume that on them the output is just as helpful as here. ;D
Back to C++ gamedev with SFML in May 2023

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 10901
    • View Profile
    • development blog
    • Email
AW: sf::Image destructor causes SIGABRT crash
« Reply #10 on: October 24, 2013, 10:33:15 pm »
I've no experience in OpenGL so I might not get what you're doing, but why don't you just use an sf::Texture and call sf::Texture::bind(...) instead of the detour through sf::Image?

@FRex: Can please either start posting useful/non-trolling/non-conspiracy-theory stuff or just not post at all?
If you find C sooooo cool then stick with it, but don't start off-topic discussions whenever possible.
Thank you!
« Last Edit: October 24, 2013, 10:35:50 pm by eXpl0it3r »
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

drummerp

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: sf::Image destructor causes SIGABRT crash
« Reply #11 on: October 25, 2013, 04:12:53 am »
It's strange that it doesn't require a const. I never use a const_cast unless a compile error needs me to. But you are right, it's a const GLvoid*, so the const_cast is unnecessary. I corrected that, and it does still work. Thank you all for your help.

 

anything