From 8d4110bd618eb03a252b18456c546cef0a8475bb Mon Sep 17 00:00:00 2001 From: John Bowler Date: Mon, 26 Dec 2016 16:24:40 -0800 Subject: Remove 'defined' within macro This removes the use of a macro containing the pre-processor 'defined' operator. It is unclear whether this is valid; a macro which "generates" 'defined' is not permitted, but the use of the work "generates" within the C90 standard seems to imply more than simple substitution of an expression itself containing a well-formed defined operation. Signed-off-by: John Bowler --- contrib/libtests/pngvalid.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index 9b85c3ef3..330e26393 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -4010,8 +4010,11 @@ check_interlace_type(int const interlace_type) # define do_own_interlace 1 #endif /* WRITE_INTERLACING tests */ -#define CAN_WRITE_INTERLACE\ - PNG_LIBPNG_VER >= 10700 || defined PNG_WRITE_INTERLACING_SUPPORTED +#if PNG_LIBPNG_VER >= 10700 || defined PNG_WRITE_INTERLACING_SUPPORTED +# define CAN_WRITE_INTERLACE 1 +#else +# define CAN_WRITE_INTERLACE 0 +#endif /* Do the same thing for read interlacing; this controls whether read tests do * their own de-interlace or use libpng. -- cgit v1.2.1 From 09dcb906a7fc7ccd92c9c3d225b593ba8c9df29d Mon Sep 17 00:00:00 2001 From: John Bowler Date: Mon, 26 Dec 2016 16:27:33 -0800 Subject: Remove unsigned overflow The previous code always results in an unsigned arithmetic overflow, this is well defined but produces errors from clang with the option to detect unsigned overflow. As the expression only gets evaluated once per row in this version of libpng it is easier just to rewrite it. Signed-off-by: John Bowler --- pngtrans.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pngtrans.c b/pngtrans.c index a5df5afe0..dcd4d98b4 100644 --- a/pngtrans.c +++ b/pngtrans.c @@ -629,12 +629,16 @@ png_do_check_palette_indexes(png_structp png_ptr, png_row_infop row_info) png_ptr->num_palette > 0) /* num_palette can be 0 in MNG files */ { /* Calculations moved outside switch in an attempt to stop different - * compiler warnings. 'padding' is in *bits* within the last byte, it is - * an 'int' because pixel_depth becomes an 'int' in the expression below, - * and this calculation is used because it avoids warnings that other - * forms produced on either GCC or MSVC. + * compiler warnings. + * + * 1.5.28: This rewritten version attempts to remove the unsigned integer + * overflow from the prior version. While this was well defined it + * resulted in unsigned overflow detection in clang. Since the result is + * always in the range 0..7 only the low three bits of of the various + * intermediates are every required, so: */ - int padding = (-row_info->pixel_depth * row_info->width) & 7; + unsigned int padding = + ((8 - (row_info->pixel_depth & 7)) * (row_info->width & 7)) & 7; png_bytep rp = png_ptr->row_buf + row_info->rowbytes; switch (row_info->bit_depth) -- cgit v1.2.1 From 95e0094f85c36cfeec0a1385cb932157236c4231 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Mon, 26 Dec 2016 16:29:16 -0800 Subject: Eliminate signed overlow in png_64bit_product The previous version produced a signed overflow as a result of both the & 0xffff on the most significant bits of a negative argument; this converted (-1) into 65535 which resulted in a subsequent overflow. Since signed overflow is undefined in C90 the code has been modified to correctly calculate a signed result. This requires changing the 'hi' result parametr to a signed value. This has been code reviewed solely by the author. A further code review is highly desireable. Nevertheless the code compiles without warnings from clang and without the prior detection of an overflow. Since it no longer truncates any of the intermediate values this should be enough to ensure that it is correct. Signed-off-by: John Bowler --- png.c | 31 ++++++++++++++++++------------- pngpriv.h | 7 ------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/png.c b/png.c index cb6708f3a..c99076a03 100644 --- a/png.c +++ b/png.c @@ -776,6 +776,8 @@ png_access_version_number(void) /* Added at libpng version 1.2.34 and 1.4.0 (moved from pngset.c) */ # ifdef PNG_CHECK_cHRM_SUPPORTED +static void png_64bit_product(long v1, long v2, long *hi_product, + unsigned long *lo_product); int /* PRIVATE */ png_check_cHRM_fixed(png_structp png_ptr, @@ -784,7 +786,8 @@ png_check_cHRM_fixed(png_structp png_ptr, png_fixed_point blue_x, png_fixed_point blue_y) { int ret = 1; - unsigned long xy_hi,xy_lo,yx_hi,yx_lo; + long xy_hi,yx_hi; + unsigned long xy_lo,yx_lo; png_debug(1, "in function png_check_cHRM_fixed"); @@ -2169,29 +2172,31 @@ png_reciprocal2(png_fixed_point a, png_fixed_point b) * A and D, and X || Y is (X << 16) + Y. */ -void /* PRIVATE */ -png_64bit_product (long v1, long v2, unsigned long *hi_product, +static void +png_64bit_product(long v1, long v2, long *hi_product, unsigned long *lo_product) { - int a, b, c, d; - long lo, hi, x, y; + long a, b, c, d; + unsigned long lo; + long hi, x, y; - a = (v1 >> 16) & 0xffff; + a = v1 >> 16; b = v1 & 0xffff; - c = (v2 >> 16) & 0xffff; + c = v2 >> 16; d = v2 & 0xffff; - lo = b * d; /* BD */ + lo = b; + lo *= d; /* BD */ x = a * d + c * b; /* AD + CB */ - y = ((lo >> 16) & 0xffff) + x; + y = (lo >> 16) + x; - lo = (lo & 0xffff) | ((y & 0xffff) << 16); - hi = (y >> 16) & 0xffff; + lo = (lo & 0xffff) | ((y & 0xffffU) << 16); + hi = y >> 16; hi += a * c; /* AC */ - *hi_product = (unsigned long)hi; - *lo_product = (unsigned long)lo; + *hi_product = hi; + *lo_product = lo; } #endif /* CHECK_cHRM */ diff --git a/pngpriv.h b/pngpriv.h index 8bdccccaf..e6ca9bf0d 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -1362,13 +1362,6 @@ PNG_EXTERN int png_check_cHRM_fixed PNGARG((png_structp png_ptr, png_fixed_point int_blue_y)); #endif -#ifdef PNG_CHECK_cHRM_SUPPORTED -/* Added at libpng version 1.2.34 and 1.4.0 */ -/* Currently only used by png_check_cHRM_fixed */ -PNG_EXTERN void png_64bit_product PNGARG((long v1, long v2, - unsigned long *hi_product, unsigned long *lo_product)); -#endif - #ifdef PNG_cHRM_SUPPORTED /* Added at libpng version 1.5.5 */ typedef struct png_xy -- cgit v1.2.1