Merge pull request #324 from bloqaudio/pr/host-test-harness

test: add host-side unit-test harness, covering cmd_editor

.github/workflows/build.yml

@@ -17,6 +17,8 @@ jobs:
         run: |
           apt update
           apt install make gcc sdcc xxd python-is-python3 libjson-c-dev zlib1g-dev -y
+      - name: Run the host-side unit tests
+        run: make -C test
       - name: Check if machine.c can be compiled for all machines
         run: make machine_check
       - name: Make project

machine.h

@@ -74,7 +74,7 @@ struct sfp_port
 	i2c_bus_t i2c;
 };
 
-typedef struct machine {
+struct machine {
 	char machine_name[30];
 	uint8_t isRTL8373;
 	// Lowest logical port number
@@ -102,7 +102,7 @@ typedef struct machine {
 	uint32_t mac_flash_offset;
 };
 
-typedef struct machine_runtime
+struct machine_runtime
 {
 	uint8_t isRTL8373 : 1;
 	uint8_t isN : 1;

rtl837x_common.h

@@ -158,7 +158,6 @@ void reg_write_m(uint16_t reg_addr);
 void sds_read(uint8_t sds_id, uint8_t page, uint8_t reg);
 void sds_write_v(uint8_t sds_id, uint8_t page, uint8_t reg, uint16_t v);
 void delay(uint16_t t);
-void sleep(uint16_t t);
 void write_char_no_syslog(char c);
 void write_char(char c);
 void print_reg(uint16_t reg);
@@ -170,15 +169,22 @@ uint8_t reg_bit_test(uint16_t reg_addr, char bit);
 void sfr_mask_data(uint8_t n, uint8_t mask, uint8_t set);
 void sfr_set_zero(void);
 void reset_chip(void);
+/* Firmware implementations that shadow libc names. Host unit-test builds
+ * (RTLP_HOST_TEST) hide these prototypes so they don't clash with glibc;
+ * argument order matches libc, so on-host callers transparently use the
+ * C library. See test/. */
+#ifndef RTLP_HOST_TEST
+void sleep(uint16_t t);
 void memcpy(__xdata void * __xdata dst, __xdata const void * __xdata src, uint16_t len);
-void memcpyc(__xdata uint8_t *dst, __code uint8_t *src, uint16_t len);
 void memset(__xdata uint8_t *dst, __xdata uint8_t v, uint8_t len);
 int memcmp(__xdata const void *a, __xdata const void *b, uint16_t len);
 uint16_t strlen(__code const char *s);
-uint16_t strlen_x(__xdata const char *s);
-uint16_t strtox(__xdata uint8_t *dst, __code const char *s);
 uint16_t strcpy(__xdata uint8_t *dst, const char *s);
 char strcmp(__xdata const uint8_t *a, __code const uint8_t *b);
+#endif
+void memcpyc(__xdata uint8_t *dst, __code uint8_t *src, uint16_t len);
+uint16_t strlen_x(__xdata const char *s);
+uint16_t strtox(__xdata uint8_t *dst, __code const char *s);
 bool strstart(__xdata const uint8_t *a, __code const uint8_t *b);
 bool strstart_x(__xdata const uint8_t *a, __xdata const uint8_t *b);
 void tcpip_output(void);

test/Makefile

@@ -0,0 +1,48 @@
+# Host unit-test harness for RTLPlayground firmware logic.
+#
+# Compiles selected firmware translation units with host gcc + AddressSanitizer,
+# force-including sdcc_shim.h to erase 8051 keywords, and links them against
+# mocked hardware edges (support.c). No SDCC, no flashing, no hardware.
+#
+#   make        build + run all tests
+#   make run    run tests
+#   make clean
+
+CC       = gcc
+SRCROOT  = ..
+# -include shim first so __xdata/__code/__banked vanish before any firmware header.
+CFLAGS   = -std=gnu11 -g -O1 -Wall -Wextra -Wno-unused-parameter -Wno-unknown-pragmas \
+           -DRTLP_HOST_TEST \
+           -fsanitize=address,undefined -fno-omit-frame-pointer \
+           -include sdcc_shim.h -I. -I$(SRCROOT) -I$(SRCROOT)/httpd -I$(SRCROOT)/uip
+ifeq ($(findstring clang,$(shell $(CC) --version 2>/dev/null)),)
+CFLAGS  += -Wno-builtin-declaration-mismatch
+endif
+LDFLAGS  = -fsanitize=address,undefined
+
+BUILDDIR = build
+
+# One binary per subsystem under test.
+TESTS = $(BUILDDIR)/test_cmd_editor
+
+all: run
+
+$(BUILDDIR):
+	mkdir -p $(BUILDDIR)
+
+# cmd_editor.c is compiled UNMODIFIED from the firmware tree.
+$(BUILDDIR)/test_cmd_editor: test_cmd_editor.c support.c $(SRCROOT)/cmd_editor.c | $(BUILDDIR)
+	$(CC) $(CFLAGS) $^ -o $@ $(LDFLAGS)
+
+run: $(TESTS)
+	@fail=0; for t in $(TESTS); do \
+		echo "=== $$t ==="; \
+		$$t || fail=1; \
+		echo; \
+	done; \
+	if [ $$fail -ne 0 ]; then echo "TESTS FAILED"; exit 1; else echo "ALL TESTS PASSED"; fi
+
+clean:
+	rm -rf $(BUILDDIR)
+
+.PHONY: all run clean

test/README.md

@@ -0,0 +1,49 @@
+# Host unit-test harness
+
+Compile and test individual firmware translation units on the build host with
+**gcc + AddressSanitizer + UBSan** — no SDCC, no flashing, no hardware. This is
+the fast inner loop for the logic-level bugs in `../../NOTES/08-findings.md`
+(parser, line editor, HTTP header, DHCP): edit → `make` → see red/green in
+seconds instead of a minutes-long flash-and-reboot cycle.
+
+## Run
+```
+cd test
+make            # build + run all tests (ASan/UBSan on)
+make clean
+```
+Exit status is non-zero if any check fails, a sanitizer trips, or the watchdog
+fires — so it drops straight into CI.
+
+## How it works
+- **`sdcc_shim.h`** (force-included) erases SDCC 8051 keywords (`__xdata`,
+  `__code`, `__banked`, …) so firmware sources compile under host gcc. It changes
+  no logic — the code under test is byte-for-byte the firmware source.
+- **`support.c` / `support.h`** mock the hardware edges: the 16-byte serial ring
+  (`sbuf`), the command/history buffers, and the character-output sink
+  (`write_char` etc.). Buffers are sized **exactly** as on target, so ASan
+  redzones catch the same off-by-one overflows the 8051 hits.
+- Builds define **`RTLP_HOST_TEST`**, which hides the firmware's libc-named
+  prototypes (`memset`/`strlen`/…) in `rtl837x_common.h` so they don't clash with
+  glibc. Argument order matches libc, so on-host callers transparently use the C
+  library. This guard is compiled out of normal firmware builds — zero on-target
+  effect.
+- A **SIGALRM watchdog** (`watchdog_arm`) turns an infinite-loop bug into a fast
+  test failure instead of a hung runner.
+
+## Current coverage
+| Test binary | TU under test | Findings exercised |
+|-------------|---------------|--------------------|
+| `test_cmd_editor` | `cmd_editor.c` | **C4** — full-line hang + `cmd_buffer` 1-byte overflow; basic entry & backspace regressions |
+
+## Adding a test for another module
+1. Write `test_<module>.c` with `main()` driving the module's entry points and
+   `CHECK(cond, msg)` assertions.
+2. Add any missing mocked symbols to `support.c` (only what the linker asks for).
+3. Add a target in the `Makefile` listing `test_<module>.c support.c ../<module>.c`.
+4. If the module calls a firmware libc-name (`memcpy`/`strcpy`/…), it will use the
+   C library on host (compatible arg order); provide a `fw_*`-style mock only if
+   the semantics differ in a way the test cares about.
+
+Good next targets (pure logic, high finding density): `cmd_parser.c` (C6/C8),
+the HTTP header parser in `httpd/httpd.c` (S1), `dhcp.c` (S5).

test/sdcc_shim.h

@@ -0,0 +1,40 @@
+/*
+ * sdcc_shim.h — neutralize SDCC 8051 keywords so firmware translation units
+ * compile under host gcc/clang for off-target unit testing.
+ *
+ * Force-included (gcc -include) ahead of every firmware source under test.
+ * It only erases storage/qualifier keywords and the banked-call attribute;
+ * it does NOT change any logic, so the code under test is byte-for-byte the
+ * firmware source. Hardware/register access is provided by mocks (support.c),
+ * never by this file.
+ */
+#ifndef SDCC_SHIM_H
+#define SDCC_SHIM_H
+
+#include <stdbool.h>
+
+/* Memory-space qualifiers → nothing (host has one flat address space). */
+#define __xdata
+#define __code
+#define __pdata
+#define __idata
+#define __data
+/* bool, not unsigned char: SDCC's bit types normalize any assigned value
+ * to 0/1, which _Bool reproduces exactly. */
+#define __bit         bool
+#define __sbit        bool
+
+/* NOTE: the firmware ships its own 8051 memcpy/memset/strlen/strcpy/sleep with
+ * incompatible signatures (rtl837x_common.h). Host TUs must therefore build with
+ * -fno-builtin (so gcc doesn't auto-declare the builtins) and must NOT include
+ * <string.h>/<unistd.h>. Test code uses the t_* helpers in support.h instead. */
+
+/* Function attributes → nothing (no banking / no 8051 calling conventions). */
+#define __banked
+#define __reentrant
+#define __naked
+#define __using(n)
+#define __interrupt(n)
+#define __at(addr)
+
+#endif /* SDCC_SHIM_H */

test/support.c

@@ -0,0 +1,103 @@
+/*
+ * support.c — mock hardware edges + test environment for the host harness.
+ *
+ * Everything the firmware expects the platform/other-TUs to provide, but which
+ * has no business touching real hardware in a unit test, lives here: the serial
+ * ring, the command/history buffers, the character-output sink, and stubs for
+ * the print helpers. The buffers are sized EXACTLY as on target so that
+ * AddressSanitizer redzones catch the same off-by-one overflows the 8051 hits.
+ */
+#include "sdcc_shim.h"
+#include <stdint.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <signal.h>
+/* Declared by hand instead of <unistd.h>, whose sleep() prototype clashes with
+ * the firmware's own sleep() declaration in rtl837x_common.h. */
+extern unsigned int alarm(unsigned int);
+extern void _exit(int);
+#include "rtl837x_common.h"     /* SBUF_SIZE, CMD_BUF_SIZE, CMD_HISTORY_SIZE, decls */
+#include "support.h"
+
+/* --- symbols the editor/parser reference (extern in the firmware headers) --- */
+__xdata volatile uint8_t sbuf_ptr;
+__xdata uint8_t          sbuf[SBUF_SIZE];
+uint8_t                  cmd_buffer[CMD_BUF_SIZE];   /* ASan-instrumented: OOB caught */
+uint8_t                  cmd_available;
+uint8_t                  err_status;
+uint8_t                  cmd_history[CMD_HISTORY_SIZE];
+uint16_t                 cmd_history_ptr;
+
+/* editor-internal read pointer (defined in cmd_editor.c, external linkage) */
+extern __xdata uint8_t l;
+
+/* the editor entry points under test */
+void cmd_editor_init(void);
+void cmd_edit(void);
+
+/* ------------------------------- output sink ------------------------------- */
+char   out_buf[8192];
+size_t out_len;
+
+void out_reset(void) { out_len = 0; out_buf[0] = 0; }
+
+static void out_putc(char c) {
+    if (out_len < sizeof(out_buf) - 1) { out_buf[out_len++] = c; out_buf[out_len] = 0; }
+}
+
+/* --- mocked print helpers (signatures match rtl837x_common.h post-shim) ---- */
+void write_char(char c)            { out_putc(c); }
+void print_string(char *p)         { while (*p) out_putc(*p++); }
+void print_string_x(char *p)       { while (*p) out_putc(*p++); }
+void print_cmd_prompt(void)        { print_string((char *)"> "); }
+void itoa(uint8_t v) {                       /* firmware prints a decimal number */
+    char t[4]; int n = 0;
+    if (!v) { out_putc('0'); return; }
+    while (v) { t[n++] = '0' + v % 10; v /= 10; }
+    while (n) out_putc(t[--n]);
+}
+
+/* ------------------------------ environment -------------------------------- */
+void env_reset(void) {
+    sbuf_ptr = 0;
+    t_memset(sbuf, 0, sizeof(sbuf));
+    t_memset(cmd_buffer, 0, sizeof(cmd_buffer));
+    t_memset(cmd_history, 0, sizeof(cmd_history));
+    cmd_history_ptr = 0;
+    cmd_available = 0;
+    err_status = 0;
+    out_reset();
+    cmd_editor_init();          /* sets l = sbuf_ptr, clears cursor/len */
+}
+
+/* Push one byte into the ring, draining first if it is full (leave 1 slot). */
+static void ring_push(uint8_t b) {
+    while (((sbuf_ptr + 1) & SBUF_MASK) == l)
+        cmd_edit();                          /* drain; under the C4 bug this spins */
+    sbuf[sbuf_ptr] = b;
+    sbuf_ptr = (sbuf_ptr + 1) & SBUF_MASK;
+}
+
+void feed_n(const char *s, size_t n) {
+    for (size_t i = 0; i < n; i++) ring_push((uint8_t)s[i]);
+    cmd_edit();
+}
+void feed(const char *s) { feed_n(s, t_strlen(s)); }
+
+/* -------------------------------- watchdog --------------------------------- */
+static const char *wd_what;
+static void wd_fire(int sig) {
+    (void)sig;
+    /* async-signal-unsafe write is acceptable: we abort immediately after. */
+    fprintf(stderr, "\n  HANG DETECTED (watchdog): %s\n", wd_what ? wd_what : "?");
+    _exit(2);
+}
+void watchdog_arm(unsigned seconds, const char *what) {
+    wd_what = what;
+    signal(SIGALRM, wd_fire);
+    alarm(seconds);
+}
+void watchdog_disarm(void) { alarm(0); }
+
+/* -------------------------------- framework -------------------------------- */
+int tests_run, tests_failed;

test/support.h

@@ -0,0 +1,53 @@
+/*
+ * support.h — test harness API for off-target firmware unit tests.
+ *
+ * Provides the mock environment (serial ring, command buffers, output capture)
+ * plus a tiny assertion framework. The firmware sources under test are compiled
+ * unmodified against this; only the hardware edges are mocked here.
+ */
+#ifndef TEST_SUPPORT_H
+#define TEST_SUPPORT_H
+
+#include <stdint.h>
+#include <stddef.h>
+
+/* Distinct-named string helpers so host code needn't include <string.h>, which
+ * would clash with the firmware's own memset/strlen/strcpy prototypes in
+ * rtl837x_common.h (different 8051 signatures). */
+static inline void  t_memset(void *d, int c, size_t n) {
+    unsigned char *p = d; while (n--) *p++ = (unsigned char)c;
+}
+static inline size_t t_strlen(const char *s) { const char *p = s; while (*p) p++; return (size_t)(p - s); }
+static inline int    t_strcmp(const char *a, const char *b) {
+    while (*a && *a == *b) { a++; b++; } return (unsigned char)*a - (unsigned char)*b;
+}
+
+/* ---- captured terminal output (what the firmware would send over serial) ---- */
+extern char     out_buf[8192];
+extern size_t   out_len;
+void            out_reset(void);
+
+/* ---- editor/parser environment reset ---- */
+void env_reset(void);           /* clears ring, buffers, editor state */
+
+/* ---- drive the serial input ring exactly like the RX ISR would ----
+ * Pushes bytes into the 16-byte sbuf ring, draining via cmd_edit() whenever the
+ * ring would overflow, then a final drain. Mirrors real ISR→editor handoff.
+ */
+void feed(const char *s);                 /* NUL-terminated */
+void feed_n(const char *s, size_t n);     /* explicit length (for embedded NULs) */
+
+/* ---- watchdog: arm before calling into code that could spin (C4 hang) ---- */
+void watchdog_arm(unsigned seconds, const char *what);
+void watchdog_disarm(void);
+
+/* ---- minimal test framework ---- */
+extern int tests_run, tests_failed;
+#define CHECK(cond, msg) do {                                            \
+        tests_run++;                                                     \
+        if (!(cond)) { tests_failed++;                                   \
+            printf("  FAIL: %s  (%s:%d)\n", (msg), __FILE__, __LINE__);  \
+        } else { printf("  ok:   %s\n", (msg)); }                        \
+    } while (0)
+
+#endif /* TEST_SUPPORT_H */

test/test_cmd_editor.c

@@ -0,0 +1,76 @@
+/*
+ * test_cmd_editor.c — host unit tests for cmd_editor.c (the serial line editor).
+ *
+ * Focus: finding C4 (08-findings.md) — a full input line both HANGS the editor
+ * (continue without advancing the ring read pointer) and OVERFLOWS cmd_buffer by
+ * one byte (guard off-by-one lets cmd_line_len reach CMD_BUF_SIZE).
+ *
+ * These tests are RED against the current source and GREEN after the fix, with
+ * no hardware and no flash cycle. The hang is caught by a SIGALRM watchdog; the
+ * overflow is caught by AddressSanitizer redzones on cmd_buffer[].
+ */
+#include "sdcc_shim.h"
+#include <stdint.h>
+#include <string.h>
+#include <stdio.h>
+#include "rtl837x_common.h"     /* CMD_BUF_SIZE */
+#include "support.h"
+
+/* editor state (globals in cmd_editor.c, external linkage) */
+extern __xdata uint8_t cmd_line_len;
+extern uint8_t cmd_buffer[];
+extern uint8_t cmd_available;
+
+static void test_basic_line(void) {
+    printf("[test] basic line entry + Enter\n");
+    env_reset();
+    feed("vlan 199\n");
+    CHECK(cmd_available == 1, "Enter marks a command available");
+    CHECK(t_strcmp((char *)cmd_buffer, "vlan 199") == 0, "cmd_buffer holds the typed line");
+}
+
+static void test_backspace(void) {
+    printf("[test] backspace edits the line\n");
+    env_reset();
+    feed("helo");
+    feed("\b");            /* delete the 'o' -> wait, cursor at end: removes last char */
+    feed("lo\n");
+    CHECK(t_strcmp((char *)cmd_buffer, "hello") == 0, "backspace then retype yields 'hello'");
+}
+
+/* C4 part 1: a line that fills the buffer must not spin forever. */
+static void test_full_line_no_hang(void) {
+    char big[200];
+    printf("[test] C4: overlong line does not hang the editor\n");
+    env_reset();
+    t_memset(big, 'a', sizeof(big));
+    watchdog_arm(5, "C4 full-line hang (editor spun without advancing ring ptr)");
+    feed_n(big, sizeof(big));      /* 200 printable chars, no newline */
+    watchdog_disarm();
+    /* Reaching here means no hang. With the fix the line caps one short of full. */
+    CHECK(cmd_line_len == CMD_BUF_SIZE - 1,
+          "line length caps at CMD_BUF_SIZE-1 (room reserved for NUL)");
+}
+
+/* C4 part 2: filling the buffer then Enter must not write cmd_buffer[CMD_BUF_SIZE]. */
+static void test_full_line_no_overflow(void) {
+    char full[CMD_BUF_SIZE];       /* 128 printable chars, exactly buffer size */
+    printf("[test] C4: full line + Enter does not overflow cmd_buffer\n");
+    env_reset();
+    t_memset(full, 'b', sizeof(full));
+    watchdog_arm(5, "C4 overflow-path hang");
+    feed_n(full, sizeof(full));
+    feed("\n");                    /* under the bug, writes cmd_buffer[128] -> ASan trap */
+    watchdog_disarm();
+    CHECK(1, "128-char line + Enter completes without ASan overflow");
+}
+
+int main(void) {
+    printf("== cmd_editor host tests ==\n");
+    test_basic_line();
+    test_backspace();
+    test_full_line_no_hang();
+    test_full_line_no_overflow();
+    printf("\n%d checks, %d failed\n", tests_run, tests_failed);
+    return tests_failed ? 1 : 0;
+}