size: our own calls pass arguments in registers, and the gate that keeps it honest

CHANGELOG.md

@@ -9,6 +9,48 @@ version at the top when it merges.
 
 ## Unreleased
 
+- Every figure below is from a tree with nothing uncommitted, and that matters
+  here more than usual: the build stamps `-dirty` into the version hash inside
+  every image whenever `git status` is not empty, and it lands unevenly -- 8
+  bytes on the rp0 arm against 4 on the rp3 arm in the default drawer, so a
+  dirty tree moves the delta. Commit or stash before measuring a size here.
+- `-mregparm=3` is what every drawer builds with, so our own calls pass their
+  arguments in registers. `bsdsocket.library` 353,404 -> 329,464 loaded bytes,
+  370,880 -> 346,608 file, 343,420 -> 319,480 of code; `anxnet.device` 43,584 ->
+  40,144 loaded, 45,704 -> 42,228 file; the resident pair -27,380.
+- Default drawer, 137 images: 7,917,004 -> 7,707,384 loaded (-209,620, -2.65%),
+  6,772,936 -> 6,560,528 file (-212,408, -3.14%). Minimal -93,460 loaded, micro
+  -87,364. Loaded is CODE + DATA + BSS, what `LoadSeg` needs room for. Measured
+  against the same tree built with `-DAMINETXDUO_REGPARM=0`.
+- Five test images grew 4-20 bytes: `tests/perf/chipscreen` +20,
+  `tests/perf/n68kmv` +16, `tests/tools/AamProbe`, `ResolveBreak` and `PtrProbe`
+  +4 each. No shipping image grew, and no image is in one arm only.
+- No LVO changed: every one is entered through a register the NDK headers pin.
+  207 foreign direct calls in `bsdsocket.library` at regparm 0, 201 at regparm 3,
+  and the only sites pushing in neither arm are libgcc's internal `jsr`.
+- The boundaries the compiler does not build keep the stack convention, pinned
+  where declared: `main()` (`crt0.o` and `src/tools/tool_startup.S` push `argv`
+  then `argc`; `include/aminetxduo/asm_main.h`), the assembly in `src/net68k` and
+  `src/crypto68k` reading `4(sp)`, `8(sp)`, `12(sp)` (`asm_abi.h`), `libc.a`,
+  `amiga.lib`, and the OS entry points whose registers the headers pin by name.
+- Gate: `-Wmissing-prototypes` with `-Werror=implicit-function-declaration`, and
+  an `#error` in `include/aminetxduo/asm_main.h` unless the m68k build is C23 or
+  later -- where `()` means `(void)` and an unprototyped `f(a, b)` is a hard
+  error. `-Wstrict-prototypes` was removed: it cannot fire on the m68k arm, and
+  the three host sites it does fire on are the NDK's and the vendored tree's.
+- `src/net68k/n68k_memcpy_hook.c` includes `<string.h>`, so its stack convention
+  is stated by a declaration and not left to GCC's recognition of the name
+  `memcpy`. Two clean trees differing only in that include leave both shipping
+  images byte-identical but for the 7 hash bytes the version string carries.
+- `tests/fuzz/CMakeLists.txt` puts `include/` on `fuzz_tls_crypto`'s path for
+  `src/crypto68k/crypto68k.h`'s `<aminetxduo/asm_abi.h>`. That target exists
+  only where `sizeof(void*) == 4`, so CI was the first to build it: it found the
+  gap in host32 and sanitize32, not on any host or cross arm.
+- Host tier: 434 targets, 0 warnings, ctest 493/493. 32-bit host tier: 24/24 and
+  within its duration budget. Emulator tier: green on both arms.
+- Build consequence: an image must come from one configuration. A `build/`
+  directory made before this change needs its objects rebuilt, not relinked.
+
 - `CheckNetConfig` names the file a default-gateway finding was read from even
   when that file is an interface file. `load_gateway()` falls back to a
   `GATEWAY=` in the first interface file when neither

CMakePresets.json

@@ -13,7 +13,8 @@
       "toolchainFile": "${sourceDir}/cmake/toolchain-m68k-amigaos.cmake",
       "binaryDir": "${sourceDir}/build/${presetName}",
       "cacheVariables": {
-        "CMAKE_BUILD_TYPE": "Release"
+        "CMAKE_BUILD_TYPE": "Release",
+        "CMAKE_PROJECT_INCLUDE": "${sourceDir}/cmake/ci-warnings.cmake"
       }
     },
     {

cmake/ci-warnings.cmake

@@ -1,92 +1,97 @@
 # Turn compiler warnings into build failures, for OUR sources only.
 #
-# Usage (no edit to any CMakeLists.txt is needed; CMake includes this file at
-# the end of the top-level project() call):
+# Included at the end of the top-level project() call.  Every preset sets
+# CMAKE_PROJECT_INCLUDE to this file (CMakePresets.json, hidden `amiga` base),
+# so there is no way to configure one of this project's own drawers without the
+# gate.  tools/ci.sh passes the same option explicitly; that is redundant and
+# kept only because the ci.sh host arm configures a directory no preset
+# describes.
 #
-#   cmake -S . -B build -DCMAKE_PROJECT_INCLUDE=cmake/ci-warnings.cmake
+# Override with -DAMINETXDUO_WARNING_FLAGS="-Wall;-Wextra", turn the whole
+# thing off with -DAMINETXDUO_WERROR=OFF.
 #
-# Override the flag set with -DAMINETXDUO_WARNING_FLAGS="-Wall;-Wextra", and
-# turn the whole thing off again with -DAMINETXDUO_WERROR=OFF.
-#
-# WHY IT IS NOT JUST add_compile_options(-Wall -Wextra -Werror)
-#
-#   Most of what this project compiles is not this project: ThreadX, NetX Duo,
-#   nx_crypto and nx_secure are vendored verbatim as submodules and are not
-#   warning-clean under -Wextra.  A global flag would therefore fail the build
-#   on code we have a standing rule never to modify.
-#
-#   Filtering by target does not work either, `threadx`, `netxduo`,
-#   `netxduo_addons` and `crypto68k_ref` are declared in OUR CMakeLists.txt
-#   files but compile vendored sources.  So the filter is per SOURCE FILE:
-#   every source whose path contains /third_party/ is left alone, everything
-#   else gets the flags.  Nothing has to be kept in a list, so a new component
-#   is covered the day it is added.
+# Filtering is per SOURCE FILE, not per target: most of what this project
+# compiles is not this project (the ThreadX, NetX Duo, nx_crypto and nx_secure
+# submodules are vendored verbatim and are not warning-clean under -Wextra),
+# while targets like `threadx` and `netxduo` are declared in OUR CMakeLists.txt
+# and compile vendored sources.  Paths containing /third_party/ are left alone;
+# nothing has to be kept in a list.
 #
 # The work is deferred to the end of the top-level directory because targets do
-# not exist yet at the point this file is included, add_subdirectory() has
-# not run.  set_property(SOURCE ... TARGET_DIRECTORY ...) is what makes it
-# legal to reach into a target declared in another directory.
+# not exist yet when this file is included.  set_property(SOURCE ...
+# TARGET_DIRECTORY ...) is what makes reaching into another directory legal.
 #
 # SPDX-License-Identifier: MIT
 
 option(AMINETXDUO_WERROR "Fail the build on any warning in our own sources" ON)
 
-# -Wmissing-prototypes is here so a function that is not static has to say what
-# it is somewhere a caller can see.  It found 34: eight that were only ever used
-# in their own file and are static now, one dead accessor, two asm-facing entry
-# points nothing declared, a C fallback a macro renamed out from under its own
-# prototype, and a handful of files that simply did not include the header
-# already declaring what they defined -- config_advice.c defined ami_cfg_advice()
-# without including the header that tells callers its shape.
+# -Wmissing-prototypes: a function that is not static has to say what it is
+# where a caller can see it.  Found 34.  Not a size lever -- single-unit LTO
+# already saw everything, so static unlocked nothing the linker did not have;
+# what it buys is the compiler checking every definition against what callers
+# were told.
 #
-# It is not a size lever.  Measured across the whole change: bsdsocket.library
-# 339,468 -> 339,476 bytes, +8.  Single-unit LTO already saw everything, so
-# static unlocked no internalization the linker did not have.  What it buys is
-# the compiler checking every definition against what callers were told.
-set(AMINETXDUO_WARNING_FLAGS "-Wall;-Wextra" CACHE STRING
-    "Warning flags applied to sources outside third_party/")
-
-# SHIPPING SOURCES ONLY -- src/ and port/, not tests/.
+# -Wstrict-prototypes is NOT here, deliberately, and what replaced it is a hard
+# error in the language rather than a warning here.
 #
-# -Wmissing-prototypes says a function that is not static has to declare itself
-# somewhere a caller can see.  That is exactly right for code that ships, and
-# it found real things: a definition whose header was never included
-# (config_advice.c defined ami_cfg_advice() without including config.h, so
-# nothing checked it against what every caller is told), a C fallback a macro
-# had renamed out from under its own prototype, and eight functions never used
-# outside their own file.
+# It was added for the mixing hazard -mregparm introduces: `VOID f();' declares
+# no prototype, so `f(a, b)' travels on the stack while a definition compiled
+# for -mregparm=3 reads registers, and the callee returns a wrong number.  Two
+# things make the flag the wrong tool.  It cannot fire where the hazard exists:
+# the m68k compiler is GCC 16 and compiles as C23 (__STDC_VERSION__ 202311L with
+# no -std given), where `()' MEANS `(void)' and `f(a, b)' is "too many arguments
+# to function" -- a hard error for a direct call and for a call through a
+# function pointer alike.  And where it does fire it cannot be satisfied: a full
+# host build (gcc 14, C17) reports 26 diagnostics from just three sites, none of
+# them ours to change -- the NDK's own `VOID (*putChProc)()' in exec_protos.h,
+# which forces the cast at src/bsdsocket/loghook.c:163; the NDK-impersonating
+# src/netdev/test/shim/exec/interrupts.h; and the vendored
+# third_party/netxduo/nx_secure/inc/nx_secure_tls_api.h.
 #
-# It is NOT right for tests.  tests/fuzz/fuzz_dns.c DEFINES
-# _tx_thread_system_suspend() to stand in for ThreadX's, and there is no header
-# it could be declared in that would not be a forgery of upstream's.  A test
-# that impersonates a symbol on purpose is not the bug this flag looks for, and
-# the findings there are per-configuration noise: each CI arm compiles a
-# different set of stubs.
-set(AMINETXDUO_SHIPPING_WARNING_FLAGS "-Wmissing-prototypes" CACHE STRING
-    "Warning flags applied to src/ and port/ only")
-
-# Per-file escapes, as <path fragment> <extra flags> pairs.  Every entry is a
-# bug someone has to fix, so each one says what it is; this list should shrink.
+# The guarantee is therefore asserted where it is relied on:
+# include/aminetxduo/asm_main.h #errors unless the m68k build is C23 or later.
+# That covers a hand-written compile line too, which no flag here would.
 #
-# IT IS EMPTY, and the last entry to leave is worth recording, because it did
-# not leave for the reason its own note gave.
+# -Werror=implicit-function-declaration: C23 makes it an error anyway; naming it
+# stops a future -std from taking that away quietly.
 #
-# src/config/test/test_config.c held -Wno-error=address for CHECK_STR's
-# `(got) ? (got) : "(null)"` printf argument, which is -Waddress when `got` is
-# an array.  That form was replaced by an or_null() helper at some point after
-# the escape was written, and nobody took the escape back out: measured on gcc
-# 14.2 with this file's own flags, the ternary form gives 7 -Werror=address and
-# the or_null form gives none.  SO THE ESCAPE HAD BEEN DEAD, and an escape that
-# is dead is worse than one that is needed -- it is a hole nothing is watching,
-# ready for the next real warning in that file to fall through silently.
+# MEASURED 2026-09-30, full host build with -Werror on (gcc 14.2, C17):
+# 0 -Wmissing-prototypes from shipping sources, 0 -Wcast-function-type, 0
+# implicit-function-declaration.  Everything -Wmissing-prototypes finds is a
+# test, which is why it is off for them below.
+set(AMINETXDUO_WARNING_FLAGS
+    "-Wall;-Wextra;-Werror=implicit-function-declaration;-Wmissing-prototypes"
+    CACHE STRING "Warning flags applied to sources outside third_party/")
+
+# NON-SHIPPING -- anything not under src/ or port/, plus the host tests under
+# src/<component>/test/.
 #
-# CHECK_STR is a function now, so the null guard is expressed once instead of
-# at 132 expansion sites.  That does not make the guard fire for array callers;
-# nothing can, an array is not null.  It means the compiler is not asked to
-# prove the same tautology 132 times, which is the thing that needed an escape.
+# -Wmissing-prototypes is right for code that ships and wrong for tests.  A test
+# that impersonates a symbol on purpose (tests/fuzz/fuzz_dns.c DEFINES
+# _tx_thread_system_suspend()) is not the bug the warning looks for, and the
+# findings are per-configuration noise: each CI arm compiles a different set of
+# stubs.  One rule, not an entry per file.
 #
-# Adding an entry here is fine.  Leaving a dead one is not: check that the
-# warning still fires before assuming an entry is load-bearing.
+# -Wno-cast-function-type used to be here, for the five tests that install a Hook
+# by casting one -- the NDK declares `h_Entry' as ULONG (*)(VOID) while a Hook
+# function is really ULONG (*)(struct Hook *, APTR, APTR).  It is gone because
+# it has no subject left: main replaced all five casts with union punning
+# (4607a0c6, test_netmon_host.c and its siblings now assign through a `HookEntry'
+# union), no shipping source ever cast h_Entry -- src/ only READS it -- and a
+# full host build with the escape removed reports 0 -Wcast-function-type.  An
+# escape nothing can trip is a hole nothing is watching; see the note on
+# AMINETXDUO_WARNING_EXEMPT below.
+set(AMINETXDUO_NONSHIPPING_WARNING_FLAGS
+    "-Wno-missing-prototypes"
+    CACHE STRING "Warning flags applied to sources that do not ship")
+
+# Per-file escapes, as <path fragment> <extra flags> pairs.  Each entry is a bug
+# somebody has to fix, so it says what it is; this list should shrink.  IT IS
+# EMPTY: the last entry, -Wno-error=address for src/config/test/test_config.c,
+# had been dead -- the ternary it was written for was replaced by an or_null()
+# helper and nobody took the escape out.  A dead escape is worse than a needed
+# one, a hole nothing is watching.  Check a warning still fires before assuming
+# an entry is load-bearing.
 set(AMINETXDUO_WARNING_EXEMPT)
 
 function(_aminetxduo_warnings_apply_dir dir)
@@ -115,25 +120,29 @@ function(_aminetxduo_warnings_apply_dir dir)
 
         set(_ours "")
         set(_shipping "")
+        set(_nonshipping "")
         foreach(_s IN LISTS _srcs)
             if(NOT IS_ABSOLUTE "${_s}")
                 set(_s "${_sdir}/${_s}")
             endif()
-            # Generator expressions and generated sources are skipped: they are
-            # not ours to police and cannot be path-matched reliably.
+            # Generator expressions and generated sources cannot be
+            # path-matched reliably, so they are not policed.
             if(_s MATCHES "\\$<")
                 continue()
             endif()
             if(_s MATCHES "/third_party/")
                 continue()
             endif()
             # tests/atf/ holds FreeBSD's tests/sys/netinet sources byte for
-            # byte, under their own BSD-2-Clause header, so the same rule
+            # byte under their own BSD-2-Clause header, so the same rule
             # applies: not ours, not modifiable, not warning-clean against the
             # Roadshow NDK's prototypes (sendto() takes APTR where FreeBSD's
-            # takes const void *).  The shim beside them -- atf-c.h,
-            # atf_main.c, atf-prelude.h -- is ours and is not exempt.
-            if(_s MATCHES "/tests/atf/" AND NOT _s MATCHES "/tests/atf/atf")
+            # takes const void *).  That is tcp_socket.c and whatever lands
+            # beside it, so the exemption names the files that are OURS
+            # instead of guessing from a filename prefix.  The shim is atf-c.h,
+            # atf-prelude.h and atf_main.c, and it is not exempt.
+            if(_s MATCHES "/tests/atf/"
+               AND NOT _s MATCHES "/tests/atf/(atf-c\\.h|atf-prelude\\.h|atf_main\\.c)$")
                 continue()
             endif()
             list(APPEND _ours "${_s}")
@@ -144,21 +153,25 @@ function(_aminetxduo_warnings_apply_dir dir)
             if((_s MATCHES "/src/" OR _s MATCHES "/port/")
                AND NOT _s MATCHES "/test/")
                 list(APPEND _shipping "${_s}")
+            else()
+                list(APPEND _nonshipping "${_s}")
             endif()
         endforeach()
 
         if(_ours)
             # APPEND, not set_source_files_properties(): src/crypto68k already
-            # puts COMPILE_OPTIONS on its two .S files and overwriting them
+            # puts COMPILE_OPTIONS on its two .S files and overwriting those
             # would drop the -m68020 the assembler needs.
             set_property(SOURCE ${_ours} TARGET_DIRECTORY ${_t}
                          APPEND PROPERTY COMPILE_OPTIONS ${_flags})
 
-            # The shipping-only half: src/ and port/, never tests/.
-            if(_shipping AND AMINETXDUO_SHIPPING_WARNING_FLAGS)
-                set_property(SOURCE ${_shipping} TARGET_DIRECTORY ${_t}
+            # Everything that does not ship gets the two flags that only make
+            # sense for shipping code turned back off.  Set after the main list
+            # so the -Wno- wins.
+            if(_nonshipping AND AMINETXDUO_NONSHIPPING_WARNING_FLAGS)
+                set_property(SOURCE ${_nonshipping} TARGET_DIRECTORY ${_t}
                              APPEND PROPERTY COMPILE_OPTIONS
-                             ${AMINETXDUO_SHIPPING_WARNING_FLAGS})
+                             ${AMINETXDUO_NONSHIPPING_WARNING_FLAGS})
             endif()
 
             # ... then the escapes, which have to come after to win.