GraphicsMagick: Fixes for undefined behavior, added more tests, ...
GraphicsMagick Commits <[email protected]>
| Newsgroups | gmane.comp.video.graphicsmagick.cvs |
|---|---|
| Message-ID | <mailman.23435.1669584980.1350.graphicsmagick-commit@lists.sourceforge.net> |
changeset 15545e750d39 in /hg/GraphicsMagick details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=15545e750d39 summary: Fixes for undefined behavior, added more tests, and a general "bug-scrub". diffstat: ChangeLog | 33 ++++++++++++++++++++++++++++ Magick++/lib/Image.cpp | 21 +++++++++++++++++- Magick++/tests/attributes.cpp | 4 +- Makefile.in | 2 + VisualMagick/installer/inc/version.isx | 4 +- VisualMagick/magick/magick_types.h.in | 4 +++ coders/msl.c | 17 +++++++++----- configure | 35 ++++++++++++++++++++++++++++- configure.ac | 29 +++++++++++++++++++++++- magick/analyze.c | 2 +- magick/colorspace.c | 40 +++++++++++++++++----------------- magick/constitute.c | 3 +- magick/fx.c | 14 +++++++---- magick/gem.c | 8 +++--- magick/image.h | 2 +- magick/magick_types.h.in | 11 +++++++- magick/version.h | 4 +- utilities/tests/convert-cmds.txt | 23 +++++++++++------- utilities/tests/convert.tap | 4 ++- www/Changelog.html | 26 ++++++++++++++++++++++ 20 files changed, 226 insertions(+), 60 deletions(-) diffs (truncated from 728 to 500 lines): diff -r 84185c5ed68b -r 15545e750d39 ChangeLog --- a/ChangeLog Sat Nov 26 16:38:03 2022 -0600 +++ b/ChangeLog Sun Nov 27 15:36:03 2022 -0600 @@ -1,5 +1,38 @@ +2022-11-27 Bob Friesenhahn <[email protected]> + + * Magick++/lib/Image.cpp: Don't just blindly return DisposeMethod + as an 'int'. + + * Magick++/tests/attributes.cpp (main): Test with a valid GIF + disposal method. + + * coders/msl.c (MSLStartElement): Eliminate 'undefined behavior' + issues. + + * magick/colorspace.c (XYZTransformPackets): Use 'double' when + performing summation and eliminate 'undefined behavior' issues. + (RGBTransformPackets): Use 'double' when performing summation and + eliminate 'undefined behavior' issues. + + * magick/constitute.c (ConstituteImage): Eliminate 'undefined + behavior' issues. + + * magick/fx.c (SolarizeImage): Eliminate 'undefined behavior' + issues. + + * magick/gem.c (Hull): Eliminate 'undefined behavior' issues. + + * configure.ac, magick/magick_types.h.in: Added some support for + deducing magick_uintptr_t and magick_ptrdiff_t types. + + * utilities/tests/convert-cmds.txt: Added more command + permutations to execute. + 2022-11-26 Bob Friesenhahn <[email protected]> + * magick/constitute.c (ConstituteImage): Fix undefined behavior: + runtime error: applying non-zero offset 2 to null pointer + * coders/logo.c (RegisterLOGOImage): Logo image subformats do not have an image extension. diff -r 84185c5ed68b -r 15545e750d39 Magick++/lib/Image.cpp --- a/Magick++/lib/Image.cpp Sat Nov 26 16:38:03 2022 -0600 +++ b/Magick++/lib/Image.cpp Sun Nov 27 15:36:03 2022 -0600 @@ -3014,8 +3014,27 @@ } unsigned int Magick::Image::gifDisposeMethod ( void ) const { + unsigned int + ret = UndefinedDispose; + // FIXME: It would be better to return an enumeration - return constImage()->dispose; + switch(constImage()->dispose) + { + case UndefinedDispose: + ret = 0; + break; + case NoneDispose: + ret = 1; + break; + case BackgroundDispose: + ret = 2; + break; + case PreviousDispose: + ret = 3; + break; + } + + return ret; } // ICC ICM color profile (BLOB) diff -r 84185c5ed68b -r 15545e750d39 Magick++/tests/attributes.cpp --- a/Magick++/tests/attributes.cpp Sat Nov 26 16:38:03 2022 -0600 +++ b/Magick++/tests/attributes.cpp Sun Nov 27 15:36:03 2022 -0600 @@ -850,8 +850,8 @@ } // Test set/get - image.gifDisposeMethod(4); - if ( image.gifDisposeMethod() != 4 ) + image.gifDisposeMethod(3); + if ( image.gifDisposeMethod() != 3 ) { ++failures; cout << "Line: " << __LINE__ diff -r 84185c5ed68b -r 15545e750d39 Makefile.in --- a/Makefile.in Sat Nov 26 16:38:03 2022 -0600 +++ b/Makefile.in Sun Nov 27 15:36:03 2022 -0600 @@ -2630,6 +2630,8 @@ PTHREAD_CFLAGS = @PTHREAD_CFLAGS@ PTHREAD_CXX = @PTHREAD_CXX@ PTHREAD_LIBS = @PTHREAD_LIBS@ +PTRDIFF_F = @PTRDIFF_F@ +PTRDIFF_T = @PTRDIFF_T@ PYTHON = @PYTHON@ PrintDelegate = @PrintDelegate@ QuantumDepth = @QuantumDepth@ diff -r 84185c5ed68b -r 15545e750d39 VisualMagick/installer/inc/version.isx --- a/VisualMagick/installer/inc/version.isx Sat Nov 26 16:38:03 2022 -0600 +++ b/VisualMagick/installer/inc/version.isx Sun Nov 27 15:36:03 2022 -0600 @@ -10,5 +10,5 @@ #define public MagickPackageName "GraphicsMagick" #define public MagickPackageVersion "1.4" -#define public MagickPackageVersionAddendum ".020221126" -#define public MagickPackageReleaseDate "snapshot-20221126" +#define public MagickPackageVersionAddendum ".020221127" +#define public MagickPackageReleaseDate "snapshot-20221127" diff -r 84185c5ed68b -r 15545e750d39 VisualMagick/magick/magick_types.h.in --- a/VisualMagick/magick/magick_types.h.in Sat Nov 26 16:38:03 2022 -0600 +++ b/VisualMagick/magick/magick_types.h.in Sun Nov 27 15:36:03 2022 -0600 @@ -81,12 +81,16 @@ # define MAGICK_SIZE_T unsigned __int64 # define MAGICK_SSIZE_T_F "I64" # define MAGICK_SSIZE_T signed __int64 + typedef size_t uintptr_t; + typedef signed __int64 ptrdiff_t; # else typedef unsigned long magick_uintptr_t; # define MAGICK_SIZE_T_F "l" # define MAGICK_SIZE_T unsigned long # define MAGICK_SSIZE_T_F "" # define MAGICK_SSIZE_T int + typedef long magick_ptrdiff_t + typedef unsigned long magick_uintptr_t; # endif // defined(WIN64) #if !defined(ssize_t) && !defined(__MINGW32__) && !defined(__MINGW64__) diff -r 84185c5ed68b -r 15545e750d39 coders/msl.c --- a/coders/msl.c Sat Nov 26 16:38:03 2022 -0600 +++ b/coders/msl.c Sun Nov 27 15:36:03 2022 -0600 @@ -3240,14 +3240,19 @@ { if (LocaleCompare(keyword, "opacity") == 0) { - int opac = OpaqueOpacity; + double opac = 0.0; size_t len = strlen( value ); - opac = MagickAtoI( value ); - if ((opac > 0) && (len > 1) && value[len-1] == '%') - opac = (int)(MaxRGB * ((float)opac/100)); - - SetImageOpacity( msl_info->image[n], opac ); + if (MagickAtoFChk( value, &opac ) == MagickFail) + { + ThrowException(msl_info->exception,OptionError, + UsageError,keyword); + break; + } + if ((opac > 0.0) && (len > 1) && value[len-1] == '%') + opac = (MaxRGBDouble * (opac/100.0)); + + SetImageOpacity( msl_info->image[n], (unsigned int) opac ); break; } diff -r 84185c5ed68b -r 15545e750d39 configure --- a/configure Sat Nov 26 16:38:03 2022 -0600 +++ b/configure Sun Nov 27 15:36:03 2022 -0600 @@ -821,6 +821,8 @@ MAGICK_SSIZE_T MAGICK_SIZE_T_F MAGICK_SIZE_T +PTRDIFF_F +PTRDIFF_T UINTPTR_F UINTPTR_T UINTMAX_F @@ -21037,6 +21039,7 @@ # with Windows builds which don't use configure. # +# int8_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed 8-bit type" >&5 $as_echo_n "checking for signed 8-bit type... " >&6; } INT8_T='signed char' @@ -21044,6 +21047,7 @@ $as_echo "$INT8_T" >&6; } +# uint8_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned 8-bit type" >&5 $as_echo_n "checking for unsigned 8-bit type... " >&6; } UINT8_T='unsigned char' @@ -21051,6 +21055,7 @@ $as_echo "$UINT8_T" >&6; } +# int16_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed 16-bit type" >&5 $as_echo_n "checking for signed 16-bit type... " >&6; } INT16_T='signed short' @@ -21058,6 +21063,7 @@ $as_echo "$INT16_T" >&6; } +# uint16_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned 16-bit type" >&5 $as_echo_n "checking for unsigned 16-bit type... " >&6; } UINT16_T='unsigned short' @@ -21065,6 +21071,7 @@ $as_echo "$UINT16_T" >&6; } +# int32_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed 32-bit type" >&5 $as_echo_n "checking for signed 32-bit type... " >&6; } INT32_T='none' @@ -21083,6 +21090,7 @@ +# uint32_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned 32-bit type" >&5 $as_echo_n "checking for unsigned 32-bit type... " >&6; } UINT32_T='none' @@ -21101,6 +21109,7 @@ +# int64_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed 64-bit type" >&5 $as_echo_n "checking for signed 64-bit type... " >&6; } INT64_T='none' @@ -21124,6 +21133,7 @@ +# uint64_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned 64-bit type" >&5 $as_echo_n "checking for unsigned 64-bit type... " >&6; } UINT64_T='none' @@ -21147,6 +21157,7 @@ +# uintmax_t { $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned maximum type" >&5 $as_echo_n "checking for unsigned maximum type... " >&6; } UINTMAX_T='none' @@ -21165,8 +21176,9 @@ -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for pointer difference type" >&5 -$as_echo_n "checking for pointer difference type... " >&6; } +# uintptr_t +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for unsigned pointer storage type" >&5 +$as_echo_n "checking for unsigned pointer storage type... " >&6; } UINTPTR_T='none' UINTPTR_F='none' if test $ac_cv_sizeof_unsigned_long -eq $ac_cv_sizeof_unsigned_intp @@ -21183,6 +21195,25 @@ +# ptrdiff_t +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed pointer difference type" >&5 +$as_echo_n "checking for signed pointer difference type... " >&6; } +PTRDIFF_T='none' +PTRDDIFF_F='none' +if test $ac_cv_sizeof_unsigned_long -eq $ac_cv_sizeof_unsigned_intp +then + PTRDIFF_T='signed long' + PTRDIFF_F='"l"' +elif test $ac_cv_sizeof_unsigned_long_long -eq $ac_cv_sizeof_unsigned_intp +then + PTRDIFF_T='long long' + PTRDIFF_F='"ll"' +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $PTRDIFF_T" >&5 +$as_echo "$PTRDIFF_T" >&6; } + + + MAGICK_SIZE_T='none' MAGICK_SIZE_T_F='none' MAGICK_SSIZE_T='none' diff -r 84185c5ed68b -r 15545e750d39 configure.ac --- a/configure.ac Sat Nov 26 16:38:03 2022 -0600 +++ b/configure.ac Sun Nov 27 15:36:03 2022 -0600 @@ -1084,26 +1084,31 @@ # with Windows builds which don't use configure. # +# int8_t AC_MSG_CHECKING([for signed 8-bit type]) INT8_T='signed char' AC_MSG_RESULT([$INT8_T]) AC_SUBST([INT8_T]) +# uint8_t AC_MSG_CHECKING([for unsigned 8-bit type]) UINT8_T='unsigned char' AC_MSG_RESULT([$UINT8_T]) AC_SUBST([UINT8_T]) +# int16_t AC_MSG_CHECKING([for signed 16-bit type]) INT16_T='signed short' AC_MSG_RESULT([$INT16_T]) AC_SUBST([INT16_T]) +# uint16_t AC_MSG_CHECKING([for unsigned 16-bit type]) UINT16_T='unsigned short' AC_MSG_RESULT([$UINT16_T]) AC_SUBST([UINT16_T]) +# int32_t AC_MSG_CHECKING([for signed 32-bit type]) INT32_T='none' INT32_F='none' @@ -1120,6 +1125,7 @@ AC_SUBST([INT32_T]) AC_SUBST([INT32_F]) +# uint32_t AC_MSG_CHECKING([for unsigned 32-bit type]) UINT32_T='none' UINT32_F='none' @@ -1136,6 +1142,7 @@ AC_SUBST([UINT32_T]) AC_SUBST([UINT32_F]) +# int64_t AC_MSG_CHECKING([for signed 64-bit type]) INT64_T='none' INT64_F='none' @@ -1157,6 +1164,7 @@ AC_SUBST([INT64_T]) AC_SUBST([INT64_F]) +# uint64_t AC_MSG_CHECKING([for unsigned 64-bit type]) UINT64_T='none' UINT64_F='none' @@ -1178,6 +1186,7 @@ AC_SUBST([UINT64_T]) AC_SUBST([UINT64_F]) +# uintmax_t AC_MSG_CHECKING([for unsigned maximum type]) UINTMAX_T='none' UINTMAX_F='none' @@ -1194,7 +1203,8 @@ AC_SUBST([UINTMAX_T]) AC_SUBST([UINTMAX_F]) -AC_MSG_CHECKING([for pointer difference type]) +# uintptr_t +AC_MSG_CHECKING([for unsigned pointer storage type]) UINTPTR_T='none' UINTPTR_F='none' if test $ac_cv_sizeof_unsigned_long -eq $ac_cv_sizeof_unsigned_intp @@ -1210,6 +1220,23 @@ AC_SUBST([UINTPTR_T]) AC_SUBST([UINTPTR_F]) +# ptrdiff_t +AC_MSG_CHECKING([for signed pointer difference type]) +PTRDIFF_T='none' +PTRDDIFF_F='none' +if test $ac_cv_sizeof_unsigned_long -eq $ac_cv_sizeof_unsigned_intp +then + PTRDIFF_T='signed long' + PTRDIFF_F='"l"' +elif test $ac_cv_sizeof_unsigned_long_long -eq $ac_cv_sizeof_unsigned_intp +then + PTRDIFF_T='long long' + PTRDIFF_F='"ll"' +fi +AC_MSG_RESULT([$PTRDIFF_T]) +AC_SUBST([PTRDIFF_T]) +AC_SUBST([PTRDIFF_F]) + MAGICK_SIZE_T='none' MAGICK_SIZE_T_F='none' MAGICK_SSIZE_T='none' diff -r 84185c5ed68b -r 15545e750d39 magick/analyze.c --- a/magick/analyze.c Sat Nov 26 16:38:03 2022 -0600 +++ b/magick/analyze.c Sun Nov 27 15:36:03 2022 -0600 @@ -294,6 +294,7 @@ % % */ +#if MaxMap == MaxRGB static inline unsigned char MinimumDepthForValue(const Quantum quantum) { register unsigned int @@ -309,7 +310,6 @@ return depth; } -#if MaxMap == MaxRGB static magick_uint8_t* AllocateDepthMap(void) { magick_uint8_t diff -r 84185c5ed68b -r 15545e750d39 magick/colorspace.c --- a/magick/colorspace.c Sat Nov 26 16:38:03 2022 -0600 +++ b/magick/colorspace.c Sun Nov 27 15:36:03 2022 -0600 @@ -258,7 +258,7 @@ /* 3D transform pixels from RGB to alternate colorspace. */ - float + double b, g, r; @@ -285,17 +285,17 @@ y_index = ScaleQuantumToMap(pixels[i].green); z_index = ScaleQuantumToMap(pixels[i].blue); - r = (xform->x[x_index].x + xform->y[y_index].x + xform->z[z_index].x + xform->primary_info.x); - g = (xform->x[x_index].y + xform->y[y_index].y + xform->z[z_index].y + xform->primary_info.y); - b = (xform->x[x_index].z + xform->y[y_index].z + xform->z[z_index].z + xform->primary_info.z); + r = xform->x[x_index].x + xform->y[y_index].x + xform->z[z_index].x + xform->primary_info.x; + g = xform->x[x_index].y + xform->y[y_index].y + xform->z[z_index].y + xform->primary_info.y; + b = xform->x[x_index].z + xform->y[y_index].z + xform->z[z_index].z + xform->primary_info.z; - r = r < 0.0f ? 0.0f : r > MaxMapFloat ? MaxMapFloat : (r + 0.5f); - g = g < 0.0f ? 0.0f : g > MaxMapFloat ? MaxMapFloat : (g + 0.5f); - b = b < 0.0f ? 0.0f : b > MaxMapFloat ? MaxMapFloat : (b + 0.5f); + r = r < 0.0 ? 0.0 : r > MaxMapDouble ? MaxMapDouble : (r + 0.5); + g = g < 0.0 ? 0.0 : g > MaxMapDouble ? MaxMapDouble : (g + 0.5); + b = b < 0.0 ? 0.0 : b > MaxMapDouble ? MaxMapDouble : (b + 0.5); - pixels[i].red = ScaleMapToQuantum((Quantum) r); - pixels[i].green = ScaleMapToQuantum((Quantum) g); - pixels[i].blue = ScaleMapToQuantum((Quantum) b); + pixels[i].red = ScaleMapToQuantum(floor(r)); + pixels[i].green = ScaleMapToQuantum(floor(g)); + pixels[i].blue = ScaleMapToQuantum(floor(b)); } return MagickPass; @@ -1266,7 +1266,7 @@ /* 3D transform pixels to RGB. */ - float + double b, g, r; @@ -1293,13 +1293,13 @@ g_index = ScaleQuantumToMap(pixels[i].green); b_index = ScaleQuantumToMap(pixels[i].blue); - r = (xform->r[r_index].r + xform->g[g_index].r + xform->b[b_index].r); - g = (xform->r[r_index].g + xform->g[g_index].g + xform->b[b_index].g); - b = (xform->r[r_index].b + xform->g[g_index].b + xform->b[b_index].b); + r = xform->r[r_index].r + xform->g[g_index].r + xform->b[b_index].r; + g = xform->r[r_index].g + xform->g[g_index].g + xform->b[b_index].g; + b = xform->r[r_index].b + xform->g[g_index].b + xform->b[b_index].b; - r = r < 0.0f ? 0.0f : r > MaxMapFloat ? MaxMapFloat : (r + 0.5f); - g = g < 0.0f ? 0.0f : g > MaxMapFloat ? MaxMapFloat : (g + 0.5f); - b = b < 0.0f ? 0.0f : b > MaxMapFloat ? MaxMapFloat : (b + 0.5f); + r = r < 0.0 ? 0.0 : r > MaxMapDouble ? MaxMapDouble : (r + 0.5); + g = g < 0.0 ? 0.0 : g > MaxMapDouble ? MaxMapDouble : (g + 0.5); + b = b < 0.0 ? 0.0 : b > MaxMapDouble ? MaxMapDouble : (b + 0.5); if ( xform->rgb_map != 0 ) { @@ -1317,9 +1317,9 @@ } else { - pixels[i].red = ScaleMapToQuantum(r); - pixels[i].green = ScaleMapToQuantum(g); - pixels[i].blue = ScaleMapToQuantum(b); + pixels[i].red = ScaleMapToQuantum(floor(r)); + pixels[i].green = ScaleMapToQuantum(floor(g)); + pixels[i].blue = ScaleMapToQuantum(floor(b)); } } diff -r 84185c5ed68b -r 15545e750d39 magick/constitute.c --- a/magick/constitute.c Sat Nov 26 16:38:03 2022 -0600 +++ b/magick/constitute.c Sun Nov 27 15:36:03 2022 -0600 @@ -569,7 +569,8 @@ } } } - indexes++; + if (NULL != (IndexPacket *) indexes) + indexes++; q++; } if (!SyncImagePixels(image)) diff -r 84185c5ed68b -r 15545e750d39 magick/fx.c --- a/magick/fx.c Sat Nov 26 16:38:03 2022 -0600 +++ b/magick/fx.c Sun Nov 27 15:36:03 2022 -0600 @@ -1222,8 +1222,8 @@