Relocate NUCLEO_F401RE to targets and adopt generic BSP framework - #50
Merged
fdesbiens merged 4 commits intoAug 31, 2026
Merged
Conversation
AmmarOkla12772
marked this pull request as ready for review
August 31, 2026 11:24
fdesbiens
force-pushed
the
feat/nucleo-bsp-migration
branch
from
August 31, 2026 15:04
d0b212d to
9d01d7d
Compare
…ric BSP framework
_sbrk() bounded the heap at BSP_RAM_END (0x20018000), the end of physical
SRAM. On this target that is not a guard at all. NUCLEO_F401RE.ld lays RAM
out as:
_end 0x20001170 heap base
_end + _Min_Heap_Size 0x20001370 intended heap ceiling
__RAM_segment_used_end__ 0x20001770 ThreadX byte pool starts here
byte pool end 0x20017000
_estack / BSP_RAM_END 0x20018000
so malloc() could grow the break through the reserved main stack, through
the entire ThreadX byte pool holding every thread stack and the queue
buffer, and into the live MSP, all while _sbrk() reported success. Only an
allocation past the top of SRAM returned ENOMEM.
The linker script now exports _heap_limit at the end of the heap
reservation and _sbrk() bounds against that, which keeps the heap inside
the 0x200 the script already set aside for it. Both bounds checks are also
computed in uintptr_t rather than by forming an out-of-bounds pointer
first, and the negative branch derives its magnitude without negating
PTRDIFF_MIN. BSP_RAM_END keeps documenting the memory map but now carries
a note against reusing it as a heap bound.
All newlib syscall overrides move into newlib_stubs.c, which the
application compiles directly. _write() and _read() previously sat in
bsp_console.c inside libbsp.a, where the linker extracted them only
because bsp_board.o happened to reference bsp_console_init(); breaking
that incidental chain would have silently handed printf to the libnosys
stubs, and CI only checks that the ELF exists. Console input stays board
specific behind nucleo_console.h, since the shared <bsp/console.h>
contract is write-only.
The BSP library is renamed nucleo_bsp, matching polarfire_bsp and freeing
the bare "bsp" target name.
The AI Disclosure header on newlib_stubs.c records the Codex-assisted
origin of the syscall shims moved in from console.c. That work was done by
Frederic Desbiens while editing PR eclipse-threadx#45, not by the PR author; the
squash-merge in 1bddd17 assigned git authorship to the contributor and its
Co-authored-by trailers are the accurate record.
bsp_console_init() dropped the return value of HAL_UART_Init(). The code it replaced routed the failure to Error_Handler(); the migration left the call bare, so a UART that failed to initialise booted on silently with no console. MISRA C:2012 Rule 17.7. The status is checked again and a failure now halts, matching the previous behaviour. The UART handle was a non-static global named UartHandle, but the extern declaration that used to live in board_init.h went away with that header, leaving an object with external linkage that nothing declared (Rule 8.4 and 8.7). It is now file-scope static console_uart. The redundant GPIOA and USART2 clock enables are gone as well: HAL_UART_MspInit() already owns that configuration, which is what "single UART owner" should mean. The const cast required by HAL_UART_Transmit() is documented as a Rule 11.8 deviation. bsp_board.c, bsp_led.c and bsp_console.c carry the AI Disclosure header required by AGENTS.md for new files. They derive from board_init.c, nucleo_bsp.c and console.c, each of which carried "Some portions generated by Codex (GPT-5)"; the rename dropped the disclosure while keeping most of the content, and git scores bsp_board.c as an 80% similarity rename. That Codex work was Frederic Desbiens', done while editing another contributor's PR under the AGENTS.md exception permitting edits to incoming PRs. It reached the tree through 1bddd17, a GitHub squash-merge of PR eclipse-threadx#45 whose git author is the PR author but whose trailers name the real participants: Co-authored-by: Ammar Okla <ammargawkla@gmail.com> Co-authored-by: Frederic Desbiens <frederic.desbiens@eclipse-foundation.org> Co-authored-by: Codex <codex@openai.com> The disclosure is therefore not a statement about this PR's author.
…tion The build-arm-nucleo job installed the distribution's gcc-arm-none-eabi, which is GCC 13.2 on ubuntu-24.04, so the compiler drifted with the runner image. AGENTS.md specifies GCC 14 and the sibling RISC-V job already pins xPack GCC 14.2.0 by URL; the ARM job now pins Arm GNU Toolchain 14.2.Rel1 the same way. The toolchain bundles its own newlib, so the separate newlib packages are no longer installed. CMAKE_C_STANDARD moves from 11 to 99 per the AGENTS.md C99 requirement. The target builds clean under both GCC 13.2.1 and the pinned GCC 14.2.1 with -Wall -Wshadow -Wdouble-promotion -Werror. README.md moved with git mv but still described the pre-migration layout: the build directory as STMicroelectronics/NUCLEO_F401RE, the HAL timebase overrides as living in board_init.c, and the BSP as lib/nucleo_bsp/. All three are corrected and the BSP entry now lists the four sources behind the generic interfaces. The Validation Record quoted 18244 B ROM and 5632 B RAM against a toolchain the CI pipeline does not use. Re-measured against the newly pinned GCC 14.2.1: 20120 B ROM and 6000 B RAM. The hardware verification checklist is left as the contributor recorded it.
fdesbiens
force-pushed
the
feat/nucleo-bsp-migration
branch
from
August 31, 2026 15:09
9d01d7d to
2743bce
Compare
fdesbiens
added a commit
that referenced
this pull request
Aug 31, 2026
The ARM target had no runtime test. Its CI job checked only that the ELF existed, so the clock configuration, the TIM2 HAL timebase, the console and the heap bound were all unexercised. This brings it to parity with the PolarFire target, which gates CI on a headless Renode run. Renode ships no NUCLEO-F401RE board description, so renode/nucleo_f401re.repl derives one from the generic STM32F4 CPU platform and corrects Flash to 512 KB and SRAM to 96 KB. The generic platform is sized for the larger F407/F429 parts, and both the linker script's heap reservation and the _sbrk() bound depend on those limits being right. main.c gains seven startup self-tests that run before tx_kernel_enter(), so a failure is reported even when the scheduler never starts. They cover the _sbrk() allocate, release, underflow and over-limit paths, the invariant that the heap reservation ends at or below the ThreadX byte pool, the 84 MHz SystemCoreClock, and that TIM2 still ticks after HAL_RCC_ClockConfig() re-enters HAL_InitTick(). Test 4 is the regression guard for the heap bound fixed in #50. Requesting 32 KB fits inside the 96 KB SRAM but far exceeds the heap reservation. Reinstating the old end-of-SRAM bound was verified to fail the suite: newlib's first malloc() then took roughly 4 KB and put the break at 0x20002170, inside the ThreadX byte pool, which also broke self-tests 1 and 3. With the bound correct, _sbrk() refuses the oversized request, newlib retries smaller, and malloc(64) succeeds using 72 bytes of the 512-byte reservation - so the reservation is adequate and no heap growth is needed. scripts/test_renode.py drives nucleo_f401re_ci.resc, which advances a fixed span of virtual time and quits on its own rather than depending on wall clock. Beyond the self-tests it asserts the boot banner, that the blink thread and the 1 Hz application timer have both run (covering the LED path and the timer service), and that the mutex, queue, event-flag and semaphore counters are all non-zero. The suite was confirmed to exit 1 on a reintroduced bug and 0 on the fixed tree. The Robot Framework suite covers the same ground for renode-test, and the new test-nucleo-renode CI job mirrors test-polarfire-renode. main.c also drops a hardcoded 0x20018000 in favour of BSP_RAM_END now that board_config.h is in scope, with the 4 KB main-stack margin named.
This was referenced Aug 31, 2026
fdesbiens
added a commit
that referenced
this pull request
Aug 31, 2026
…#56) The framework's stated purpose is that application code can be built for any board implementing the `bsp/` contracts. Neither demo could be, because both `main.c` files reached past those contracts for two things no portable application can see: the board's memory extents, and the hardware specifics its startup self-tests assert on. Neither was ever application logic. The self-tests test the BSP - linker reservations, clock trees, interrupt controllers - so they belong in the BSP. Two new contracts move both behind the boundary: * `bsp/selftest.h` - `bsp_self_test()` runs the board's checks and reports each through an application-supplied callback, so message formatting (and therefore the choice between `printf()` and `bsp_console_write()`) stays with the application while the checks stay with the board. * `bsp/memory.h` - `bsp_ram_region()` reports what RAM the application may claim, clamped against whatever the board reserves. Both targets implement both, and both `main.c` files now include nothing but the C standard headers, `tx_api.h` and `<bsp/...>`. The NUCLEO byte pool comes out byte-identical at 88204 bytes. Prototyping `bsp_ram_region()` on the PolarFire first surfaced a latent bug there. Its `_sbrk()` bounded the heap at the end of DRAM, which is the same mistake the NUCLEO shipped with before #50: harmless only because nothing else claimed that memory. The moment `bsp_ram_region()` promises it to an application, an oversized `malloc()` could take memory holding thread stacks. The heap is now bounded against a documented 64 KB reservation that `bsp_ram_region()` skips, and a new self-test guards the bound the way the NUCLEO's test 4 does. Measured heap use is under 256 bytes, so the reservation has ample headroom. The PolarFire tick-rate compile-time check moved to `hwtimer.c`, next to the `TICK_CYCLES` constant it guards, since it needed ThreadX headers that a portable `main.c` should not have to pull in. Verified locally on both targets: clean builds plus `test_renode.py` green on Cortex-M4 and RV64. Both suites were also confirmed to still have teeth - weakening each target's `_sbrk()` bound back to the end of RAM makes them exit 1 (three failures on the NUCLEO, one on the PolarFire), so no assertion was lost in the move. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(nucleo): Relocate NUCLEO_F401RE to targets and adopt generic BSP framework
Summary
Follow-up PR relocating the existing STMicroelectronics
NUCLEO_F401REtarget intotargets/and migrating it to the generic BSP framework introduced in #49.Changes
NUCLEO_F401REusinggit mv, with theSTM32CubeF4 vendor tree detected as
R100renames without any changes to the vendor files.nucleo_bspwithlib/bsp/implementing thegeneric board, LED, and console interfaces. The CMake target is named
nucleo_bsp,matching
polarfire_bsp._sbrk()bounds the heap against_heap_limit, a new linker-scriptsymbol at the end of the heap reservation, and takes a PRIMASK critical section.
bsp_console_init()owns USART2 configuration; pin and clock setupstays with
HAL_UART_MspInit().APP_CONFIGandCONFIG_DIRlogic from theCMake and build scripts.
build-arm-nucleojob to.github/workflows/ci.yml, pinnedto Arm GNU Toolchain 14.2.Rel1.
Review fixes
Applied on top of the original commit after review:
_sbrk()bounded the heap atBSP_RAM_END(0x20018000), the end of physical SRAM. Thatlet
malloc()grow the break through the reserved main stack, through the whole ThreadXbyte pool holding every thread stack, and into the live MSP, all while reporting success.
It now stops at
_heap_limit(0x20001370), inside the 0x200 the linker script reserves.bsp_console_init()had dropped theHAL_UART_Init()return value, so a failed UARTbooted on silently. Restored the failure path (MISRA C:2012 Rule 17.7).
UartHandlewas a global whose only declaration died withboard_init.h; now file-scopestatic console_uart(Rules 8.4, 8.7).newlib_stubs.c._write()and
_read()had been archived into the BSP static library, where the linker extractedthem only incidentally.
uintptr_tinstead of by forming out-of-bounds pointers;prototypes added ahead of every syscall definition; the
HAL_UART_Transmit()const castdocumented as a Rule 11.8 deviation.
CMAKE_C_STANDARD11 -> 99, and the ARM toolchain pinned to GCC 14.2.Rel1 rather thantracking the runner's distribution package (13.2). AGENTS.md specifies GCC 14 and C99.
README.mdcorrected: it had moved withgit mvbut still described the pre-migrationpaths,
board_init.c, andlib/nucleo_bsp/. Validation figures re-measured.AI assistance disclosure
bsp_board.c,bsp_led.c,bsp_console.candnewlib_stubs.ccarry the AGENTS.md AIDisclosure header. They derive from
board_init.c,nucleo_bsp.candconsole.c, each ofwhich carried
// Some portions generated by Codex (GPT-5).before this migration droppedit.
That Codex work was mine (@fdesbiens), not @AmmarOkla12772's. It was done while editing
PR #45 under the AGENTS.md exception allowing maintainer edits to incoming PRs, and landed
via squash-merge 1bddd17 — which assigns git authorship to the PR author while the real
participants appear only in its trailers (
Ammar Okla,Frederic Desbiens,Codex). Thedisclosure headers are not a statement about this PR's author.
Verification
nucleo_f401re.elfunder both GCC 13.2.1 and the pinned GCC 14.2.1, with-Wall -Wshadow -Wdouble-promotion -Werror. 20120 B ROM, 6000 B RAM._end0x20001170 ->_heap_limit0x20001370,clear of
__RAM_segment_used_end__0x20001770 where the ThreadX pool begins._write,_readand_sbrkconfirmed resolved from the application object, absent fromlibnucleo_bsp.a.R100with zero vendor-tree churn.Not addressed
No runtime regression test for this target. The PolarFire target gates CI on a headless
Renode suite;
build-arm-nucleoonly checks that the ELF exists, so the clock configuration,TIM2 timebase, console and the new heap bound are unexercised. Renode has STM32F4 platform
support and this gap should be closed in a follow-up.