From fb5fd67ccb78829074577cb2aaa4008c00acd776 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 24 Nov 2016 21:41:09 -0500 Subject: [PATCH] Fixed all known static analysis bugs, with checker-279 on macOS. --- src/libm/e_rem_pio2.c | 9 +- src/libm/k_rem_pio2.c | 9 +- src/render/SDL_render.c | 134 ++++++++++++++++++------------ src/render/SDL_yuv_sw.c | 32 +++++-- src/render/opengl/SDL_render_gl.c | 15 ++-- src/video/SDL_RLEaccel.c | 7 +- src/video/SDL_rect.c | 8 +- src/video/SDL_video.c | 11 ++- 8 files changed, 148 insertions(+), 77 deletions(-) diff --git a/src/libm/e_rem_pio2.c b/src/libm/e_rem_pio2.c index a8ffe31421bea..dfa5d941c976b 100644 --- a/src/libm/e_rem_pio2.c +++ b/src/libm/e_rem_pio2.c @@ -24,6 +24,8 @@ static const char rcsid[] = #include "math_libm.h" #include "math_private.h" +#include "SDL_assert.h" + libm_hidden_proto(fabs) /* @@ -189,7 +191,12 @@ __ieee754_rem_pio2(x, y) } tx[2] = z; nx = 3; - while (tx[nx - 1] == zero) + + /* If this assertion ever fires, here's the static analysis that warned about it: + http://buildbot.libsdl.org/sdl-static-analysis/sdl-macosx-static-analysis/sdl-macosx-static-analysis-1101/report-8c6ccb.html#EndPath */ + SDL_assert((tx[0] != zero) || (tx[1] != zero) || (tx[2] != zero)); + + while (nx && tx[nx - 1] == zero) nx--; /* skip zero term */ n = __kernel_rem_pio2(tx, y, e0, nx, 2, two_over_pi); if (hx < 0) { diff --git a/src/libm/k_rem_pio2.c b/src/libm/k_rem_pio2.c index 23c2b61dd9185..50e1d52fb4157 100644 --- a/src/libm/k_rem_pio2.c +++ b/src/libm/k_rem_pio2.c @@ -204,8 +204,13 @@ __kernel_rem_pio2(x, y, e0, nx, prec, ipio2) /* compute q[0],q[1],...q[jk] */ for (i = 0; i <= jk; i++) { - for (j = 0, fw = 0.0; j <= jx; j++) - fw += x[j] * f[jx + i - j]; + for (j = 0, fw = 0.0; j <= jx; j++) { + const int32_t idx = jx + i - j; + SDL_assert(idx >= 0); + SDL_assert(idx < 20); + SDL_assert(idx <= m); + fw += x[j] * f[idx]; + } q[i] = fw; } diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c index 94750810d8bfc..e562d2e22db38 100644 --- a/src/render/SDL_render.c +++ b/src/render/SDL_render.c @@ -734,8 +734,8 @@ SDL_UpdateTextureYUV(SDL_Texture * texture, const SDL_Rect * rect, if (texture->access == SDL_TEXTUREACCESS_STREAMING) { /* We can lock the texture and copy to it */ - void *native_pixels; - int native_pitch; + void *native_pixels = NULL; + int native_pitch = 0; if (SDL_LockTexture(native, rect, &native_pixels, &native_pitch) < 0) { return -1; @@ -745,18 +745,18 @@ SDL_UpdateTextureYUV(SDL_Texture * texture, const SDL_Rect * rect, SDL_UnlockTexture(native); } else { /* Use a temporary buffer for updating */ - void *temp_pixels; - int temp_pitch; - - temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); - temp_pixels = SDL_malloc(rect->h * temp_pitch); - if (!temp_pixels) { - return SDL_OutOfMemory(); + const int temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); + const size_t alloclen = rect->h * temp_pitch; + if (alloclen > 0) { + void *temp_pixels = SDL_malloc(alloclen); + if (!temp_pixels) { + return SDL_OutOfMemory(); + } + SDL_SW_CopyYUVToRGB(texture->yuv, rect, native->format, + rect->w, rect->h, temp_pixels, temp_pitch); + SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); + SDL_free(temp_pixels); } - SDL_SW_CopyYUVToRGB(texture->yuv, rect, native->format, - rect->w, rect->h, temp_pixels, temp_pitch); - SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); - SDL_free(temp_pixels); } return 0; } @@ -767,10 +767,14 @@ SDL_UpdateTextureNative(SDL_Texture * texture, const SDL_Rect * rect, { SDL_Texture *native = texture->native; + if (!rect->w || !rect->h) { + return 0; /* nothing to do. */ + } + if (texture->access == SDL_TEXTUREACCESS_STREAMING) { /* We can lock the texture and copy to it */ - void *native_pixels; - int native_pitch; + void *native_pixels = NULL; + int native_pitch = 0; if (SDL_LockTexture(native, rect, &native_pixels, &native_pitch) < 0) { return -1; @@ -781,19 +785,19 @@ SDL_UpdateTextureNative(SDL_Texture * texture, const SDL_Rect * rect, SDL_UnlockTexture(native); } else { /* Use a temporary buffer for updating */ - void *temp_pixels; - int temp_pitch; - - temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); - temp_pixels = SDL_malloc(rect->h * temp_pitch); - if (!temp_pixels) { - return SDL_OutOfMemory(); + const int temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); + const size_t alloclen = rect->h * temp_pitch; + if (alloclen > 0) { + void *temp_pixels = SDL_malloc(alloclen); + if (!temp_pixels) { + return SDL_OutOfMemory(); + } + SDL_ConvertPixels(rect->w, rect->h, + texture->format, pixels, pitch, + native->format, temp_pixels, temp_pitch); + SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); + SDL_free(temp_pixels); } - SDL_ConvertPixels(rect->w, rect->h, - texture->format, pixels, pitch, - native->format, temp_pixels, temp_pitch); - SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); - SDL_free(temp_pixels); } return 0; } @@ -853,10 +857,14 @@ SDL_UpdateTextureYUVPlanar(SDL_Texture * texture, const SDL_Rect * rect, full_rect.h = texture->h; rect = &full_rect; + if (!rect->w || !rect->h) { + return 0; /* nothing to do. */ + } + if (texture->access == SDL_TEXTUREACCESS_STREAMING) { /* We can lock the texture and copy to it */ - void *native_pixels; - int native_pitch; + void *native_pixels = NULL; + int native_pitch = 0; if (SDL_LockTexture(native, rect, &native_pixels, &native_pitch) < 0) { return -1; @@ -866,18 +874,18 @@ SDL_UpdateTextureYUVPlanar(SDL_Texture * texture, const SDL_Rect * rect, SDL_UnlockTexture(native); } else { /* Use a temporary buffer for updating */ - void *temp_pixels; - int temp_pitch; - - temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); - temp_pixels = SDL_malloc(rect->h * temp_pitch); - if (!temp_pixels) { - return SDL_OutOfMemory(); + const int temp_pitch = (((rect->w * SDL_BYTESPERPIXEL(native->format)) + 3) & ~3); + const size_t alloclen = rect->h * temp_pitch; + if (alloclen > 0) { + void *temp_pixels = SDL_malloc(alloclen); + if (!temp_pixels) { + return SDL_OutOfMemory(); + } + SDL_SW_CopyYUVToRGB(texture->yuv, rect, native->format, + rect->w, rect->h, temp_pixels, temp_pitch); + SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); + SDL_free(temp_pixels); } - SDL_SW_CopyYUVToRGB(texture->yuv, rect, native->format, - rect->w, rect->h, temp_pixels, temp_pitch); - SDL_UpdateTexture(native, rect, temp_pixels, temp_pitch); - SDL_free(temp_pixels); } return 0; } @@ -924,6 +932,10 @@ int SDL_UpdateYUVTexture(SDL_Texture * texture, const SDL_Rect * rect, rect = &full_rect; } + if (!rect->w || !rect->h) { + return 0; /* nothing to do. */ + } + if (texture->yuv) { return SDL_UpdateTextureYUVPlanar(texture, rect, Yplane, Ypitch, Uplane, Upitch, Vplane, Vpitch); } else { @@ -1882,6 +1894,24 @@ SDL_RenderPresent(SDL_Renderer * renderer) renderer->RenderPresent(renderer); } +/* this isn't responsible for removing the deleted texture from the list! + (this is to keep static analysis happy in SDL_DestroyRenderer().) */ +static void +SDL_DestroyTextureInternal(SDL_Texture * texture) +{ + SDL_Renderer *renderer = texture->renderer; + if (texture->native) { + SDL_DestroyTexture(texture->native); + } + if (texture->yuv) { + SDL_SW_DestroyYUVTexture(texture->yuv); + } + SDL_free(texture->pixels); + + renderer->DestroyTexture(renderer, texture); + SDL_free(texture); +} + void SDL_DestroyTexture(SDL_Texture * texture) { @@ -1894,7 +1924,7 @@ SDL_DestroyTexture(SDL_Texture * texture) SDL_SetRenderTarget(renderer, NULL); } - texture->magic = NULL; + texture->magic = NULL; /* just in case, but we're about to free this... */ if (texture->next) { texture->next->prev = texture->prev; @@ -1905,29 +1935,27 @@ SDL_DestroyTexture(SDL_Texture * texture) renderer->textures = texture->next; } - if (texture->native) { - SDL_DestroyTexture(texture->native); - } - if (texture->yuv) { - SDL_SW_DestroyYUVTexture(texture->yuv); - } - SDL_free(texture->pixels); - - renderer->DestroyTexture(renderer, texture); - SDL_free(texture); + SDL_DestroyTextureInternal(texture); } void SDL_DestroyRenderer(SDL_Renderer * renderer) { + SDL_Texture *texture = NULL; + SDL_Texture *nexttexture = NULL; + CHECK_RENDERER_MAGIC(renderer, ); SDL_DelEventWatch(SDL_RendererEventWatch, renderer); /* Free existing textures for this renderer */ - while (renderer->textures) { - SDL_DestroyTexture(renderer->textures); + SDL_SetRenderTarget(renderer, NULL); + + for (texture = renderer->textures; texture; texture = nexttexture) { + nexttexture = texture->next; + SDL_DestroyTexture(texture); } + renderer->textures = NULL; if (renderer->window) { SDL_SetWindowData(renderer->window, SDL_WINDOWRENDERDATA, NULL); diff --git a/src/render/SDL_yuv_sw.c b/src/render/SDL_yuv_sw.c index 7fc6b88654c31..044c8da3a4768 100644 --- a/src/render/SDL_yuv_sw.c +++ b/src/render/SDL_yuv_sw.c @@ -875,14 +875,16 @@ number_of_bits_set(Uint32 a) * Low performance, do not call often. */ static int +free_bits_at_bottom_nonzero(Uint32 a) +{ + SDL_assert(a != 0); + return (((Sint32) a) & 1l) ? 0 : 1 + free_bits_at_bottom_nonzero(a >> 1); +} + +static SDL_INLINE int free_bits_at_bottom(Uint32 a) { - /* assume char is 8 bits */ - if (!a) - return sizeof(Uint32) * 8; - if (((Sint32) a) & 1l) - return 0; - return 1 + free_bits_at_bottom(a >> 1); + return a ? free_bits_at_bottom_nonzero(a) : 32; } static int @@ -894,6 +896,7 @@ SDL_SW_SetupYUVDisplay(SDL_SW_YUVTexture * swdata, Uint32 target_format) int i; int bpp; Uint32 Rmask, Gmask, Bmask, Amask; + int freebits; if (!SDL_PixelFormatEnumToMasks (target_format, &bpp, &Rmask, &Gmask, &Bmask, &Amask) || bpp < 15) { @@ -910,13 +913,24 @@ SDL_SW_SetupYUVDisplay(SDL_SW_YUVTexture * swdata, Uint32 target_format) */ for (i = 0; i < 256; ++i) { r_2_pix_alloc[i + 256] = i >> (8 - number_of_bits_set(Rmask)); - r_2_pix_alloc[i + 256] <<= free_bits_at_bottom(Rmask); + freebits = free_bits_at_bottom(Rmask); + if (freebits < 32) { + r_2_pix_alloc[i + 256] <<= freebits; + } r_2_pix_alloc[i + 256] |= Amask; + g_2_pix_alloc[i + 256] = i >> (8 - number_of_bits_set(Gmask)); - g_2_pix_alloc[i + 256] <<= free_bits_at_bottom(Gmask); + freebits = free_bits_at_bottom(Gmask); + if (freebits < 32) { + g_2_pix_alloc[i + 256] <<= freebits; + } g_2_pix_alloc[i + 256] |= Amask; + b_2_pix_alloc[i + 256] = i >> (8 - number_of_bits_set(Bmask)); - b_2_pix_alloc[i + 256] <<= free_bits_at_bottom(Bmask); + freebits = free_bits_at_bottom(Bmask); + if (freebits < 32) { + b_2_pix_alloc[i + 256] <<= freebits; + } b_2_pix_alloc[i + 256] |= Amask; } diff --git a/src/render/opengl/SDL_render_gl.c b/src/render/opengl/SDL_render_gl.c index 1bf0a7288fe37..4dae7a5e40981 100644 --- a/src/render/opengl/SDL_render_gl.c +++ b/src/render/opengl/SDL_render_gl.c @@ -1430,18 +1430,21 @@ GL_RenderReadPixels(SDL_Renderer * renderer, const SDL_Rect * rect, GL_ActivateRenderer(renderer); + if (!convert_format(data, temp_format, &internalFormat, &format, &type)) { + return SDL_SetError("Texture format %s not supported by OpenGL", + SDL_GetPixelFormatName(temp_format)); + } + + if (!rect->w || !rect->h) { + return 0; /* nothing to do. */ + } + temp_pitch = rect->w * SDL_BYTESPERPIXEL(temp_format); temp_pixels = SDL_malloc(rect->h * temp_pitch); if (!temp_pixels) { return SDL_OutOfMemory(); } - if (!convert_format(data, temp_format, &internalFormat, &format, &type)) { - SDL_free(temp_pixels); - return SDL_SetError("Texture format %s not supported by OpenGL", - SDL_GetPixelFormatName(temp_format)); - } - SDL_GetRendererOutputSize(renderer, &w, &h); data->glPixelStorei(GL_PACK_ALIGNMENT, 1); diff --git a/src/video/SDL_RLEaccel.c b/src/video/SDL_RLEaccel.c index 6eb8ecb340911..792f19dcb048b 100644 --- a/src/video/SDL_RLEaccel.c +++ b/src/video/SDL_RLEaccel.c @@ -1277,7 +1277,7 @@ RLEColorkeySurface(SDL_Surface * surface) int y; Uint8 *srcbuf, *lastline; int maxsize = 0; - int bpp = surface->format->BytesPerPixel; + const int bpp = surface->format->BytesPerPixel; getpix_func getpix; Uint32 ckey, rgbmask; int w, h; @@ -1300,6 +1300,9 @@ RLEColorkeySurface(SDL_Surface * surface) maxsize = surface->h * (4 * (surface->w / 65535 + 1) + surface->w * 4) + 4; break; + + default: + return -1; } rlebuf = (Uint8 *) SDL_malloc(maxsize); @@ -1393,7 +1396,7 @@ RLEColorkeySurface(SDL_Surface * surface) surface->map->data = p; } - return (0); + return 0; } int diff --git a/src/video/SDL_rect.c b/src/video/SDL_rect.c index 2b29451c79bc4..8de15fc364e4c 100644 --- a/src/video/SDL_rect.c +++ b/src/video/SDL_rect.c @@ -22,7 +22,7 @@ #include "SDL_rect.h" #include "SDL_rect_c.h" - +#include "SDL_assert.h" SDL_bool SDL_HasIntersection(const SDL_Rect * A, const SDL_Rect * B) @@ -441,9 +441,15 @@ SDL_IntersectRectAndLine(const SDL_Rect * rect, int *X1, int *Y1, int *X2, y = recty2; x = x1 + ((x2 - x1) * (y - y1)) / (y2 - y1); } else if (outcode2 & CODE_LEFT) { + /* If this assertion ever fires, here's the static analysis that warned about it: + http://buildbot.libsdl.org/sdl-static-analysis/sdl-macosx-static-analysis/sdl-macosx-static-analysis-1101/report-b0d01a.html#EndPath */ + SDL_assert(x2 != x1); /* if equal: division by zero. */ x = rectx1; y = y1 + ((y2 - y1) * (x - x1)) / (x2 - x1); } else if (outcode2 & CODE_RIGHT) { + /* If this assertion ever fires, here's the static analysis that warned about it: + http://buildbot.libsdl.org/sdl-static-analysis/sdl-macosx-static-analysis/sdl-macosx-static-analysis-1101/report-39b114.html#EndPath */ + SDL_assert(x2 != x1); /* if equal: division by zero. */ x = rectx2; y = y1 + ((y2 - y1) * (x - x1)) / (x2 - x1); } diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index b8d362e043c43..ba724e28bd767 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -338,9 +338,14 @@ SDL_CreateWindowTexture(SDL_VideoDevice *unused, SDL_Window * window, Uint32 * f /* Create framebuffer data */ data->bytes_per_pixel = SDL_BYTESPERPIXEL(*format); data->pitch = (((window->w * data->bytes_per_pixel) + 3) & ~3); - data->pixels = SDL_malloc(window->h * data->pitch); - if (!data->pixels) { - return SDL_OutOfMemory(); + + { + /* Make static analysis happy about potential malloc(0) calls. */ + const size_t allocsize = window->h * data->pitch; + data->pixels = SDL_malloc((allocsize > 0) ? allocsize : 1); + if (!data->pixels) { + return SDL_OutOfMemory(); + } } *pixels = data->pixels;