summaryrefslogtreecommitdiff
path: root/common_modules/module_manager
diff options
context:
space:
mode:
authorFrédéric Desbiens <[email protected]>2026-08-28 10:40:37 -0400
committerGitHub <[email protected]>2026-08-28 10:40:37 -0400
commitff7fbe02f873170d1bd32b7c2ba7eae8d5e59f96 (patch)
tree3cdf4b612bb89fe866343c3136b9906f2e7be434 /common_modules/module_manager
parent5b94bad6a207ebb6d8135d719efff0de86e712e6 (diff)
Added a GCC check for the Arm ports and ran it in CI (#675)
* Assembled the module ports, which no check had ever compiled scripts/check_clang.sh globbed ports_module/*/gnu/src, which does not exist -- the module ports keep their assembly in module_manager/src. The [ -d ] guard skipped it in silence, so 116 assembly files across nine Arm module ports were assembled by no check, with either compiler, in the script whose own comments state three times that "a port that is simply absent from the count reads as covered". Stage 1 goes from 724 of 724 to 840 of 840; the feature-macro stage had the same gap and goes from 412 files to 469. Correcting the path exposed five defects, and only one of them was a build failure. The other four assembled cleanly and did the wrong thing, because GAS runs the C preprocessor on .S and not on .s: ports_smp/cortex_a7_smp/gnu/src/tx_thread_smp_unprotect.s, the only .s in a directory of twenty-one .S, ignored all four of its own feature macros. It wrote the caller's LR into the protection structure on every unprotect -- a store guarded by TX_MPCORE_DEBUG_ENABLE -- sent an unconditional SEV, and returned through both BX lr and MOV pc, lr. Its cortex_a5_smp and cortex_a9_smp siblings are .S. ports_module/cortex_m33/.../tx_thread_stack_build.s emitted both arms of an #ifdef TX_SINGLE_MODE_SECURE, so the non-secure LR value overwrote the secure one and the secure build got the wrong frame. ports_module/cortex_m23/.../tx_thread_context_{save,restore}.S carried the POP {r0, lr} that check_clang.sh's own comment describes as the reason the feature-macro stage exists. The 16-bit Thumb POP takes r0-r7 and pc only. The identical fix already sits in ports/cortex_m23/gnu/src; the module copy never got it because nothing scanned it. ports_module/cortex_m23/.../tx_thread_secure_stack_initialize.S used MOV rather than MOVS for an 8-bit immediate, latent behind TX_SINGLE_MODE_SECURE. Both siblings in the same directory already use MOVS. ports_module/cortex_a7/gnu/module_manager/src is the one that failed to assemble, on GCC 14.3 as well as on LLVM: #define SYS_MODE was never expanded, so #SYS_MODE reached the assembler as an undefined symbol. Twenty-nine .s files under gnu trees are renamed to .S. Every one of them is already named .S by the build scripts that compile it, so this repairs those scripts rather than churning them -- ports_module/cortex_a7's build_threadx.bat names all eighteen with a capital S, and works today only on a case-insensitive filesystem. Renaming rather than converting the #defines to GNU assignments is what fixes the #ifdef blocks as well as the constants; the assignments would have fixed two files and left twenty-seven silently ignoring their macros. Files with no preprocessor directives are left as .s: they are not broken, and check_ports.sh gains a check that keeps them that way. Only the gnu trees are checked there -- the IAR, Arm Compiler 5 and Keil assemblers preprocess .s themselves, and about three hundred files in this repository rely on that. Verified with both toolchains on the same tree: 840 of 840 assembled by ATfE 22.1.0 and by arm-gnu-toolchain 14.3.rel1, all five stages of check_clang.sh green, and check_ports.sh green including the reproducibility check. The new check was shown to fail by planting a copy of the file it was written for. No regression test accompanies this. The assembly it covers is executed by no host test, and the check itself going from 724 files to 840 is the coverage AGENTS.md asks for -- together with the new check_ports.sh section, which is what stops the class recurring. Assisted-by: Claude Code (Opus 5) <[email protected]> * Fixed the AArch64 samples, none of which had ever linked with GCC Every AArch64 gnu example build failed at the sample link, all 27 of them -- 13 under ports/ and 14 under ports_smp/: libg.a(libc_a-init.o): in function `__libc_init_array': undefined reference to `_init' relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `_init' libg.a(libc_a-fini.o): in function `__libc_fini_array': undefined reference to `_fini' build_threadx_sample.sh links with -nostartfiles, which is correct for a port carrying its own reset path, and that drops crti.o and crtn.o along with everything else. startup.S calls __libc_init_array by design, and newlib's implementation calls _init, which crti.o is what defines. The AArch32 scripts are unaffected: they use nosys.specs and never reach __libc_init_array. The fix links crti.o and crtn.o explicitly, bracketing the object list -- the first must precede every .init contribution and the second must follow all of them, so their position is load-bearing rather than stylistic. Both paths come from the compiler's own -print-file-name, so nothing here hard-codes a toolchain layout. The atfe branch sets both to empty, deliberately: picolibc's __libc_init_array does not call _init, those 27 images link today, and adding crti.o would change a working link for no reason. That is also why check_clang.sh is green on these and does not list them as expected to fail -- the LLVM path never reached the gap, so nothing has ever linked them and failed. Fixed in ports_arch/ARMv8-A/threadx/ports/gnu/example_build, which is the single source for both the ports/ and ports_smp/ copies, then regenerated with update.sh --port-sets tx,tx_smp. The 27 generated copies are in this commit because ports_arch_check compares them. Verified: all 27 link with arm-gnu-toolchain 14.3.rel1 aarch64-none-elf, where 0 of 27 did before; _init and _fini disassemble to the expected crti prologue and crtn epilogue over a ret; check_clang.sh with ATfE 22.1.0 is still green on all five stages, including the 42 script-driven example builds; check_ports.sh is green including the reproducibility check. No regression test: these are link-only example images that no host test executes. What guards them is check_clang.sh's example stage today, and check_gcc.sh's, which is the next change and is the reason this was found. Assisted-by: Claude Code (Opus 5) <[email protected]> * Added a GCC check for the Arm ports, which nothing had ever compiled GCC is the project's declared default compiler (AGENTS.md, "The default compiler for the project is GCC 14 on Linux"), it is what the gnu ports exist for, and it is what nearly every downstream user builds with -- and nothing in CI compiled a line of any port with it. The only cross-compilation check that ran was the LLVM one, so the ATfE path was better guarded than the GNU one, on ports whose directory is literally named gnu. ci_cortex_m covers four port families; this covers forty. Five stages, mirroring scripts/check_clang.sh stage for stage: 1. assemble every .S and .s of every Arm gnu port -- 840 files 2. assemble again the parts behind TX_ENABLE_VFP_SUPPORT, TX_ENABLE_FIQ_SUPPORT, TX_LOW_POWER and TX_ENABLE_EXECUTION_CHANGE_NOTIFY -- 469 files 3. compile common/src for one core per architecture profile -- 185 x 9 4. link the script-driven example builds -- 42 5. link the CMake-driven Cortex-R52 images -- 5 Two scripts rather than one with a --toolchain flag: the flag surface differs (a prefixed driver against --target=), the C library differs, and the set of examples that can link differs. Folding them together makes it easy to weaken one check while working on the other. Two toolchains, both required. Arm ships arm-none-eabi and aarch64-none-elf as separate downloads and PORT_TARGET maps every port to one of exactly those two triples, so --arm-none-eabi and --aarch64-none-elf each take a driver or the directory holding it, defaulting to the environment and then to PATH. A missing one is a hard error rather than a soft skip: letting a run cover half the tree and still report "all checks passed" is the failure this script exists to end. PORT_TARGET is copied verbatim from check_clang.sh, including its warning not to prefix-match core names -- cortex_a5* also matches the AArch64 cortex_a53. VFP_EXTRA is the one map that is not a copy, and check_clang.sh's comment about it is false for GCC. That comment says the A-profile defaults are already correct; arm-none-eabi-gcc defaults to -mfloat-abi=soft, which disables the FPU outright, so every VFP file fails with "selected processor does not support 'vmrs r1,FPSCR' in ARM mode". -mfloat-abi=hard alone is the fix and is the right one, because it selects the core's own default FPU rather than naming a -d16 one -- which is the trap the clang script warns about, since the A-profile paths save D16-D31. Cortex-R4 is the exception in both scripts and for the same reason: its FPU is an option rather than part of the core, so an explicit -mfpu is required. Every value was measured against 14.3.rel1. Stage 4 *unsets* TOOLCHAIN rather than setting it. The example build scripts already default to GNU, and a stray TOOLCHAIN=atfe from a developer's shell would otherwise make this stage silently check the other compiler. It cleans the example directories on both sides, because the success test is the existence of sample_threadx.out rather than the driver's exit status, and a stale image from a previous toolchain would report success. Failure logs are printed unfiltered: a missing tool says "command not found", and GNU ld's undefined-symbol lines carry no "error:" at all. Every skip is printed by name with a reason, per the house rule check_clang.sh states three times -- a port simply absent from the count reads as covered. This script also says outright that arm9 and arm11 are Arm and are skipped for having no PORT_TARGET entry, which the clang script's "not Arm" wording glosses. Verified on this tree with arm-gnu-toolchain 14.3.rel1: all five stages green, every count identical to check_clang.sh's on the same tree -- 840, 469, 185x9, 42, 5 -- in 4m28s. The failure paths were tested, not assumed. A deliberately broken .S in a module port is reported by name and line in stages 1 and 2 and exits 1, in --quiet mode as well. Reverting the AArch64 _init/_fini fix on one port only gives "FAIL: cortex_a53: example build produced no image", 41 of 42, and exit 1 -- and the log tail it prints contains no "error:" anywhere, which is why it is not filtered. A missing or wrong toolchain path exits 1 naming which triple was not found. RISC-V is deliberately out of scope for this first version: both ports assemble 8 of 8 with the project's own cmake flags, but adding them widens the toolchain download and the review surface for a family that is not regressing. No regression test accompanies this. The script is the test, it exercises no runtime behaviour, and its own failure paths are exercised above. Assisted-by: Claude Code (Opus 5) <[email protected]> * Ran the GCC port check in CI, on dev as well as master scripts/check_gcc.sh with nothing invoking it would be a script nobody runs. This adds the workflow, modelled on clang_check.yml, and fixes a trigger gap in that file at the same time. One job, two cache steps. Arm ships AArch32 and AArch64 as separate downloads and the script needs both, so two caches keep the checks list short and let a single invocation see both compilers. The AArch32 cache path and key match cortex_m's exactly, so the two workflows share one entry rather than each holding its own copy of the same archive -- noted in a comment, because the only symptom of breaking that is a slower run. Both triggers name dev. A workflow that triggers only on master gates no pull request anybody opens; that is the defect ports_arch_check.yml carries a comment about, and it cost cortex_m three months of failing in seven seconds unnoticed. push is included as well as pull_request so dev's own history has a baseline and a bad squash-merge is caught rather than waiting for the next PR. The checksum suffix is .sha256asc and it is not interchangeable with .sha256. Arm publishes both for this release, and verified 26 Aug 2026, the .sha256 file for arm-none-eabi contains a 32-character MD5 rather than a SHA-256, so sha256sum -c on it fails with "no properly formatted checksum lines found". .sha256asc is a plain sha256sum-format line for both triples. The plan warned that this suffix had changed between releases; the sharper truth is that both suffixes exist simultaneously and one of them is not a SHA-256 at all. Recorded in a comment beside the step. Verified before writing them in rather than copied: both archive URLs and both checksum URLs resolve, the archives are xz, the checksum files are sha256sum-format for .sha256asc, and the AArch64 archive extracts to arm-gnu-toolchain-14.3.rel1-x86_64-aarch64-none-elf/bin/aarch64-none-elf-gcc, which is the path the workflow builds. The paths: lists are duplicated between push and pull_request rather than shared through a YAML anchor, deliberately: GitHub Actions' parser does not dependably honour anchors and the failure mode is the workflow refusing to parse, which is the cortex_m failure again. Ten duplicated lines are cheaper. clang_check.yml's paths: list was missing CMakeLists.txt, cmake/ and common_smp/, so that check did not run when files it reads changed -- the ports_smp example builds compile common_smp/src and its CMake stage reads the toolchain file and the top-level project. Both lists are now identical apart from each file's own name, and both say so. cortex_m is kept rather than deleted, against the plan's recommendation. It builds four ports *through CMake*, and that is the only thing exercising cmake/cortex_m*.cmake and the top-level CMakeLists for the M profile; this script's CMake stage covers cortex_r52 only. The overlap is the assembly and the C sources, not the build system, so deleting it would lose coverage rather than remove a duplicate. Said so in the workflow header. The script is passed explicit toolchain paths rather than left to find the drivers on PATH, so nothing about the runner image can decide which compiler runs, and it prints both versions it resolved. Assisted-by: Claude Code (Opus 5) <[email protected]>
Diffstat (limited to 'common_modules/module_manager')
0 files changed, 0 insertions, 0 deletions