From 5e6fae288efa7869f75c100b56cfca63d8154374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Gro=C3=9Fmann?= Date: Thu, 13 Aug 2026 08:41:03 +0200 Subject: [PATCH 1/3] fix(as370): keep the -Werror build clean on modern GNU gcc Two separate diagnostics broke the project's documented build line on current GNU gcc, while Apple clang never saw either -- so 'warning-clean under -Werror' only held on one toolchain and a contributor on gcc could not build at all. -Wdangling-pointer (#11): expr_val() parked its cursor argument in the file static xp_, and callers pass stack buffers, so on return xp_ pointed at storage that had gone out of scope. Nothing outside the expression evaluator ever reads xp_ -- the only uses are lines 228..283 -- so this was latent rather than live, but the store is genuinely invalid and gcc is right to reject it. Clear xp_ before returning; the early-return path is folded into the same exit so there is one place to do it. -Wformat-overflow (#33): the deck sequence number was printed through a format string built at run time (sprintf(fmt, "%%0%dld", nd)), which gcc cannot bound. Take the width from the argument instead -- snprintf(d, sizeof d, "%0*ld", nd, ...) -- which is the same output with no constructed format and no fmt[] at all. Verified with gcc 16.1 at -O2 -Wall -Wextra -Werror (stricter than the CI runner: it reports #11, and does not happen to reproduce #33) and with clang. Output is unchanged where it matters: 'make test-as370' still reports ALL SAMPLES BYTE-IDENTICAL TO IFOX00, and the libc370 corpus check produces output identical byte-for-byte before and after this commit. Closes #11 Closes #33 Claude-Session: https://claude.ai/code/session_018SbqQkXrAyDhVgcF8X8cdz --- as370/src/as370.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/as370/src/as370.c b/as370/src/as370.c index 10fb138..2039353 100644 --- a/as370/src/as370.c +++ b/as370/src/as370.c @@ -278,11 +278,18 @@ static long x_add(void) { return v; } static long expr_val(const char *e, int *reloc) { + long v = 0; xp_ = e; xrl_ = 0; while (*xp_ == ' ') xp_++; - if (!*xp_ || *xp_ == '(' || *xp_ == ',') { if (reloc) *reloc = 0; return 0; } /* leading '(' = subscript with no displacement prefix */ - long v = x_add(); - if (reloc) *reloc = xrl_; + if (!*xp_ || *xp_ == '(' || *xp_ == ',') { if (reloc) *reloc = 0; } /* leading '(' = subscript with no displacement prefix */ + else { v = x_add(); if (reloc) *reloc = xrl_; } + /* Drop the cursor before returning. Callers hand us stack buffers, so + * leaving this file-static pointing at one that has just gone out of scope + * is a dangling store -- harmless today because nothing outside this + * evaluator reads xp_, but gcc rightly rejects it under -Werror + * (-Wdangling-pointer, issue #11). Clearing it costs nothing and makes the + * lifetime obvious. */ + xp_ = NULL; return v; } /* evaluate a register operand, accepting the (r) parenthesised form (common in @@ -1816,8 +1823,12 @@ static void cseq(unsigned char *c, int seq) { int nl = (int)strlen(deck_id); if (nl > 8) nl = 8; int nd = 8 - nl; /* digits available for the sequence */ for (i = 0; i < nl; i++) c[72 + i] = a2e((unsigned char)deck_id[i]); - if (nd > 0) { char fmt[8], d[16]; long m = 1; int k; for (k = 0; k < nd; k++) m *= 10; - sprintf(fmt, "%%0%dld", nd); sprintf(d, fmt, (long)(seq % m)); + /* Width comes from the argument (%0*ld) rather than a format string + * built at run time: identical output, but gcc can bound it, so the + * -Werror build holds on GNU gcc (issue #33). nd is 1..7 here and + * seq % m < 10^nd, so d[] is ample. */ + if (nd > 0) { char d[16]; long m = 1; int k; for (k = 0; k < nd; k++) m *= 10; + snprintf(d, sizeof d, "%0*ld", nd, (long)(seq % m)); for (i = 0; i < nd; i++) c[72 + nl + i] = a2e(d[i]); } return; } From c7db7af07ce09be563e8687130618456430ba6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Gro=C3=9Fmann?= Date: Thu, 13 Aug 2026 08:41:03 +0200 Subject: [PATCH 2/3] ci: build the tools and run the suites on both gcc and clang cc370 had no CI, while mbt's reusable workflow builds this repo's main for every downstream project -- so a broken main went out to the whole ecosystem and came back as a red build in mvsmf. That is the wrong direction for the signal to travel. Scope is the five standalone tools and their suites, not the GCC fork, since that is where the regressions have been and 'make compiler' takes minutes. Both compilers build the tools on purpose: the -Wformat-overflow break that prompted this reached main because Apple clang does not implement that warning, so a clang-only check cannot defend -Werror. libc370 is cloned as a sibling because four as370 samples assemble against its macro libraries; nothing is built from it. test-corpus stays out (its SHA manifest has drifted from libc370's current sources and it is red regardless of any change here) and so does test-cc370 (needs the GCC fork). Claude-Session: https://claude.ai/code/session_018SbqQkXrAyDhVgcF8X8cdz --- .github/workflows/build.yml | 74 +++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..2f7ccd5 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,74 @@ +# Minimal CI for the cc370 toolchain. +# +# Scope is deliberately the five standalone tools and their suites, not the GCC +# fork: `make compiler` takes many minutes, and every regression this is meant +# to catch lives in the tools. Downstream projects (mbt's reusable workflow) +# already build the full toolchain from this repo's main, so a break here goes +# out to every consumer -- which is exactly what happened when a +# -Wformat-overflow error in xmit370 reached main and turned mvsmf's CI red. +# +# Both gcc and clang build the tools, on purpose. That bug slipped through +# local testing because Apple clang does not implement -Wformat-overflow at +# all, so a clang-only check is not enough to defend `-Werror`. Note the two +# compilers only differ for `make tools`; the test suites drive their own +# compiler. +# +# libc370 is checked out as a sibling because the as370 suite assembles four +# samples against its macro libraries (maclib/sysmac -- PDPTOP, SAVE, RETURN). +# Nothing is built from it; only the macro sources are read. +# +# Not included, and why: +# test-corpus the committed SHA manifest has drifted from libc370's current +# sources, so it is red independently of any change here +# test-cc370 needs the GCC fork (`make compiler`) + +name: Build + +on: + pull_request: + push: + branches: [main] + +jobs: + tools: + name: tools + suites (${{ matrix.cc }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + cc: [gcc, clang] + defaults: + run: + working-directory: cc370 + steps: + - uses: actions/checkout@v4 + with: + path: cc370 + + # as370/tests/run.sh looks for CRENT=../../libc370, i.e. a sibling of the + # repo root -- the same layout the ecosystem uses locally. + - name: Clone libc370 (macro libraries only) + working-directory: . + run: git clone --depth 1 https://github.com/mvslovers/libc370.git libc370 + + - name: Install compilers + working-directory: . + run: | + sudo apt-get update + sudo apt-get install -y build-essential clang + + - name: Compiler version + run: ${{ matrix.cc }} --version + + # The -Werror build is the point of this job. + - name: Build the standalone tools + run: make tools HOSTCC=${{ matrix.cc }} + + - name: as370 suite (byte-identity to the IFOX00 reference decks) + run: make test-as370 + + - name: ld370 suite (byte-identity to the IEWL/IEBCOPY oracles) + run: sh ld370/tests/run.sh + + - name: xmit370 suite + run: make test-xmit370 From 5c53f21032a7af56e3fc2ecdc2aa0f41821f9034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Gro=C3=9Fmann?= Date: Thu, 13 Aug 2026 08:42:16 +0200 Subject: [PATCH 3/3] refactor(as370): finish the crent370 -> libc370 rename in the test harness 61b4e99 renamed the crent370 references but left the shell variable itself called CRENT in tests/run.sh and tests/listref/check.sh, while the Makefile and tests/corpus/check.sh had already moved to LIBC370. Two names for one thing, and the stale one points at a libc that is frozen and no longer checked out. Rename to LIBC370 everywhere so the whole harness reads the same. The default (../../libc370) is unchanged; anyone passing CRENT= explicitly now falls back to that default, which fails loudly with 'Undefined operation code ... PDPPRLG' rather than silently, if their checkout lives elsewhere. Verified all three paths: default, LIBC370= override, and a bad path. Claude-Session: https://claude.ai/code/session_018SbqQkXrAyDhVgcF8X8cdz --- .github/workflows/build.yml | 4 ++-- as370/tests/listref/README.md | 2 +- as370/tests/listref/check.sh | 6 +++--- as370/tests/run.sh | 9 +++++---- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2f7ccd5..7654f1e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -45,8 +45,8 @@ jobs: with: path: cc370 - # as370/tests/run.sh looks for CRENT=../../libc370, i.e. a sibling of the - # repo root -- the same layout the ecosystem uses locally. + # The suites default to LIBC370=../../libc370, i.e. a sibling of the repo + # root -- the same layout the ecosystem uses locally. - name: Clone libc370 (macro libraries only) working-directory: . run: git clone --depth 1 https://github.com/mvslovers/libc370.git libc370 diff --git a/as370/tests/listref/README.md b/as370/tests/listref/README.md index fe659df..09839d4 100644 --- a/as370/tests/listref/README.md +++ b/as370/tests/listref/README.md @@ -16,7 +16,7 @@ through) and **RELOCATION DICTIONARY** sections against this reference, byte-for-byte: ```sh -as/tests/listref/check.sh # CRENT=../../libc370 by default +as/tests/listref/check.sh # LIBC370=../../libc370 by default ``` Two differences from the IFOX reference are expected and tolerated: diff --git a/as370/tests/listref/check.sh b/as370/tests/listref/check.sh index a0a1d71..6539f00 100755 --- a/as370/tests/listref/check.sh +++ b/as370/tests/listref/check.sh @@ -11,15 +11,15 @@ cd "$(dirname "$0")/../.." || exit 2 # Macro-library root (maclib + sysmac). These live in libc370 now; the default # used to point at crent370, the frozen v1.x libc, which no longer needs to be -# checked out. Override with CRENT=... . -CRENT=${CRENT:-../../libc370} +# checked out. Override with LIBC370=/path . +LIBC370=${LIBC370:-../../libc370} fail=0 # --- case 1: tstlist -- general listing (ESD + SOURCE + RLD) ---------------- REF=tests/listref/ifox-listing-tstlist.txt OUT=/tmp/as370-listref.$$ ASMDATE=06/18/26 ASMTIME=06.42 ./as370 tests/listref/tstlist.s \ - -I "$CRENT/maclib" -I "$CRENT/sysmac" -a="$OUT" >/dev/null 2>&1 \ + -I "$LIBC370/maclib" -I "$LIBC370/sysmac" -a="$OUT" >/dev/null 2>&1 \ || { echo "listref tstlist: ASSEMBLE FAILED"; fail=1; } python3 - "$REF" "$OUT" <<'PY' import sys diff --git a/as370/tests/run.sh b/as370/tests/run.sh index 0a5c521..c0d51db 100755 --- a/as370/tests/run.sh +++ b/as370/tests/run.sh @@ -5,12 +5,13 @@ cd "$(dirname "$0")/.." || exit 2 # Macro libraries: maclib (the PDP macros -- PDPTOP/PDPPRLG/PDPEPIL) and sysmac # (host-only mirror of the SYS1.MACLIB members the build needs: SAVE/RETURN/ -# IHBERMAC, SVC macros). These now live in libc370; the default used to point at +# IHBERMAC, SVC macros). These live in libc370; the default used to point at # crent370, the frozen v1.x libc, which no longer needs to be checked out -- so # the suite failed with "Undefined operation code ... PDPPRLG" wherever it was -# absent. Override the repo root with CRENT=... . -CRENT=${CRENT:-../../libc370} -MACLIB="-I $CRENT/maclib -I $CRENT/sysmac" +# absent. Override the checkout with LIBC370=/path (same name the Makefile and +# tests/corpus/check.sh already use). +LIBC370=${LIBC370:-../../libc370} +MACLIB="-I $LIBC370/maclib -I $LIBC370/sysmac" # sample8 (tinitvl, WTO) and sample9 (irxtmpw, XCTL->IHBINNRB) are real rexx370 # modules that exercise the hardest macro paths — they guard against regressing # the byte-exact REXX corpus when changing the assembler for other projects.