Skip to content

Commit

Permalink
Avoid tautological comparisons
Browse files Browse the repository at this point in the history
Several TurboJPEG functions store their return value in an unsigned
long long intermediate and compare it against the maximum value of
unsigned long or size_t in order to avoid integer overflow.  However,
such comparisons are tautological (always true, i.e. redundant) unless
the size of unsigned long or size_t is less than the size of unsigned
long long.  Explicitly guarding the comparisons with #if avoids compiler
warnings with -Wtautological-constant-in-range-compare in Clang and also
makes it clear to the reader that the comparisons are only intended for
32-bit code.

Refer to libjpeg-turbo#752
  • Loading branch information
dcommander committed Mar 8, 2024
1 parent 34c0558 commit 905ec0f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 2 deletions.
7 changes: 7 additions & 0 deletions tjbench.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#include <math.h>
#include <errno.h>
#include <limits.h>
#if !defined(_MSC_VER) || _MSC_VER > 1600
#include <stdint.h>
#endif
#include <cdjpeg.h>
#include "./tjutil.h"
#include "./turbojpeg.h"
Expand Down Expand Up @@ -223,9 +226,11 @@ static int decomp(unsigned char **jpegBufs, size_t *jpegSizes, void *dstBuf,
pitch = scaledw * ps;

if (dstBuf == NULL) {
#if ULLONG_MAX > SIZE_MAX
if ((unsigned long long)pitch * (unsigned long long)scaledh *
(unsigned long long)sampleSize > (unsigned long long)((size_t)-1))
THROW("allocating destination buffer", "Image is too large");
#endif
if ((dstBuf = malloc((size_t)pitch * scaledh * sampleSize)) == NULL)
THROW_UNIX("allocating destination buffer");
dstBufAlloc = 1;
Expand Down Expand Up @@ -382,9 +387,11 @@ static int fullTest(tjhandle handle, void *srcBuf, int w, int h, int subsamp,
int ntilesw = 1, ntilesh = 1, pitch = w * ps;
const char *pfStr = pixFormatStr[pf];

#if ULLONG_MAX > SIZE_MAX
if ((unsigned long long)pitch * (unsigned long long)h *
(unsigned long long)sampleSize > (unsigned long long)((size_t)-1))
THROW("allocating temporary image buffer", "Image is too large");
#endif
if ((tmpBuf = malloc((size_t)pitch * h * sampleSize)) == NULL)
THROW_UNIX("allocating temporary image buffer");

Expand Down
6 changes: 6 additions & 0 deletions tjexample.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <limits.h>
#if !defined(_MSC_VER) || _MSC_VER > 1600
#include <stdint.h>
#endif
#include <turbojpeg.h>


Expand Down Expand Up @@ -335,9 +339,11 @@ int main(int argc, char **argv)
outSubsamp = inSubsamp;

pixelFormat = TJPF_BGRX;
#if ULLONG_MAX > SIZE_MAX
if ((unsigned long long)width * height * tjPixelSize[pixelFormat] >
(unsigned long long)((size_t)-1))
THROW("allocating uncompressed image buffer", "Image is too large");
#endif
if ((imgBuf =
(unsigned char *)malloc(sizeof(unsigned char) * width * height *
tjPixelSize[pixelFormat])) == NULL)
Expand Down
7 changes: 5 additions & 2 deletions turbojpeg-mp.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C)2009-2023 D. R. Commander. All Rights Reserved.
* Copyright (C)2009-2024 D. R. Commander. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
Expand Down Expand Up @@ -366,8 +366,11 @@ DLLEXPORT _JSAMPLE *GET_NAME(tj3LoadImage, BITS_IN_JSAMPLE)
*pixelFormat = cs2pf[cinfo->in_color_space];

pitch = PAD((*width) * tjPixelSize[*pixelFormat], align);
if ((unsigned long long)pitch * (unsigned long long)(*height) >
if (
#if ULLONG_MAX > SIZE_MAX
(unsigned long long)pitch * (unsigned long long)(*height) >
(unsigned long long)((size_t)-1) ||
#endif
(dstBuf = (_JSAMPLE *)malloc(pitch * (*height) *
sizeof(_JSAMPLE))) == NULL)
THROW("Memory allocation failure");
Expand Down
11 changes: 11 additions & 0 deletions turbojpeg.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@

#include <ctype.h>
#include <limits.h>
#if !defined(_MSC_VER) || _MSC_VER > 1600
#include <stdint.h>
#endif
#include <jinclude.h>
#define JPEG_INTERNALS
#include <jpeglib.h>
Expand Down Expand Up @@ -946,8 +949,10 @@ DLLEXPORT size_t tj3JPEGBufSize(int width, int height, int jpegSubsamp)
mcuh = tjMCUHeight[jpegSubsamp];
chromasf = jpegSubsamp == TJSAMP_GRAY ? 0 : 4 * 64 / (mcuw * mcuh);
retval = PAD(width, mcuw) * PAD(height, mcuh) * (2ULL + chromasf) + 2048ULL;
#if ULLONG_MAX > ULONG_MAX
if (retval > (unsigned long long)((unsigned long)-1))
THROWG("Image is too large", 0);
#endif

bailout:
return (size_t)retval;
Expand Down Expand Up @@ -981,8 +986,10 @@ DLLEXPORT unsigned long TJBUFSIZE(int width, int height)
larger than the uncompressed input (we wouldn't mention it if it hadn't
happened before.) */
retval = PAD(width, 16) * PAD(height, 16) * 6ULL + 2048ULL;
#if ULLONG_MAX > ULONG_MAX
if (retval > (unsigned long long)((unsigned long)-1))
THROWG("Image is too large", (unsigned long)-1);
#endif

bailout:
return (unsigned long)retval;
Expand All @@ -1008,8 +1015,10 @@ DLLEXPORT size_t tj3YUVBufSize(int width, int align, int height, int subsamp)
if (pw == 0 || ph == 0) return 0;
else retval += (unsigned long long)stride * ph;
}
#if ULLONG_MAX > ULONG_MAX
if (retval > (unsigned long long)((unsigned long)-1))
THROWG("Image is too large", 0);
#endif

bailout:
return (size_t)retval;
Expand Down Expand Up @@ -1123,8 +1132,10 @@ DLLEXPORT size_t tj3YUVPlaneSize(int componentID, int width, int stride,
else stride = abs(stride);

retval = (unsigned long long)stride * (ph - 1) + pw;
#if ULLONG_MAX > ULONG_MAX
if (retval > (unsigned long long)((unsigned long)-1))
THROWG("Image is too large", 0);
#endif

bailout:
return (size_t)retval;
Expand Down

0 comments on commit 905ec0f

Please sign in to comment.