Assembled the module ports, which no check had ever compiled (#672)

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) <noreply@anthropic.com>
This commit is contained in:
Frédéric Desbiens
2026-08-28 09:24:30 -04:00
committed by GitHub
parent 147754cc86
commit 9c32abb17d
35 changed files with 94 additions and 6 deletions
+13 -1
View File
@@ -211,7 +211,13 @@ say ""
say "== Assembly sources of every Arm gnu port =="
total=0
for dir in ports/*/gnu/src ports_smp/*/gnu/src ports_module/*/gnu/src; do
# The module ports keep their assembly in module_manager/src, not src. This
# glob read ports_module/*/gnu/src until 26 Aug 2026; that directory does not
# exist, the [ -d ] guard below skipped it in silence, and 116 files across
# nine Arm module ports were assembled by no check with either compiler. The
# count went from 724 to 840 when the path was corrected, and three of the new
# files did not assemble.
for dir in ports/*/gnu/src ports_smp/*/gnu/src ports_module/*/gnu/module_manager/src; do
[ -d "$dir" ] || continue
core="$(echo "$dir" | cut -d/ -f2)"
spec="${PORT_TARGET[$core]:-}"
@@ -246,7 +252,13 @@ say "== Assembly behind feature macros =="
for macro in $FEATURE_MACROS; do
macro_total=0
macro_bad=0
# The module ports are named here for the same reason as in the stage
# above: they were absent from this list until 26 Aug 2026 and so read as
# covered. Adding them found the Cortex-M23 module manager carrying the
# very POP {r0, lr} this comment describes, six months after the same fix
# landed in its non-module sibling.
for src in $(grep -rl "$macro" ports/*/gnu/src/*.S ports_smp/*/gnu/src/*.S \
ports_module/*/gnu/module_manager/src/*.S \
2>/dev/null | sort); do
core="$(echo "$src" | cut -d/ -f2)"
spec="${PORT_TARGET[$core]:-}"
+55 -1
View File
@@ -39,6 +39,11 @@
# body in ports/cortex_m4/ac6/inc/tx_port.h, which placed statements
# outside any function. See issue 569.
#
# 4. Lowercase .s under a gnu tree. GAS preprocesses .S and not .s, so a
# .s file's #ifdef blocks are assembled whichever way the macro is set.
# Twenty-nine files were in that state, including one non-module SMP
# kernel port, and only one of them failed to assemble.
#
# The last section reports, without failing, on port families that have no copy
# script and so cannot be checked for reproducibility.
#
@@ -197,7 +202,56 @@ done < <(find ports ports_arch ports_module ports_smp -name "*.h" -type f \
[ "$orphans" -eq 0 ] && say " ok: no port header carries code at file scope"
# --------------------------------------------------------------------------
# 4. Report only: families with no copy script.
# 4. GNU assembly that uses the preprocessor must be named .S, not .s.
# --------------------------------------------------------------------------
# GAS runs the C preprocessor on .S and not on .s. In a .s file every line
# beginning with # is just a comment, so nothing fails and nothing is
# substituted: a #define constant reaches the assembler as an undefined
# symbol, an #ifdef block is assembled whatever the macro says, and an
# #if/#else pair emits *both* branches. The port silently ignores its own
# feature macros.
#
# Measured 26 Aug 2026, when a corrected glob in check_clang.sh first offered
# the module ports to a compiler. Twenty-nine files were affected, and the
# damage was not only cosmetic:
#
# ports_smp/cortex_a7_smp/gnu/src/tx_thread_smp_unprotect.s -- the only .s
# in a directory of twenty-one .S -- wrote the caller's LR into the
# protection structure on every unprotect, a store guarded by
# TX_MPCORE_DEBUG_ENABLE, and returned through both BX lr and MOV pc, lr.
#
# ports_module/cortex_m33/.../tx_thread_stack_build.s emitted both arms of
# an #ifdef TX_SINGLE_MODE_SECURE, so the second LR value overwrote the
# first and the secure build got the non-secure one.
#
# ports_module/cortex_a7/.../tx_thread_schedule.s did fail to assemble, on
# GCC 14.3 as well as on LLVM, because #define SYS_MODE was never expanded.
# That is the only one of the twenty-nine any compiler complained about.
#
# Only the gnu trees are checked. The IAR, Arm Compiler 5 and Keil assemblers
# preprocess .s themselves, so the same combination is correct there, and
# around three hundred files in this repository depend on it.
say ""
say "== GNU assembly using the preprocessor is named .S =="
lowercase=0
while IFS= read -r f; do
first="$(grep -nE '^[ \t]*#[ \t]*(define|include|if|ifdef|ifndef|else|elif|endif|undef)\b' \
"$f" | head -1)"
if [ -n "$first" ]; then
fail "$f: preprocessor directive in a .s file, which GAS does not preprocess"
echo " line $first"
echo " Rename the file to .S. Check the callers first: a build script"
echo " that already names it .S is the usual sign of how this happened."
lowercase=$((lowercase + 1))
fi
done < <(find ports ports_arch ports_module ports_smp -name "*.s" -type f \
-path "*/gnu/*" 2>/dev/null | sort)
[ "$lowercase" -eq 0 ] && say " ok: no .s file under a gnu tree uses the preprocessor"
# --------------------------------------------------------------------------
# 5. Report only: families with no copy script.
# --------------------------------------------------------------------------
# These are maintained by hand, so a fix applied to one toolchain can silently
# miss the others. Nothing here fails the run; it is a prompt to look.