FreeImage / Bugs / #295 Strange segfault when calling FreeImage

Just to make things a little more clear, here is the path my code takes before calling FreeImage_GetPixelColor() at which point I receive a segfault from the FreeImage library :

#include "ccc-common.h"

int main(int argc, char **argv) {

    char *cpmsg = (char *) FreeImage_GetCopyrightMessage();

    printf("( %s ) \n\n", cpmsg);

    if (argc <= 1) {
        fprintf(stderr, "You must specify an input file...\n");
        return(1);
    }

    FreeImage_Initialise(0);

    FreeImage_SetOutputMessage(FreeImageErrorHandler);

    FIBITMAP *image_data;

    char *file_name = (*(argv + 1));

    FREE_IMAGE_FORMAT file_type = FreeImage_GetFileType((const char *) file_name, 0);

    switch(file_type) {

        case FIF_BMP : {
            image_data = FreeImage_Load(FIF_BMP, (const char *) file_name, BMP_DEFAULT);
            if (errorlevel) {
                fprintf(stderr, "Error loading input file...\n\n");
                FreeImage_DeInitialise();
                return(1);
            }
            break;          
        }

        case FIF_JPEG : {
            image_data = FreeImage_Load(FIF_JPEG, (const char *) file_name, JPEG_ACCURATE);
            if (errorlevel) {
                fprintf(stderr, "Error loading input file...\n\n");
                FreeImage_DeInitialise();
                return(1);
            }
            break;
        }

        case FIF_PNG : {
            image_data = FreeImage_Load(FIF_PNG, (const char *) file_name, PNG_DEFAULT);
            if (errorlevel) {
                fprintf(stderr, "Error loading input file...\n\n");
                FreeImage_DeInitialise();
                return(1);
            }
            break;
        }

        case FIF_TIFF : {
            image_data = FreeImage_Load(FIF_TIFF, (const char *) file_name, TIFF_DEFAULT);
            if (errorlevel) {
                fprintf(stderr, "Error loading input file...\n\n");
                FreeImage_DeInitialise();
                return(1);
            }
            break;
        }

        default : {
            image_data = GenericLoader(file_name, 0);
            if (errorlevel || (image_data == NULL)) {
                fprintf(stderr, "Unkown file type. You *MUST* specify a BMP, JPEG, PNG or TIFF file...\n\n");
                FreeImage_DeInitialise();
                return(1);
            }
            break;
        }
    }

    RGBQUAD *rgb2 = FreeImage_GetPalette(image_data);

    if (rgb2 != NULL) {
        raise_error(image_data, "ERROR : Palettized images are not supported...\n");
        return(1);
    }


/*  if (errorlevel) {
        raise_error(image_data, "Error loading input file...\n");
        return(1);
    }*/

    FreeImage_ConvertTo24Bits(image_data);

    if (errorlevel) {
        raise_error(image_data, "Error converting image to 24 bits per pixel...\n");
        return(1);
    }


    int width = (int) FreeImage_GetWidth(image_data);

    if (errorlevel) {
        raise_error(image_data, "Error getting image width...\n");
        return(1);
    }

    int height = (int) FreeImage_GetHeight(image_data);

    if (errorlevel) {
        raise_error(image_data, "Error getting image height...\n");
        return(1);
    }

    if ( ((width % 4) + (height % 4)) ) {
        raise_error(image_data, "Error : the input image must have dimensions that are divisible by 4...\n");
        return(1);
    }

    pixel *pixels = calloc(width*height, sizeof(pixel));

    if(pixels == NULL) {
        raise_error(image_data, "Not eneough memory to allocate pixel bitmap...\n");
        return(1);
    }

 double *pixel_luminances = calloc(width*height, sizeof(double));

    if(pixel_luminances == NULL) {
        raise_error(image_data, "Not eneough memory to allocate pixel luminances array...\n");
        free(pixels);
        return(1);
    }


pixel_block *pixel_blocks = calloc((width/4)*(height/4), sizeof(pixel_block));

if(pixel_blocks == NULL) {
        printf("Not eneough memory to allocate pixel_blocks array...\n");
        free(pixel_luminances);
        free(pixels);
        return(1);
    }

    int pixels_max_size = width*height*sizeof(pixel);
    int pixel_blocks_max_size = width*height*sizeof(pixel_block);
    int pixel_luminances_max_size = width*height*sizeof(double);

    //The following function call causes FreeImage to segfault
    getPixels(image_data, pixels, width, height);

    if (errorlevel) {
        error_free((void *) pixels, (void *) pixel_blocks, (void *) pixel_luminances);
        raise_error(image_data, "Error trying to read pixel data...");
        return 1;
    }
#endif

As for ccc-common.c and ccc-common.h, the relevant code is :

struct pixel {
    uint32_t color;
    uint8_t r;
    uint8_t g;
    uint8_t b;
};

typedef struct pixel pixel;

and

#include "ccc-common.h"

//Global variable which indicates if a FreeImage library function has encountered an error
int errorlevel = 0;

void check_indices(int index1, int bound1) {
    if (index1 >= bound1) {
        errorlevel = 1;
    }
}

//Function to free() malloc()/calloc() allocated 2D arrays
void error_free(void *p1, void *p2, void* p3) {
    if(p1 != NULL) { free(p1); }
    if(p2 != NULL) { free(p2); }
    if(p3 != NULL) { free(p3); }
}

//Note : This function was copied from the FreeImage developer documentation
/** Generic image loader
@param lpszPathName Pointer to the full file name
@param flag Optional load flag constant
@return Returns the loaded dib if successful, returns NULL otherwise
*/
FIBITMAP* GenericLoader(const char* lpszPathName, int flag) {
    FREE_IMAGE_FORMAT fif = FIF_UNKNOWN;
    // check the file signature and deduce its format
    // (the second argument is currently not used by FreeImage)
    fif = FreeImage_GetFileType(lpszPathName, 0);
    if(fif == FIF_UNKNOWN) {
        // no signature ?
        // try to guess the file format from the file extension
        fif = FreeImage_GetFIFFromFilename(lpszPathName);
    }
    // check that the plugin has reading capabilities ...
    if((fif != FIF_UNKNOWN) && FreeImage_FIFSupportsReading(fif)) {
        // ok, let's load the file
        FIBITMAP *dib = FreeImage_Load(fif, lpszPathName, flag);
        // unless a bad file format, we are done !
        return dib;
    }
    return NULL;
}

//Note : This function is based on a similar function in the FreeImage developer documentation
/**
FreeImage error handler
@param fif Format / Plugin responsible for the error
@param message Error message
*/
//Error handler for FreeImage library functions
void FreeImageErrorHandler(FREE_IMAGE_FORMAT fif, const char *message) {
    //printf("Error : *** \n");
    printf(" *** \n");
    if(fif != FIF_UNKNOWN) {
        //printf("Format is type %s\n It *MUST* be type BMP, JPEG, PNG or TIFF\n", FreeImage_GetFormatFromFIF(fif));
        printf("Format is type %s\n It *MUST* be type BMP, JPEG, PNG or TIFF\n", FreeImage_GetFormatFromFIF(fif));
    }
    printf("%s\n", message);
    printf(" *** \n");
    errorlevel = 1; //Set errorlevel global variable to 1 to indicate that an error condition was detected
}

//Generic 2-dimensional array access functions

//Sets an element in a 2D array with bounds check
void setElement(void *array1, int width1, size_t elementSize, int x, int y, const void *element, int max_size) {
    if ((x*elementSize + y*width1*elementSize) > max_size) {
        fprintf(stderr, "ERROR : setElement() tried to access an out-of-bounds element...\n");
        fprintf(stderr, "Arguments were -- width1 : %i , elementSize : %i , x : %i , y : %i , max_size : %i \n", width1, (int) elementSize, x, y, max_size);
        errorlevel = 1;
        return;
    }
    memcpy((array1 + y*width1*(elementSize) + (x*elementSize)), element, elementSize);
}

//Gets an element in a 2D array with bounds check
void *getElement(void *array1, int width1, size_t elementSize, int x, int y, int max_size) {
    if ((x*elementSize + y*width1*elementSize) > max_size) {
        fprintf(stderr, "ERROR : getElement() tried to access an out-of-bounds element...\n");
        fprintf(stderr, "Arguments were -- width1 : %i , elementSize : %i , x : %i , y : %i , max_size : %i \n", width1, (int) elementSize, x, y, max_size);
        errorlevel = 1;
        return;
    }
    return (array1 + y*width1*(elementSize) + ((x*elementSize)));
}

//Function needed by qsort when the color histogram needs to be sorted
int compare_colors(const void *a, const void *b) {
    return ( *(int*)a - *(int*)b );
}

//The segault occurs in the FreeImage library in the following function
//Initializes the pixels 2-dimensional array
void getPixels(FIBITMAP *image, pixel *pixels1, int width1, int height1) {
    for (int y = 0; y < height1; y++) {
        for (int x = 0; x < width1; x++) {
            RGBQUAD *rgb1;
            int pixels_max_size1 = width1*height1*sizeof(pixel);
            //The following function call is what causes the segfault...
            if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, rgb1)) {
                fprintf(stderr, "FreeImage_GetPixelColor error\n");
                errorlevel = 1;
                return;
            }

pixel *t1 = getElement((void *) pixels1, width1, sizeof(pixel), x, y, pixels_max_size1);

if (errorlevel) {
                return;
            }

            t1->color = ((((rgb1->rgbRed) << 16) & 0xFF0000 ) + (((rgb1->rgbGreen) << 8) & 0x00FF00) + (((rgb1->rgbBlue)) & 0x0000FF)) & 0xFFFFFF;
            t1->r = (uint8_t) (rgb1->rgbRed);
            t1->g = (uint8_t) (rgb1->rgbGreen);
            t1->b = (uint8_t) (rgb1->rgbBlue);
        }
    }
}

I apologize for the sloppiness in the code. The code is in the pre-alpha stage and is in a constant state of flux and there are a lot of errors to fix in the rest of the code.

Any help on resolving the apparent bug in the FreeImage library would be welcome.

jdb2

Last edit: jdb2 2019-07-23

jdb2 - 2019-07-22

I just tested it on a 2 inch x 2 inch TIFF file with a blue box inside created with Windows Paint, and I get the same errors and segfault. This is definitely a bug in FreeImage.

jdb2

Last edit: jdb2 2019-07-22

Blue box.tif

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Grant Boyle - 2019-07-22

Not sure if it's related but it looks like the cases in the switch(file_type) can fall through. That is, they don't break when errorlevel==0.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

jdb2 - 2019-07-22

Well, if errorlevel is equal to zero, then there is no error exit from main(), but I don't see how that would cause the switch statement to misbehave, as, in each case, the code is :

if (errorlevel) { fprintf(stderr, "Error loading input file...\n"); FreeImage_DeInitialise(); return(1); }

If errorlevel is equal to zero, then this code never gets executed. I do know that the switch statement works for TIFF files as getPixels() eventually gets called which calls FreeImage_GetPixelColor(), which is the source of the segfault.

Regards,

jdb2
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Grant Boyle - 2019-07-22

Say file_type is FIF_TIFF and FreeImage_Load(FIF_TIFF,...) works. That is, errorlevel==0. Execution will fall through to the default: case and call GenericLoader(...).

I guess I'm suggesting adding break to each case, e.g.:

case FIF_TIFF : { image_data = FreeImage_Load(FIF_TIFF, (const char *) file_name, TIFF_DEFAULT); if (errorlevel) { fprintf(stderr, "Error loading input file...\n"); FreeImage_DeInitialise(); return(1); } break; }
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- jdb2 - 2019-07-22
  
  Say file_type is FIF_TIFF and FreeImage_Load(FIF_TIFF,...) works. That is, errorlevel==0. > Execution will fall through to the default: case and call GenericLoader(...).
  
  I guess I'm suggesting adding break to each case, e.g.:
  
  Ooops! :D I made a boo-boo and overlooked that :P -- stupid mistake. I also code in Verilog RTL and other languages and I forgot about inserting the breaks for some reason :$
  
  I've corrected the code in the switch statement but the same problems still manifest themselves in terms of FreeImage_GetImage_color() segfaulting and spitting out non-fatal errors.
  
  Thanks for the heads up on my stupid mistake :)
  
  Regards,
  
  jdb2
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

jdb2 - 2019-07-23

Just to eliminate any unknowns, I temporarily commented out all file type detection code and loaded the test.tiff file using a hard coded name in the source. I get the same segfault in FreeImage_GetPixelColor()

jdb2

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Something I just noticed is that the call to FreeImage_GetPixelColor is passing an uninitialized pointer for the RGBQUAD.

            //RGBQUAD *rgb1;
            RGBQUAD rgb1;

            ...

            //if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, rgb1)) {
            if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, &rgb1)) {
                ...
            }

jdb2 - 2019-07-23

Yeah, and GCC complains about it :) According to the FreeImage manual :

FreeImage_GetPixelColor
16
24
32
DLL_API BOOL DLL_CALLCONV FreeImage_GetPixelColor(FIBITMAP dib, unsigned x, unsigned
y, RGBQUAD value);
Get the pixel color of a 16-, 24- or 32-bit image at position (x, y), including range check (slow
access). Parameter x is the pixel position in horizontal direction, and parameter y is the pixel
position in vertical direction. The function returns TRUE on success, and returns FALSE
otherwise (e.g. for palettized images).

That is, the RGBQUAD pointer isn't initialized as FreeImage_GetPixelColor() is supposed to set it :) I guess I could initialize it to NULL before calling the function. I'll try that now...

Also, I generated another test image, this time a 512x512 pixel JPEG exported from the GIMP and FreeImage_GetPixelColor() still segfaults.

Regards,

jdb2

512x512-blue-square-test.jpg

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

jdb2 - 2019-07-23

Well, I just tried setting rgb1 in getPixels() to NULL before I call FreeImage_GetPixelColor() and I still get the same segfault, although now GCC doesn't complain about a possibly unitialized variable.

jdb2

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

FreeImage_GetPIxelColor() takes an RGBQUAD pointer, so the code :

    //RGBQUAD *rgb1;
    RGBQUAD rgb1;

    ...

    //if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, rgb1)) {
    if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, &rgb1)) {
        ...
    }

generates the following errors :

ccc-common.c: In function ‘getPixels’:
ccc-common.c:125:67: warning: passing argument 4 of ‘FreeImage_GetPixelColor’ from incompatible pointer type [-Wincompatible-pointer-types]
    if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, &rgb1)) {
                                                                   ^
In file included from ccc-common.h:17:0,
                 from ccc-common.c:1:
/usr/include/FreeImage.h:908:27: note: expected ‘RGBQUAD * {aka struct tagRGBQUAD *}’ but argument is of type ‘RGBQUAD ** {aka struct tagRGBQUAD **}’
 DLL_API BOOL DLL_CALLCONV FreeImage_GetPixelColor(FIBITMAP *dib, unsigned x, unsigned y, RGBQUAD *value);
                           ^~~~~~~~~~~~~~~~~~~~~~~

rgb1 is passed by value but its value is a pointer to an RGBQUAD :)

jdb2

Last edit: jdb2 2019-07-23

Grant Boyle - 2019-07-23

FreeImage_GetPixelColor can't set the pointer for you in this case. It is taking the pointer by value (that is, it passes a copy of the pointer you pass). Things might be different if it took a pointer to pointer or a reference to a pointer.

I believe the expected behavior is for FreeImage_GetPixelColor to write the color to the RGBQUAD pointed at by the pointer you pass. I would try creating a local RGBQUAD and passing its address as the pointer.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- jdb2 - 2019-07-23
  
  It looks like you've found the source of the problem! :D Thanks so much! I guess I wasn't fully understanding the semantics of the RGBQUAD pointer passed to FreeImage_GetPixelColor().
  
  My code now doesn't segfault in the FreeImage library anymore thanks to your suggestion :D
  
  Thanks a lot! :D
  
  I'll mark this "bug "as solved / closed :)
  
  Regards,
  
  jdb2
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

jdb2 - 2019-07-23

Well, I can't seem to figure out how to mark this ticket as closed / solved, so an admin will have to do it, unless I'm missing something obvious, which I probably am :).

jdb2

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Hervé Drolon - 2019-07-24

status: open --> closed

assigned_to: Hervé Drolon

Group: -->
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Hervé Drolon - 2019-07-24

Hi,

To sum up

using
RGBQUAD *rgb2 = FreeImage_GetPalette(image_data);
rgb2 is NULL for 24-bit images

using
FreeImage_ConvertTo24Bits(image_data);
will cause a memory leak
use instead
FIBITMAP *dst = FreeImage_ConvertTo24Bits(image_data);
if(dst) {
FreeImage_Unload(image_data);
image_data = dst;
}
else {
FreeImage_Unload(image_data);
return 1;
}

Use

RGBQUAD rgb1 = {0, 0, 0, 0};
if(!FreeImage_GetPixelColor(image, (unsigned) x, (unsigned) y, &rgb1)) {
...
}

Note that you can optimize pixel access using FreeImage_GetScanLine and memcpy

Hervé

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Strange segfault when calling FreeImage_GetPixelColor()

Group

Searches

Help

#295 Strange segfault when calling FreeImage_GetPixelColor()

Discussion