Page 1 of 1
MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-19T08:57:37-07:00
by mi
The following piece of code now causes a crash on FreeBSD, where malloc/free track freed memory:
Code: Select all
profile = MagickRemoveImageProfile(wandPtr, (const char*)name, &length);
puts(profile);
MagickRelinquishMemory(profile);
According to the debugger, both profile and name are (or recently were) valid:
Code: Select all
(gdb) p profile
$4 = (unsigned char *) 0xb88000 "foo"
(gdb) p name
$5 = 0x5d9730 "icc"
But, according to FreeBSD's free, the memory pointed to by profile has already been freed...
This worked with 6.3.3-5 -- not sure, when it broke
Just checked with 6.3.5-3 -- still broken.
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-19T10:28:41-07:00
by magick
We have a patch for the problem you reported. It will be available in ImageMagick 6.3.5-3 Beta sometime tomorrow.
(Non-)Regression tests?
Posted: 2007-07-19T12:13:10-07:00
by mi
Ok, thanks. This calls into question your testing methods, however
I encountered this crash by running TclMagick's self-tests. Does ImageMagick bundle any (non-)regression tests with the distribution, that anyone can run after building -- to quickly detect possible developer bugs as well as any local miscompilations?
Thanks!
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-19T13:06:07-07:00
by magick
We run over 1000 regression tests before we release ImageMagick. Unfortunately MagickRemoveImageProfile() was not one of the tests, however, we will include a unit test for it in the next release.
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T06:04:39-07:00
by mi
Can one download the regression tests? I'd like to include them in the routine build of ImageMagick -- to catch local miscompilations and other problems.
I doubt, you test on platforms as "exotic" as FreeBSD/amd64...
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T06:29:32-07:00
by magick
The regression tests are part of the source distribution. Type
to run them.
I doubt, you test on platforms as "exotic" as FreeBSD/amd64.
FreeBSD is far from exotic and yes we run the regression tests under FreeBSD and Linux and Solaris and Mac OS X and HPUX and AIX and others.
Improving the MagickRemoveImageProfile interface
Posted: 2007-07-20T06:44:56-07:00
by mi
In many cases, the profile returned by the function is not really needed by the caller and ends up being passed to MagickRelinquishMemory right away.
By specifying the length-pointer as NULL, the caller could indicate their disinterest:
Code: Select all
--- wand/magick-image.c 2007-07-16 20:49:49.000000000 -0400
+++ wand/magick-image.c 2007-07-20 09:43:29.000000000 -0400
@@ -7714,6 +7714,8 @@
return((unsigned char *) NULL);
}
- *length=0;
profile=RemoveImageProfile(wand->images,name);
+ if (length == NULL)
+ return NULL;
+ *length=0;
if (profile != (StringInfo *) NULL)
*length=GetStringInfoLength(profile);
Regression tests fail on FreeBSD/amd64
Posted: 2007-07-20T07:19:43-07:00
by mi
FreeBSD (on i386) is not exotic, but amd64 has not seen regression tests in a while. When I try to do `make check' here, I get:
Code: Select all
wand/wandtest.c: In function `main':
wand/wandtest.c:72: error: `description' undeclared (first use in this function)
wand/wandtest.c:72: error: (Each undeclared identifier is reported only once
wand/wandtest.c:72: error: for each function it appears in.)
wand/wandtest.c: At top level:
wand/wandtest.c:417: error: conflicting types for 'magick_wand'
wand/wandtest.c:388: error: previous declaration of 'magick_wand' was here
wand/wandtest.c:417: warning: initialization makes integer from pointer without a cast
wand/wandtest.c:417: error: initializer element is not constant
wand/wandtest.c:417: warning: data definition has no type or storage class
wand/wandtest.c:418: error: syntax error before "void"
wand/wandtest.c:400: error: register name not specified for 'i'
This is easy to pach -- just remove the blank lines in the ``#define ThrowAPIException''...
I can't see, how it could possibly have worked anywhere, though, which means, you don't run these tests as often as you upload tar-balls into beta/ subdirectory of the ftp-site.
Having fixed the macro, I can the actual tests, and get a whole lot failures and segfaults (mostly in FPX, which is known to be severely broken on 64-bit platforms), but not only...
I wish you
really were testing on FreeBSD/amd64...
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T07:34:27-07:00
by magick
We do our regression tests on an AMD64 Redhat box. We'll try to locate an AMD64 FreeBSD box and run the regression tests. We are confident they will run without complaint.
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T07:40:46-07:00
by mi
I hate to be rude, but you are not running them
often enough. FreeBSD or RedHat -- gcc would not have compiled the wandtest.c as it is currently shipped (two blank lines inserted into the ThrowAPIException-macro).
This means, uploads to the ftp-site happen more often, than attempts at "make check".
Don't kill yourself (yet) trying to find a FreeBSD/amd64 box. Just run every beta through "make check" on the machines you have already -- something you
demonstrably do not do at the moment...
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T07:49:33-07:00
by magick
Our ImageMagick release build script cannot complete unless the regression tests run and complete successfully. Not sure why its failing for you but the important thing is that it works for us.
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T07:58:30-07:00
by mi
magick wrote:Our ImageMagick release build script cannot complete unless the regression tests run and complete successfully.
Can you post a link to the build log of the currently posted beta/ImageMagick-6.3.5-3.tar.bz2?
Here is
my build log
magick wrote:Not sure why its failing for you but the important thing is that it works for us.
Come on. You are writing this for me - the user - not for yourself...
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T08:00:33-07:00
by magick
Well that's the problem. Did you notice ImageMagick 6.3.5-3 is in the Beta directory? Its not released so there may be some problems with it. Blessed regression testing is only valid for the ImageMagick release versions.
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T08:10:20-07:00
by mi
magick wrote:Did you notice ImageMagick 6.3.5-3 is in the Beta directory? Its not released so there may be some problems with it. Blessed regression testing is only valid for the ImageMagick release versions.
Throughout this thread I kept repeating, that I used the 6.3.5-3 version, which, you are right, is currently in beta.
I also wrote:
mi wrote:I can't see, how it could possibly have worked anywhere, though, which means, you don't run these tests as often as you upload tar-balls into beta/ subdirectory of the ftp-site.
In other words, I know, it is "beta", but I still don't understand, why you don't run through regression tests even the beta code... It is (or should be) much easier to have computer(s) do the work, than to wrangle with the upset users on the forums...
You are right that "there may be some problems with it", but that should not include any of the problems, for which test-cases have already been developed. What's the point of uploading even a beta, if it has not passed the automated regression tests?
Re: MagickRemoveImageProfile returns freed memory in 6.3.5-2, -3
Posted: 2007-07-20T09:11:58-07:00
by magick
We do run the regressions on Beta release-- just not as diligently as a release. Just a few days ago we added regression tests for Wand profiles, properties, and options and it looks like there is a version skew between the Beta and our development area. We'll get that fixed and release a new Beta by tomorrow.