CODEWITHSUNDEEP
c++sdl-2address-sanitizer
Amanda
Amanda

Reputation: 331

SDL_LockTexture() / AddressSanitizer / double-free

My project makes extensive use of SDL2-2.0.8 to display data frames from various scientific imaging cameras. My actual project is using wxWidgets 3.1.1 and SDL_CreateWindowFrom(xid), not SDL_CreateWindow().

I have recently started using AddressSanitizer to help debug my application and find possible memory leaks (Valgrind is way too slow for my application). AddressSanitizer has informed me of a serious memory leak which I am struggling to fix. What follows is a stand-alone fully compilable example which illustrates my problem. 

#include <iostream>
#include <unistd.h>
#include <random>

#include <SDL2/SDL.h>

int main()
{

   SDL_Window *window = nullptr;
   SDL_Renderer* renderer = nullptr;
   SDL_Texture *texture = nullptr;

   uint8_t *pixels = new uint8_t[640 * 480 * 3];

   // Random Numbers
   std::mt19937 rng;
   rng.seed(std::random_device()());
   std::uniform_int_distribution<std::mt19937::result_type> random(0, 255);

   int pitch;

   if ((window = SDL_CreateWindow("Test", 0, 0, 640, 480, 0)) == nullptr)
   {
      std::cerr << SDL_GetError() << "\n";
      return -1;
   }

   if ((renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED)) == nullptr)
   {
       std::cerr << SDL_GetError() << "\n";
       return -1;
   }

    // Create the SDL Texture
   if ((texture = SDL_CreateTexture(renderer, SDL_PIXELFORMAT_RGB24, SDL_TEXTUREACCESS_STREAMING, 640, 480)) == nullptr)
   {
       std::cerr << SDL_GetError() << "\n";
       return -1;
   }

   int counter = 0;

   while (counter < 500)
   {
       SDL_LockTexture(texture, nullptr, (void**) &pixels, &pitch);

       // Create greyscale noise - a bit like old television sets without an antenna
       for (int n = 0; n < 640 * 480 * 3; n += 3)
       {
          int random_pix = random(rng);
          pixels[n] = random_pix;
          pixels[n + 1] = random_pix;
          pixels[n + 2] = random_pix;
       }

       SDL_UnlockTexture(texture);
       SDL_RenderCopy(renderer, texture, NULL, NULL);
       SDL_RenderPresent(renderer);

       counter++;
}

// If the following line is uncommented and delete [] pixels commented, then I get a double-free in SDL
//free(pixels);

    SDL_DestroyTexture(texture);
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);

    // This line causes a double-free error, if it's commented out then I get a memory leak of 921600 bytes
    delete[] pixels;

   atexit(SDL_Quit);

   return 0;
}

It would seem that SDL is attempting to deallocate the memory assigned to my pixel buffer but is failing and causing a memory leak. Interestingly, if I use:

SDL_UpdateTexture(texture, NULL, pixels, m_Width * 3);

Then I can deallocate pixels myself and so fix the memory leak. Does anybody know what is going on? Is this merely a false positive from AddressSanitizer?

More information: My project is written in C++ and compiled with GCC-8 on Fedora 28. AddressSanitizer is from the standard Fedora repositories. I know that many of you will think that I should be using smart pointers, but to do so would require a significant refactor of my project which I simply don't have time for.

Many thanks for reading and I appreciate any help offered.

Amanda

Upvotes: 1

Views: 724

Answers (1)

Brad Allred
Brad Allred

Reputation: 7534

Notice the signature of SDL_LockTexture() takes a pointer to a pointer. This is a hint that you shouldn't be passing your own buffer. You are leaking because SDL_LockTexture() is changing the value of the pointer whos address you are passing to point to its own internal buffer and now you cannot delete your (uselessly) allocated buffer. Obviously then the double free is happening because you are deleting a buffer that does not belong to you that was already freed when you called SDL_DestroyTexture() (probably, I'm not sure of the exact implementation).

Also be aware that the data may not actually be in the format you expect. SDL_CreateTexture() is free to ignore your suggestion and give you the "closest" matching format supported by the backend. You need to query the actual format of the texture to know how to deal with the pixels.

As noted in the comments: you must use SDL_UpdateTexture() if you wish to supply your own pixels in your own format, but this will of course be much slower.

A note about address sanitizer: it does not produce false positives. if it reports an issue you do actually have a problem.

Upvotes: 3

Related Questions