From d2361ae4f9029a6b2a3093788f9786d70fcca2a0 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sun, 1 Jan 2023 16:39:52 -0600 Subject: [PATCH 1/4] Avoid an undefined shift (1 << 32), an operation on a signed 32-bit int, is undefined in C. The operation on the unsigned int (1u<<32) is defined as zero, which is the desired outcome (subtracting 1 yields the value with all bits set) This problem was detected by clang scan-build static analysis --- shared-module/displayio/Bitmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c index 676aec2186..ab1b85ea16 100644 --- a/shared-module/displayio/Bitmap.c +++ b/shared-module/displayio/Bitmap.c @@ -70,8 +70,8 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, self->x_shift++; power_of_two <<= 1; } - self->x_mask = (1 << self->x_shift) - 1; // Used as a modulus on the x value - self->bitmask = (1 << bits_per_value) - 1; + self->x_mask = (1u << self->x_shift) - 1u; // Used as a modulus on the x value + self->bitmask = (1u << bits_per_value) - 1u; self->dirty_area.x1 = 0; self->dirty_area.x2 = width; From d8081857444496b8849a8a910ef0fda771f1cfce Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sun, 1 Jan 2023 16:40:06 -0600 Subject: [PATCH 2/4] Emphasize that ALIGN_BITS is a constant --- shared-module/displayio/Bitmap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c index ab1b85ea16..7a1b3e6b17 100644 --- a/shared-module/displayio/Bitmap.c +++ b/shared-module/displayio/Bitmap.c @@ -30,12 +30,12 @@ #include "py/runtime.h" -enum { align_bits = 8 * sizeof(uint32_t) }; +enum { ALIGN_BITS = 8 * sizeof(uint32_t) }; static int stride(uint32_t width, uint32_t bits_per_value) { uint32_t row_width = width * bits_per_value; // align to uint32_t - return (row_width + align_bits - 1) / align_bits; + return (row_width + ALIGN_BITS - 1) / ALIGN_BITS; } void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t width, @@ -66,7 +66,7 @@ void common_hal_displayio_bitmap_construct_from_buffer(displayio_bitmap_t *self, self->x_shift = 0; // Used to divide the index by the number of pixels per word. Its used in a // shift which effectively divides by 2 ** x_shift. uint32_t power_of_two = 1; - while (power_of_two < align_bits / bits_per_value) { + while (power_of_two < ALIGN_BITS / bits_per_value) { self->x_shift++; power_of_two <<= 1; } From 34043c2d38c9e9c52fbcee8fe487099098eab927 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sun, 1 Jan 2023 16:44:12 -0600 Subject: [PATCH 3/4] Only store up to 'width' pixels, not 'stride' error detected by clang scan-build static analysis --- shared-module/bitmaptools/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-module/bitmaptools/__init__.c b/shared-module/bitmaptools/__init__.c index 4c73d8fb8e..67b5254b32 100644 --- a/shared-module/bitmaptools/__init__.c +++ b/shared-module/bitmaptools/__init__.c @@ -689,7 +689,7 @@ STATIC void fill_row(displayio_bitmap_t *bitmap, int swap, int16_t *luminance_da static void write_pixels(displayio_bitmap_t *bitmap, int y, bool *data) { if (bitmap->bits_per_value == 1) { uint32_t *pixel_data = (uint32_t *)(bitmap->data + bitmap->stride * y); - for (int i = 0; i < bitmap->stride; i++) { + for (int i = 0; i < bitmap->width; i++) { uint32_t p = 0; for (int j = 0; j < 32; j++) { p = (p << 1); From ef8b297d7f6e04818370de2fe3409823f45b9d02 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sun, 1 Jan 2023 16:56:46 -0600 Subject: [PATCH 4/4] Avoid null pointer dereference when no kwargs clang scan-build reports "Access to field 'table' results in a dereference of a null pointer (loaded from variable 'kw_args')" --- py/objtype.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/py/objtype.c b/py/objtype.c index 0914ad5f2e..76b9551be8 100644 --- a/py/objtype.c +++ b/py/objtype.c @@ -106,7 +106,9 @@ STATIC mp_obj_t native_base_init_wrapper(size_t n_args, const mp_obj_t *pos_args // copy in args memcpy(args2, pos_args, n_args * sizeof(mp_obj_t)); // copy in kwargs - memcpy(args2 + n_args, kw_args->table, 2 * n_kw * sizeof(mp_obj_t)); + if (n_kw) { + memcpy(args2 + n_args, kw_args->table, 2 * n_kw * sizeof(mp_obj_t)); + } self->subobj[0] = native_base->make_new(native_base, n_args, n_kw, args2); m_del(mp_obj_t, args2, n_args + 2 * n_kw);