diff --git a/ANNOUNCE b/ANNOUNCE index 43d4753cb..519f140c9 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -1,5 +1,5 @@ -Libpng 1.7.0beta40 - November 5, 2014 +Libpng 1.7.0beta40 - November 6, 2014 This is not intended to be a public release. It will be replaced within a few weeks by a public version or by another test version. @@ -620,13 +620,28 @@ Version 1.7.0beta38 [October 17, 2014] Version 1.7.0beta39 [November 1, 2014] Ported cosmetic changes from libpng-1.6.15beta02. -Version 1.7.0beta40 [November 5, 2014] +Version 1.7.0beta40 [November 6, 2014] Made a one-line revision to configure.ac to support ARM on aarch64 (bug report by Marcin Juszkiewicz, fix by John Bowler). Use png_get_libpng_ver(NULL) instead of PNG_LIBPNG_VER_STRING in example.c, pngtest.c, and applications in the contrib directory. Avoid out-of-bounds memory access in png_user_version_check(). Simplified and future-proofed png_user_version_check(). + Fixed GCC unsigned int->float warnings. Various versions of GCC + seem to generate warnings when an unsigned value is implicitly + converted to double. This is probably a GCC bug but this change + avoids the issue by explicitly converting to (int) where safe. + Free all allocated memory in pngimage. The file buffer cache was left + allocated at the end of the program, harmless but it causes memory + leak reports from clang. + Fixed array size calculations to avoid warnings. At various points + in the code the number of elements in an array is calculated using + sizeof. This generates a compile time constant of type (size_t) which + is then typically assigned to an (unsigned int) or (int). Some versions + of GCC on 64-bit systems warn about the apparent narrowing, even though + the same compiler does apparently generate the correct, in-range, + numeric constant. This adds appropriate, safe, casts to make the + warnings go away. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/CHANGES b/CHANGES index 378dc56ed..5df89cd3e 100644 --- a/CHANGES +++ b/CHANGES @@ -4909,13 +4909,28 @@ Version 1.7.0beta38 [October 17, 2014] Version 1.7.0beta39 [November 1, 2014] Ported cosmetic changes from libpng-1.6.15beta02. -Version 1.7.0beta40 [November 5, 2014] +Version 1.7.0beta40 [November 6, 2014] Made a one-line revision to configure.ac to support ARM on aarch64 (bug report by Marcin Juszkiewicz, fix by John Bowler). Use png_get_libpng_ver(NULL) instead of PNG_LIBPNG_VER_STRING in example.c, pngtest.c, and applications in the contrib directory. Avoid out-of-bounds memory access in png_user_version_check(). Simplified and future-proofed png_user_version_check(). + Fixed GCC unsigned int->float warnings. Various versions of GCC + seem to generate warnings when an unsigned value is implicitly + converted to double. This is probably a GCC bug but this change + avoids the issue by explicitly converting to (int) where safe. + Free all allocated memory in pngimage. The file buffer cache was left + allocated at the end of the program, harmless but it causes memory + leak reports from clang. + Fixed array size calculations to avoid warnings. At various points + in the code the number of elements in an array is calculated using + sizeof. This generates a compile time constant of type (size_t) which + is then typically assigned to an (unsigned int) or (int). Some versions + of GCC on 64-bit systems warn about the apparent narrowing, even though + the same compiler does apparently generate the correct, in-range, + numeric constant. This adds appropriate, safe, casts to make the + warnings go away. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/contrib/libtests/pngimage.c b/contrib/libtests/pngimage.c index 372845ead..dccfbce12 100644 --- a/contrib/libtests/pngimage.c +++ b/contrib/libtests/pngimage.c @@ -337,6 +337,9 @@ validate_T(void) * In both cases the file data is held in a linked list of buffers - not all * of these are in use at any time. */ +#define NEW(type) ((type *)malloc(sizeof (type))) +#define DELETE(ptr) (free(ptr)) + struct buffer_list { struct buffer_list *next; /* next buffer in list */ @@ -361,6 +364,25 @@ buffer_init(struct buffer *buffer) buffer->current = NULL; } +static void +buffer_destroy_list(struct buffer_list *list) +{ + if (list != NULL) + { + struct buffer_list *next = list->next; + DELETE(list); + buffer_destroy_list(next); + } +} + +static void +buffer_destroy(struct buffer *buffer) +{ + struct buffer_list *list = buffer->first.next; + buffer_init(buffer); + buffer_destroy_list(list); +} + #ifdef PNG_WRITE_SUPPORTED static void buffer_start_write(struct buffer *buffer) @@ -390,8 +412,6 @@ get_buffer(png_structp pp) return (struct buffer*)png_get_io_ptr(pp); } -#define NEW(type) ((type *)malloc(sizeof (type))) - static struct buffer_list * buffer_extend(struct buffer_list *current) { @@ -598,6 +618,17 @@ display_clean(struct display *dp) dp->results = 0; /* reset for next time */ } +static void +display_destroy(struct display *dp) +{ + /* Release any memory held in the display. */ +# ifdef PNG_WRITE_SUPPORTED + buffer_destroy(&dp->written_file); +# endif + + buffer_destroy(&dp->original_file); +} + static struct display * get_dp(png_structp pp) /* The display pointer is always stored in the png_struct error pointer */ @@ -1605,6 +1636,9 @@ main(const int argc, const char * const * const argv) display_clean(&d); } + /* Release allocated memory */ + display_destroy(&d); + return errors != 0; } } diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index 6701c19a0..9feef7261 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -157,6 +157,13 @@ define_exception_type(struct png_store*); &(ps)->exception_context #define context(ps,fault) anon_context(ps); png_store *fault +/* This macro returns the number of elements in an array as an (unsigned int), + * it is necessary to avoid the inability of certain versions of GCC to use + * the value of a compile-time constant when performing range checks. It must + * be passed an array name. + */ +#define ARRAY_SIZE(a) ((unsigned int)((sizeof (a))/(sizeof (a)[0]))) + /******************************* UTILITIES ************************************/ /* Error handling is particularly problematic in production code - error * handlers often themselves have bugs which lead to programs that detect @@ -4106,7 +4113,7 @@ make_errors(png_modifier* PNG_CONST pm, png_byte PNG_CONST colour_type, standard_name(name, sizeof name, 0, colour_type, 1<this, colour_type, DEPTH(bdlo), interlace_type, test, name); @@ -10084,12 +10091,12 @@ int main(int argc, char **argv) /* Store the test gammas */ pm.gammas = gammas; - pm.ngammas = (sizeof gammas) / (sizeof gammas[0]); + pm.ngammas = ARRAY_SIZE(gammas); pm.ngamma_tests = 0; /* default to off */ /* And the test encodings */ pm.encodings = test_encodings; - pm.nencodings = (sizeof test_encodings) / (sizeof test_encodings[0]); + pm.nencodings = ARRAY_SIZE(test_encodings); pm.sbitlow = 8U; /* because libpng doesn't do sBIT below 8! */ diff --git a/png.c b/png.c index a0f0a2471..ec6da7139 100644 --- a/png.c +++ b/png.c @@ -691,13 +691,13 @@ png_get_copyright(png_const_structrp png_ptr) #else # ifdef __STDC__ return PNG_STRING_NEWLINE \ - "libpng version 1.7.0beta40 - November 5, 2014" PNG_STRING_NEWLINE \ + "libpng version 1.7.0beta40 - November 6, 2014" PNG_STRING_NEWLINE \ "Copyright (c) 1998-2014 Glenn Randers-Pehrson" PNG_STRING_NEWLINE \ "Copyright (c) 1996-1997 Andreas Dilger" PNG_STRING_NEWLINE \ "Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc." \ PNG_STRING_NEWLINE; # else - return "libpng version 1.7.0beta40 - November 5, 2014\ + return "libpng version 1.7.0beta40 - November 6, 2014\ Copyright (c) 1998-2014 Glenn Randers-Pehrson\ Copyright (c) 1996-1997 Andreas Dilger\ Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc."; @@ -3621,7 +3621,31 @@ png_gamma_8bit_correct(unsigned int value, png_fixed_point gamma_val) if (value > 0 && value < 255) { # ifdef PNG_FLOATING_ARITHMETIC_SUPPORTED - double r = floor(255*pow(value/255.,gamma_val*.00001)+.5); + /* 'value' is unsigned, ANSI-C90 requires the compiler to correctly + * convert this to a floating point value. This includes values that + * would overflow if 'value' were to be converted to 'int'. + * + * Apparently GCC, however, does an intermediate convertion to (int) + * on some (ARM) but not all (x86) platforms, possibly because of + * hardware FP limitations. (E.g. if the hardware convertion always + * assumes the integer register contains a signed value.) This results + * in ANSI-C undefined behavior for large values. + * + * Other implementations on the same machine might actually be ANSI-C90 + * conformant and therefore compile spurious extra code for the large + * values. + * + * We can be reasonably sure that an unsigned to float convertion + * won't be faster than an int to float one. Therefore this code + * assumes responsibility for the undefined behavior, which it knows + * can't happen because of the check above. + * + * Note the argument to this routine is an (unsigned int) because, on + * 16-bit platforms, it is assigned a value which might be out of + * range for an (int); that would result in undefined behavior in the + * caller if the *argument* ('value') were to be declared (int). + */ + double r = floor(255*pow((int)/*SAFE*/value/255.,gamma_val*.00001)+.5); return (png_byte)r; # else png_int_32 lg2 = png_log8bit(value); @@ -3644,7 +3668,13 @@ png_gamma_16bit_correct(unsigned int value, png_fixed_point gamma_val) if (value > 0 && value < 65535) { # ifdef PNG_FLOATING_ARITHMETIC_SUPPORTED - double r = floor(65535*pow(value/65535.,gamma_val*.00001)+.5); + /* The same (unsigned int)->(double) constraints apply here as above, + * however in this case the (unsigned int) to (int) convertion can + * overflow on an ANSI-C90 compliant system so the cast needs to ensure + * that this is not possible. + */ + double r = floor(65535.*pow((png_int_32)value/65535., + gamma_val*.00001)+.5); return (png_uint_16)r; # else png_int_32 lg2 = png_log16bit(value); diff --git a/pngread.c b/pngread.c index 567b5f1a1..f67d4a0c5 100644 --- a/pngread.c +++ b/pngread.c @@ -1618,7 +1618,7 @@ png_image_skip_unused_chunks(png_structrp png_ptr) /* But do not ignore image data handling chunks */ png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_AS_DEFAULT, - chunks_to_process, (sizeof chunks_to_process)/5); + chunks_to_process, (int)/*SAFE*/(sizeof chunks_to_process)/5); } } diff --git a/pngset.c b/pngset.c index 92164517c..e1cb50056 100644 --- a/pngset.c +++ b/pngset.c @@ -1373,7 +1373,7 @@ png_set_keep_unknown_chunks(png_structrp png_ptr, int keep, }; chunk_list = chunks_to_ignore; - num_chunks = (sizeof chunks_to_ignore)/5; + num_chunks = (unsigned int)/*SAFE*/(sizeof chunks_to_ignore)/5U; } else /* num_chunks_in > 0 */