Page 1 of 1

Uninitialized exception string

Posted: 2010-04-08T15:09:24-07:00
by asouclks
I was having a problem where my ExceptionInfo.reason was growing every time I got an error. I was testing error cases in which I would try to load a file that was not there over and over. I could tell it was related to memory in a way because I could comment out code before it and then it would work. Or, if I would shorten the filename I was looking for, it would work.

This code would result in exception.reason to grow on every call:

strcpy(image_info.filename,"/the/file/is/not/there");
image=ReadImage(image_info,exception);
if (image == (Image *) NULL) {
printf ("Cannot read image file: %s (exception=%s)", image_info->filename, exception->reason);
}

After 1st call:
Cannot read image file: /the/file/is/not/there (exception=unable to open image `/the/file/is/not/there': @error/blob.c/OpenBlob/2489)
After 2nd call:
Cannot read image file: /the/file/is/not/there (exception=unable to open image `/the/file/is/not/there': c/OpenBlob/2489 @ error/blob.c/OpenBlob/2489)
After 3rd call:
Cannot read image file: /the/file/is/not/there (exception=unable to open image `/the/file/is/not/there': 9 @ error/blob.c/OpenBlob/2489 @ error/blob.c/OpenBlob/2489)

And so on...

I decided to run a simple test program through valgrind to see if I could find out why this was happening. Here is the test program:

MagickCoreGenesis(NULL,MagickTrue);
Image *image;
ImageInfo *image_info;
ExceptionInfo *exception;

exception=AcquireExceptionInfo();
image_info=CloneImageInfo((ImageInfo *) NULL);
strcpy(image_info->filename,"/the/file/is/not/there");
image=ReadImage(image_info,exception);
if (image == (Image *) NULL) {
printf ("Cannot read image file: %s (exception=%s)\n", image_info->exception, exception->reason);
}
image_info=DestroyImageInfo(image_info);
exception=DestroyExceptionInfo(exception);
MagickCoreTerminus();

If I ran the ReadImage in a loop, I could not reproduce the same error (growing ExceptionInfo.reason) I had in my real program. This also led me to believe it was memory related since it was very unpredictable.

So, I removed the loop and just made it do ReadImage once and ran it through valgrind. The above code produced this error:

==6984== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 35 from 1)
==6984==
==6984== 1 errors in context 1 of 1:
==6984== Conditional jump or move depends on uninitialised value(s)
==6984== at 0x400623B: strlen (mc_replace_strmem.c:246)
==6984== by 0x41994B6: ConstantString (string.c:632)
==6984== by 0x40EE09E: GetExceptionMessage (exception.c:465)
==6984== by 0x404D557: OpenBlob (blob.c:2489)
==6984== by 0x4119120: SetImageInfo (image.c:3284)
==6984== by 0x4081DE9: ReadImage (constitute.c:445)
==6984== by 0x8048770: main (test.c:57)
--6984--

I noticed in exception.c in GetExceptionMessage, the variable exception is not initialized. So, I added this code and recompiled the library

MagickExport char *GetExceptionMessage(const int error)
{
char
exception[MaxTextExtent];
(void) ResetMagickMemory(&exception,0,sizeof(exception)); // Initialize exception variable

After doing this, the valgrind error went away and my real program no longer exhibits the appending behavior.

I'm not sure if this is the right fix or not, but I thought I would post to let you know.

Re: Uninitialized exception string

Posted: 2010-04-08T16:00:20-07:00
by magick
We'll add your patch to ImageMagick 6.6.1-2 Beta. Thanks for the analysis and patch.