SFML community forums

Help => Graphics => Topic started by: netrick on July 23, 2013, 01:57:29 pm

Title: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 01:57:29 pm
Lubuntu 13.04 64 bit, g++ 4.7, SFML2 from github (very recent build).

#include <SFML/Graphics.hpp>

int main()
{
sf::Texture tex;
text.loadFromFile("data/");  //CRASH if data folder actually exists
//alternative version:
text.loadFromFile("data"); //CRASH if data folder actually exists
}
 

There will be seg fault on loadFromFile when the dir actually exist. When the folder "data" doesn't exist in app's location there is no seg fault. It may be linux-only as well.

Call stack:
Quote
0  0x00007ffff78bf66c  stbi_load   
1  0x00007ffff78c33f4  sf::priv::ImageLoader::loadImageFromFile(std::string const&, std::vector<unsigned char, std::allocator<unsigned char> >&, sf::Vector2<unsigned int>&)   
2  0x00007ffff78c8843  sf::Texture::loadFromFile(std::string const&, sf::Rect<int> const&)   

I encountered this bug in following situation: (so it's a bug which may affect some ppl in rare situations)
sf::Texture tex;
std::string textureDir = "data/";
std::string textureName = map.getTextureName(..); // by error in map file, it returned empty string

tex.loadFromFile(textureDir+textureName);
 

What I expect in above case is that loadFromFile will return false and I will have an empty texture.
However, it simply crashed.

Remember that it must be existing folder as the function argument. When I set:
textureDir = "muhahadfjs2343434/";
 

Then there is no crash and expected output in console:
Quote
Failed to load image "muhahadfjs2343434/". Reason : Unable to open file


I hope that you will look into it. I can provide more debug info if you want.
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 02:01:07 pm
So, to summarize, the loadFromFile function crashes if you give it the path of a directory rather than a file?

Can you compile SFML in debug mode to get more information from the debugger (especially the line of the crash)?
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 02:01:44 pm
Yes, if you give it the path to existing directory.
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 02:02:34 pm
Can you compile SFML in debug mode to get more information from the debugger (especially the line of the crash)?

Sure, if you could tell me cmake command to compile it in debug mode?
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 02:08:51 pm
Set CMAKE_BUILD_TYPE to Debug.

It is explained in the tutorial very clearly.
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 02:12:52 pm
When I link SFML libs in debug mode there is no crash and I get:
Quote
Failed to load image "data". Reason : Image not of any known type, or corrupt

The crash happens only with release SFML libs.

Very strange. So I can't provide you the line of crash because with debug libs there is no crash.

EDIT:

But looking at the loadFromFile() source I know the line of crash:
106:    unsigned char* ptr = stbi_load(filename.c_str(), &width, &height, &channels, STBI_rgb_alpha);
 
It is the last function called in release mode.

I have no idea what STBI is. But it may be upstream bug in release mode.

EDIT2:

I see that STBI is packaged with SFML source and I think it has no active bug tracker. So it looks like it's the issue we need to fix ourselves. It works in debug mode, so we need to find out what is the difference between release and debug mode in STBI.
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 02:25:38 pm
The crash is inside the stbi_load function, but you don't know which at line exactly (it's not the call itself which crashes, it's something inside).

It's very strange that it works in debug mode... I'll try to investigate that myself.
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 02:32:25 pm
It's very strange that it works in debug mode... I'll try to investigate that myself.

That's good to hear. I don't feel like I'm able to solve it myself. STBI code is nothing like SFML code you know.
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 02:44:50 pm
Yes. STBI code is kind of scary ;D
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 07:10:20 pm
After checking, fopen works when path is a directory and access is read only. That's because, well, everything is a file on Unixes.

What's annoying is that there's no standard and easy way of checking whether the path (or the open FILE*) refers to a file or a directory; one would have to call stat() first for example.
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 07:12:30 pm
So we need to add some IFDEF for Unix and call some unix functions to check if it's dir or file?
Title: Re: Seg fault in loadFromFile
Post by: FRex on July 23, 2013, 07:41:11 pm
Quote
So we need to add some IFDEF for Unix and call some unix functions to check if it's dir or file?
Probably/possibly, it's really few lines and easy to do but it's kind of trolling the dev when a directory opens for reading images from it. :D

Quote
That's because, well, everything is a file on Unixes.
That is SO funny.(Not the fact everything is a file, I knew that, but the fact that it came in such lol context and caused such a  bug :D).
 
Quote
What's annoying is that there's no standard and easy way of checking whether the path (or the open FILE*) refers to a file or a directory; one would have to call stat() first for example.
Time to implement SFML.Filesystem! 8) ;D
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 07:48:06 pm
Time to implement SFML.Filesystem! 8) ;D

Haha that's true - for example I don't know any cross platform way for example to list files in a dir in C++ (only huge I-take-10-mins-to-compile-your-app boost).
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 23, 2013, 08:00:11 pm
Time to implement SFML.Filesystem! 8) ;D

Haha that's true - for example I don't know any cross platform way for example to list files in a dir in C++ (only huge I-take-10-mins-to-compile-your-app boost).

Instead of add something to SFML like this, why not use PhysFS?  There is an sf::InputStream implementation or two around that work with PhysFS and SFML2.

An added benefit to this would be packaging your resources etc in archives (save disk space) and reduce complexities in listing resources in a multi-platform way.  And yes PhysFS does support reading straight from a directory.
Title: Re: Seg fault in loadFromFile
Post by: netrick on July 23, 2013, 08:03:43 pm
@Atani

That's very interesting, I didn't know that lib! I will look into it for sure.

However the bug in SFML still needs to be fixed :P I think that IFDEF solution will work, who would look into so ugly STBI code anyway? :P
Title: Re: Seg fault in loadFromFile
Post by: FRex on July 23, 2013, 08:07:40 pm
Quote
Haha that's true - for example I don't know any cross platform way for example to list files in a dir in C++ (only huge I-take-10-mins-to-compile-your-app boost).
That was a joke(note the  8) and  ;D) because once in a while people suggest whacky features like filesystem, 3d, etc. while there are alternatives and better libraries for these.
Title: Re: Seg fault in loadFromFile
Post by: Nexus on July 23, 2013, 08:09:33 pm
(only huge I-take-10-mins-to-compile-your-app boost).
Keep in mind that if the compile times are really an issue, it is possible to use the PImpl idiom to abstract dependencies away. Especially in the case of Boost.Filesystem, where you don't need templates, you can wrap the most important functionality.

But PhysFS is also nice-- it's written in C however. Thus, in order to use it reasonably in bigger projects, you would have to build RAII wrappers, too.
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 23, 2013, 08:12:54 pm
However the bug in SFML still needs to be fixed :P I think that IFDEF solution will work, who would look into so ugly STBI code anyway? :P

Agreed, an ifdef to use stat on most platforms and http://msdn.microsoft.com/en-us/library/windows/desktop/aa364944(v=vs.85).aspx (http://msdn.microsoft.com/en-us/library/windows/desktop/aa364944(v=vs.85).aspx) on windows.
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 23, 2013, 08:26:37 pm
But PhysFS is also nice-- it's written in C however. Thus, in order to use it reasonably in bigger projects, you would have to build RAII wrappers, too.

I would imagine most bigger projects would integrate it using the sf::InputStream approach and cache the SFML objects and not the actual file system objects.

Further on that thought, it would likely be in a small number of Cache objects (ideally one or one per object type) that internally know how to deal with PhysFS or similar interaction with the filesystem objects.
Title: Re: Seg fault in loadFromFile
Post by: Laurent on July 23, 2013, 10:10:29 pm
The conditional use of stat() in the code would definitely work. But this seems to be a more general problem, if what we say is true then loading any kind of resource (shader, texture, font, sound, music, ...) with the path of a directory should produce similar results. And not only in SFML. So I really feel like we are missing something obvious.

Quote
and http://msdn.microsoft.com/en-us/library/windows/desktop/aa364944(v=vs.85).aspx on windows
There's no problem on Windows, directories are not files on this OS, and fopen correctly fails in this case.
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 23, 2013, 10:17:19 pm
....loading any kind of resource (shader, texture, font, sound, music, ...) with the path of a directory should produce similar results. And not only in SFML. So I really feel like we are missing something obvious.

The problem reported was trying to open "data/" or "data" (both directories, existing) as a file to load something from.  I think this is more of a generic problem in the caller and not necessarily something in SFML that is at fault here.

Though, handling this in a more graceful approach would be nice to have.  Using stat() or GetFileAttributes() would ease the creation of an exception or similar case to be raised when a caller provides an invalid value (directory instead of file, or missing file, etc).
Title: Re: Seg fault in loadFromFile
Post by: FRex on July 24, 2013, 02:40:35 am
I've been on it for a long while:
This is some schrodinger bug, I can't accurately track when it occurs and when not, I tried -g, -O1, -O2 but in the end results were inconsistent, sometimes it happens, sometimes not(because it involves ub by accessing undefined ptr, see below)

Directory open for reading as a file seems to always(both FILE and ifstream) return that it read 0 bytes and has size of 2^31-1.

-sf::Shader.loadFromFile attempts to allocate 2^31-1 which throws std::bad_alloc(I have 2 gbs of ram) you probably should catch that exception up in loadFromFile and print what() to sf::err() and return false from load.

-sf::Image stbi attepts to decrement by one a pointer that has values null, 0x1, sometimes valid adresses, in the program adress space, it's just uninit ptr
typedef struct
{
   uint32 img_x, img_y;
   int img_n, img_out_n;
   
   stbi_io_callbacks io;
   void *io_user_data;

   int read_from_callbacks;
   int buflen;
   uint8 buffer_start[128];

   uint8 *img_buffer, *img_buffer_end;//these 2 ptrs are important
   uint8 *img_buffer_original;
} stbi;

unsigned char *stbi_load_from_file(FILE *f, int *x, int *y, int *comp, int req_comp)
{
   stbi s;
   start_file(&s,f);
   return stbi_load_main(&s,x,y,comp,req_comp);
}

static void start_file(stbi *s, FILE *f)
{
   start_callbacks(s, &stbi_stdio_callbacks, (void *) f);
}

static void start_callbacks(stbi *s, stbi_io_callbacks *c, void *user)
{
   s->io = *c;
   s->io_user_data = user;
   s->buflen = sizeof(s->buffer_start);
   s->read_from_callbacks = 1;
   s->img_buffer_original = s->buffer_start;
   refill_buffer(s);// goes into there with these 2 pointers with undefined values in them
}

//the callback in refill buffer is that one, it always returns 0 for directories open as FILE
static int stdio_read(void *user, char *data, int size)
{
   return (int) fread(data,1,size,(FILE*) user);
}

static void refill_buffer(stbi *s)
{
   int n = (s->io.read)(s->io_user_data,(char*)s->buffer_start,s->buflen);
   if (n == 0) {//we go here
      // at end of file, treat same as if from memory
      s->read_from_callbacks = 0;
      s->img_buffer = s->img_buffer_end-1;//this wasnt ever initialized, sometimes its null, sometimes its 0x1, idk why
      *s->img_buffer = 0;//this line might do the segfault, writing to 0xffffffff or 0x0, in general writing into random space
   } else {
      s->img_buffer = s->buffer_start;
      s->img_buffer_end = s->buffer_start + n;
   }
}
 

now if we hit into own memory, we are kind of ok and go to this function,
tatic unsigned char *stbi_load_main(stbi *s, int *x, int *y, int *comp, int req_comp)
{
   if (stbi_jpeg_test(s)) return stbi_jpeg_load(s,x,y,comp,req_comp);
   if (stbi_png_test(s))  return stbi_png_load(s,x,y,comp,req_comp);
   if (stbi_bmp_test(s))  return stbi_bmp_load(s,x,y,comp,req_comp);
   if (stbi_gif_test(s))  return stbi_gif_load(s,x,y,comp,req_comp);
   if (stbi_psd_test(s))  return stbi_psd_load(s,x,y,comp,req_comp);
   if (stbi_pic_test(s))  return stbi_pic_load(s,x,y,comp,req_comp);

   #ifndef STBI_NO_HDR
   if (stbi_hdr_test(s)) {
      float *hdr = stbi_hdr_load(s, x,y,comp,req_comp);
      return hdr_to_ldr(hdr, *x, *y, req_comp ? req_comp : *comp);
   }
   #endif

   // test tga last because it's a crappy test!
   if (stbi_tga_test(s))
      return stbi_tga_load(s,x,y,comp,req_comp);
   return epuc("unknown image type", "Image not of any known type, or corrupt");
}
 

-jpeg gets 'marker' m which if not 0xd8 will return error:
#define SOI(x)         ((x) == 0xd8)
if (!SOI(m)) return e("no SOI","Corrupt JPEG");

-png, bmp and gif check for the three letters of their respective format in first bytes

-psd checks for "8BPS"

-pic checks for "\x53\x80\xF6\x34"

-hdr checks for "#?RADIANCE\n" but I'm not sure if SFML doesn't define it out, but it doesn't matter

-tga discards one byte, then returns if second byte is >1 or third is unequal to any of 1 2 3 9 10 11

Since we have 0 as first and only byte, and in case of tga we run out and call refill which 'refills' with 0 again so we are basically endless stream of 0s and fail all tests. I'd say that's a bug in stbi.

-sf::SoundBuffer and sf::Music, "Failed to open sound file "." (File contains data in an unknown format.)", I don't have source of sndfile but it seems it's immune to the fact that data can't be read from seemingly ok long file

-sf::Font "Failed to load font "." (failed to create the font face)" I guess freetype is immune too..

In any case this needs more testing but shader can be fixed right away,
stbi too:
static void start_callbacks(stbi *s, stbi_io_callbacks *c, void *user)
{
   s->io = *c;
   s->io_user_data = user;
   s->buflen = sizeof(s->buffer_start);
   s->read_from_callbacks = 1;
   s->img_buffer_original = s->buffer_start;
   s->img_buffer_end=s->buffer_start+1;//this line fixes it because now we use the first byte from out buffer_start[128]
   refill_buffer(s);
}
 
I think..

But I might have made mistakes, and these really need more work:
-check freetype reactions and safety
-check sndfile reactions and safety
-check behavior of directory in FILE * on various *nixes(linux definitely affected at least my lovely Fedora and netrick's lubuntu, maybe other distros and macs too?)
-just use stat or dirent or something to avoid opening directory like that?
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 24, 2013, 02:46:31 am
-just use stat or dirent or something to avoid opening directory like that?

This is what I would recommend here.  However, the checking of this should be done wherever file access is done (could there be a common base class that handles file interaction?)
Title: Re: Seg fault in loadFromFile
Post by: FRex on July 24, 2013, 03:03:10 am
I don't know, I'm putting what I know up there for Laurent to decide.
Without my 1 line fix stbi causes same thing(segfault) to happen with empty file(because it returns 0 from that callback again).
Shader, Music, SoundBuffer and Font are all fine with empty files it seems(error correctly with alright error message).
Title: Re: Seg fault in loadFromFile
Post by: Atani on July 24, 2013, 03:05:32 am
image files must be more than 1 byte in length.  So I can see why stbi is crashing on that.  I am not sure the fix you proposed is right either though.  I haven't looked at the stbi code in depth (too much of it!) but I am sure there is another simpler (and safer?) fix.

Perhaps after opening the file (via fopen, etc), a file length check can be done and abort if the file is not long enough for any of the image signatures?
Title: Re: Seg fault in loadFromFile
Post by: FRex on July 24, 2013, 03:13:25 am
Yes, but when things aren't as they 'must' be we print profanities to sf::err() and stay in well defined state, not invoke ub and segfault. ;)
It literally can't get any worse for dirs and empty files, we are acessing uninitialized variables and writing to random byte in memory as it is now.
I guess length check would fix it too but directories seem to have 2^31-1 length on my machine so that'd need attending too.

Basically what refill does(and it does that after opening file for first time and then again if exhausted the buffer) is(excuse pythonic indent):
    if read at least 1 byte from source:
        set read ptr to beginning of buffer, set end ptr 1 byte past last read byte
    else
        set read ptr at last loaded byte in last refill(which is end ptr - 1)
        change that byte value to 0

Then during reads this happens in get8:
    if read ptr is equal to end ptr:
        refill
    else
        return current value under read ptr, move read ptr forward 1 byte

get8 is called directly in code and by all other get.. functions.
The problem happens if we got to else during first call to refill, because then the end ptr is uninitialized.

Also it seems like loading from sf::InputStream if it always returns 0 from read should segfault too because it goes through start_callbacks with uninit ptr too but it doesn't segfault on my machine so far.