diff --git a/.github/workflows/unit_test.yml b/.github/workflows/unit_test.yml
index a06d38d8259..37f48023334 100644
--- a/.github/workflows/unit_test.yml
+++ b/.github/workflows/unit_test.yml
@@ -24,6 +24,11 @@ jobs:
test:
runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ sanitize: ['no', 'yes']
+
container: ghcr.io/qmk/qmk_cli
steps:
@@ -35,4 +40,4 @@ jobs:
run: pip3 install -r requirements-dev.txt
- name: Run tests
- run: qmk test-c -j $(nproc)
+ run: qmk test-c -j $(nproc) -e SANITIZE=${{ matrix.sanitize }}
diff --git a/builddefs/build_test.mk b/builddefs/build_test.mk
index a9c9e5be2ee..8ed5092fd53 100644
--- a/builddefs/build_test.mk
+++ b/builddefs/build_test.mk
@@ -36,6 +36,15 @@ $(GTEST_OUTPUT)_INC := $(GTEST_INC) $(GTEST_INTERNAL_INC)
LDFLAGS += -lstdc++ -lpthread -shared-libgcc
CREATE_MAP := no
+# Build and run the tests under AddressSanitizer and UndefinedBehaviorSanitizer with `make test:all SANITIZE=yes`.
+SANITIZE ?= no
+ifeq ($(strip $(SANITIZE)),yes)
+SANITIZE_FLAGS := -fsanitize=address,undefined -fno-sanitize-recover=all -fno-omit-frame-pointer
+CFLAGS += $(SANITIZE_FLAGS)
+CXXFLAGS += $(SANITIZE_FLAGS)
+LDFLAGS += $(SANITIZE_FLAGS)
+endif
+
VPATH += \
$(LIB_PATH)/googletest \
$(LIB_PATH)/googlemock \
diff --git a/docs/unit_testing.md b/docs/unit_testing.md
index aec4ec8334d..49643c9d226 100644
--- a/docs/unit_testing.md
+++ b/docs/unit_testing.md
@@ -52,6 +52,14 @@ make test:all DEBUG=1
Alternatively, add `CONSOLE_ENABLE=yes` to the tests `rules.mk`.
+The tests can also be built and run under AddressSanitizer and UndefinedBehaviorSanitizer, which abort on the first out-of-bounds access, use-after-free, or undefined operation:
+
+```
+make test:all SANITIZE=yes
+```
+
+The emulated EEPROM used by the tests aborts in the same way on any access outside its configured size.
+
## Full Integration Tests
It's not yet possible to do a full integration test, where you would compile the whole firmware and define a keymap that you are going to test. However there are plans for doing that, because writing tests that way would probably be easier, at least for people that are not used to unit testing.
diff --git a/drivers/encoder/encoder_quadrature.c b/drivers/encoder/encoder_quadrature.c
index b3aeb136b2f..53d44b8741a 100644
--- a/drivers/encoder/encoder_quadrature.c
+++ b/drivers/encoder/encoder_quadrature.c
@@ -9,6 +9,7 @@
#include "action.h"
#include "keycodes.h"
#include "wait.h"
+#include "util.h"
#ifdef SPLIT_KEYBOARD
# include "split_util.h"
@@ -123,7 +124,7 @@ void encoder_driver_init(void) {
memset(encoder_pulses, 0, sizeof(encoder_pulses));
const pin_t encoders_pad_a_left[] = ENCODER_A_PINS;
const pin_t encoders_pad_b_left[] = ENCODER_B_PINS;
- for (uint8_t i = 0; i < thisCount; i++) {
+ for (uint8_t i = 0; i < thisCount && i < ARRAY_SIZE(encoders_pad_a_left); i++) {
encoders_pad_a[i] = encoders_pad_a_left[i];
encoders_pad_b[i] = encoders_pad_b_left[i];
}
diff --git a/platforms/test/eeprom.c b/platforms/test/eeprom.c
index 193a3bb4cee..c060896fc83 100644
--- a/platforms/test/eeprom.c
+++ b/platforms/test/eeprom.c
@@ -14,18 +14,28 @@
* along with this program. If not, see .
*/
+#include
+#include
#include "eeprom.h"
static uint8_t buffer[TOTAL_EEPROM_BYTE_COUNT];
-uint8_t eeprom_read_byte(const uint8_t *addr) {
+// Abort on any access outside the emulated EEPROM rather than silently reading or corrupting whatever follows the backing array.
+static uintptr_t checked_offset(const void *addr) {
uintptr_t offset = (uintptr_t)addr;
- return buffer[offset];
+ if (offset >= TOTAL_EEPROM_BYTE_COUNT) {
+ fprintf(stderr, "eeprom: address %#zx is outside the emulated EEPROM (%zu bytes)\n", (size_t)offset, (size_t)TOTAL_EEPROM_BYTE_COUNT);
+ abort();
+ }
+ return offset;
+}
+
+uint8_t eeprom_read_byte(const uint8_t *addr) {
+ return buffer[checked_offset(addr)];
}
void eeprom_write_byte(uint8_t *addr, uint8_t value) {
- uintptr_t offset = (uintptr_t)addr;
- buffer[offset] = value;
+ buffer[checked_offset(addr)] = value;
}
uint16_t eeprom_read_word(const uint16_t *addr) {
diff --git a/platforms/test/eeprom_legacy_emulated_flash_tests.cpp b/platforms/test/eeprom_legacy_emulated_flash_tests.cpp
index 41b27b621df..b5b7a9abb58 100644
--- a/platforms/test/eeprom_legacy_emulated_flash_tests.cpp
+++ b/platforms/test/eeprom_legacy_emulated_flash_tests.cpp
@@ -227,7 +227,8 @@ TEST_F(EepromStm32Test, TestWriteWord) {
/* Check values */
/* Direct compacted-area */
EXPECT_EQ(*(uint16_t*)&FlashBuf[EEPROM_BASE], (uint16_t)~0xdead);
- EXPECT_EQ(*(uint16_t*)&FlashBuf[EEPROM_BASE + 3], (uint16_t)~0xbeef);
+ EXPECT_EQ(FlashBuf[EEPROM_BASE + 3], (uint8_t)~0xef);
+ EXPECT_EQ(FlashBuf[EEPROM_BASE + 4], (uint8_t)~0xbe);
EXPECT_EQ(*(uint16_t*)&FlashBuf[EEPROM_BASE + 200], (uint16_t)~0xabcd);
EXPECT_EQ(FlashBuf[EEPROM_BASE + 203], (uint8_t)~0x76);
EXPECT_EQ(FlashBuf[EEPROM_BASE + 204], (uint8_t)~0x98);
diff --git a/quantum/sequencer/sequencer.c b/quantum/sequencer/sequencer.c
index ff243c870bb..9956f860e8b 100644
--- a/quantum/sequencer/sequencer.c
+++ b/quantum/sequencer/sequencer.c
@@ -70,10 +70,16 @@ void sequencer_set_track_notes(const uint16_t track_notes[SEQUENCER_TRACKS]) {
}
bool is_sequencer_track_active(uint8_t track) {
+ if (track >= SEQUENCER_TRACKS) {
+ return false;
+ }
return (sequencer_internal_state.active_tracks >> track) & true;
}
void sequencer_set_track_activation(uint8_t track, bool value) {
+ if (track >= SEQUENCER_TRACKS) {
+ return;
+ }
if (value) {
sequencer_internal_state.active_tracks |= (1 << track);
} else {
@@ -87,6 +93,9 @@ void sequencer_toggle_track_activation(uint8_t track) {
}
void sequencer_toggle_single_active_track(uint8_t track) {
+ if (track >= SEQUENCER_TRACKS) {
+ return;
+ }
if (is_sequencer_track_active(track)) {
sequencer_internal_state.active_tracks = 0;
} else {