From cea35a6fe70cc47d6d282393e18b14fc2677f345 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Tue, 16 Jan 2024 13:46:08 -0600 Subject: [PATCH] common: fix UB in some bit shifts Fix a few instances of undefined behavior (that I'm not yet aware of any concrete consequences of) in left shift expressions. Roughly, it's undefined behavior to shift a `1` into the sign bit of a signed type. Operands in bit shift expressions undergo integer promotions, but not the usual arithmetic conversions. Notably, the second operand can't promote the first operand to unsigned. Shifting `1` rather than `1U` can lead to shifting a `1` into the sign bit, causing undefined behavior. Relatedly, the integer promotions on a 32-bit platform will usually promote `uint16_t` to a signed `int`, leading to undefined behavior in left shift expressions. --- src/platforms/common/stm32/gpio.h | 4 ++-- src/platforms/common/swdptap.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/platforms/common/stm32/gpio.h b/src/platforms/common/stm32/gpio.h index cd0f9707b40..0974d2fb058 100644 --- a/src/platforms/common/stm32/gpio.h +++ b/src/platforms/common/stm32/gpio.h @@ -46,10 +46,10 @@ static inline void bmp_gpio_clear(const uint32_t gpioport, const uint16_t gpios) #else #if defined(STM32F4) || defined(STM32F7) /* NOLINTNEXTLINE(clang-diagnostic-int-to-pointer-cast) */ - GPIO_BSRR(gpioport) = gpios << 16U; + GPIO_BSRR(gpioport) = (uint32_t)gpios << 16U; #endif /* NOLINTNEXTLINE(clang-diagnostic-int-to-pointer-cast) */ - GPIO_BSRR(gpioport) = gpios << 16U; + GPIO_BSRR(gpioport) = (uint32_t)gpios << 16U; #endif } diff --git a/src/platforms/common/swdptap.c b/src/platforms/common/swdptap.c index 6742ba9fbe8..20366557ee2 100644 --- a/src/platforms/common/swdptap.c +++ b/src/platforms/common/swdptap.c @@ -151,7 +151,7 @@ static void swdptap_seq_out_clk_delay(const uint32_t tms_states, const size_t cl { for (size_t cycle = 0; cycle < clock_cycles; ++cycle) { gpio_clear(SWCLK_PORT, SWCLK_PIN); - gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1 << cycle)); + gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1U << cycle)); for (volatile uint32_t counter = target_clk_divider; counter > 0; --counter) continue; gpio_set(SWCLK_PORT, SWCLK_PIN); @@ -167,7 +167,7 @@ static void swdptap_seq_out_no_delay(const uint32_t tms_states, const size_t clo { for (size_t cycle = 0; cycle < clock_cycles; ++cycle) { gpio_clear(SWCLK_PORT, SWCLK_PIN); - gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1 << cycle)); + gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1U << cycle)); gpio_set(SWCLK_PORT, SWCLK_PIN); } gpio_clear(SWCLK_PORT, SWCLK_PIN);