CocoGm2 - CRS.mod Analysis and Fix =================================== Summary ------- The file CRS.mod (a Modula-2 scanner generated by Coco/R) failed to compile with the GNU Modula-2 compiler (gm2). The compilation command was: gm2 -fiso -c CRS.mod The compiler crashed with an internal compiler error (ICE) instead of reporting a normal error/warning. This document records the analysis and the fixes applied. ---------------------------------------------------------------------- 1. Original symptoms ---------------------------------------------------------------------- The raw compile of CRS.mod produced: cc1gm2: internal compiler error: ProcedureSym kind has not yet been declared ...SymbolTable_GetProcedureKind ...P2SymBuild_BuildFPSection ...FormalParameters -> ... -> ProcedureHeading -> ... -> Declaration FileIO.mod compiled fine with the same flags, so the problem was specific to CRS.mod, not to the toolchain in general. ---------------------------------------------------------------------- 2. Initial findings during review ---------------------------------------------------------------------- Several suspicious items were noted during the code review: (a) Forward reference GetString() called the function CharAt() which was declared LATER in the file. In Modula-2 a procedure must be declared before it is used. (b) Unused local ORDL wrapper A module-level procedure `ORDL(n: INT32): CARDINAL` wrapped FileIO.ORDL(), but the rest of the code called FileIO.ORDL() directly, so the wrapper was dead code. (c) Nested procedures inside Get() Get() contained two nested procedures: - Equal(s: ARRAY OF CHAR): BOOLEAN - CheckLiteral Nested procedures with open-array (ARRAY OF CHAR) parameters were a suspected compiler-bug trigger. (d) CurrentCh procedure-type variable CurrentCh was declared as type GetCH = PROCEDURE (INT32): CHAR and was (re)assigned to CharAt() in the module initialization block. ---------------------------------------------------------------------- 3. Isolation (minimization) ---------------------------------------------------------------------- The ICE was reproduced and reduced to a minimal case: IMPLEMENTATION MODULE CRS2; IMPORT FileIO; ... PROCEDURE ORDL (n: INT32): CARDINAL; BEGIN RETURN FileIO.ORDL(n) END ORDL; ... Results of the reductions: - Removing Err() -> still crashed - Removing NextCh() -> still crashed - Keeping ONLY the ORDL procedure -> STILL crashed - Renaming "ORDL" to "ORDLX" -> compiled cleanly CONCLUSION: the crash was caused by DECLARING a module-level procedure with the SAME NAME as an imported procedure from FileIO (i.e. `ORDL` shadowing `FileIO.ORDL`). gm2 trips over the name clash and hits "ProcedureSym kind has not yet been declared" while building the formal parameter section of surrounding/affected procedures. The earlier hypothesis (nested procedures, procedure-type variables, open-array parameters) was explored but is NOT the cause of the ICE; those constructs compiled fine in isolation. ---------------------------------------------------------------------- 4. Fixes applied to CRS.mod ---------------------------------------------------------------------- (1) Removed the unused module-level `ORDL` wrapper procedure. This is the actual fix for the compiler crash. All remaining references already use the fully-qualified FileIO.ORDL(), which is correct. (2) Restored `CurrentCh: GetCH;` in the VAR section (it had been lost from the file during earlier save/stash actions). CurrentCh is used by NextCh(), Equal() and CheckLiteral(). (3) Moved CharAt() and CapChAt() BEFORE GetString()/GetName() to resolve the forward reference of CharAt() from GetString(). (4) Extracted the nested procedures Equal() and CheckLiteral() out of Get() to module level for clarity and to keep away from gm2's nested-procedure handling. CheckLiteral now takes sym as a VAR parameter and the call site was updated to CheckLiteral(sym). ---------------------------------------------------------------------- 5. Verification ---------------------------------------------------------------------- After the fixes the module compiles cleanly: gm2 -fiso -c CRS.mod (no errors, no warnings) Note: the git diff for CRS.mod appears large because the original file used CRLF line endings while the edited file uses LF; the real functional change is the four items listed in section 4. ---------------------------------------------------------------------- 6. Whole-project build (./build.sh) ---------------------------------------------------------------------- After CRS.mod compiled, ./build.sh was run. Two further problems were found and fixed. Both were exposed only during the final link step, in which gm2 links with the scaffold flags "-fscaffold-dynamic -fscaffold-main" and re-checks the imported implementation modules. 6.1 gm2 false "s already declared" error in CRA.mod (MatchDFA) During the link step gm2 reported: cc1gm2: error: In procedure « MatchDFA »: symbol « s » is already declared in this scope, use a different name or remove the declaration ./CRA.mod:659:7: error: symbol « s » also declared in this module 659 | s, to: INTEGER (* State *); Analysis: - CRA.mod compiles fine standalone (gm2 -fiso -c CRA.mod). - CR.mod compiles fine standalone (gm2 -fiso -c CR.mod). - The error appears ONLY when compiling CR.mod with the scaffold flags (as gm2 does during linking): gm2 -fiso -fscaffold-dynamic -fscaffold-main -c CR.mod - The scaffolding re-parses the imported implementation module CRA.mod, and a procedure-local variable "s" from some earlier scope leaks into MatchDFA's scope, so the local declaration of "s" is flagged as a duplicate. This is a gm2 scope-tracking bug, not a real error in the source. - Renaming the local "s" in procedures such as TheState() did NOT help; renaming the "s" inside MatchDFA() itself does. Fix applied to CRA.mod: Renamed MatchDFA()'s local variable "s" to "st" (declaration and all 7 uses inside the procedure). This avoids the collision with the leaked symbol while keeping the code identical in behaviour. 6.2 build.sh errors in the module list Once compilation and linking succeeded past the CRA error, the link failed twice more because build.sh referenced module object files that were either never built or not needed: - "CRQ.o" was in the link line but is never compiled. CRQ.mod is a legacy alternate MAIN module (not CR); CR.mod does not import CRQ. -> removed CRQ.o from the link line. - "CRX.o" was built by the script but was missing from the link line, yet CR.mod calls CRX.GenCompiler and CRX.WriteStatistics. -> added CRX.o to the link line. Final link command in build.sh: gm2 -fiso -o CR CRA.o CRC.o CRG.o CRP.o CRS.o CRT.o CRX.o \ FileIO.o Sets.o CR.mod ---------------------------------------------------------------------- 7. Final verification ---------------------------------------------------------------------- ./build.sh Building Coc/R with GNU Modula-2 Deleting all o files compiling the needed modules +++++++Modules compiled ++++++++++++++++++++++ Compiling main module and linking all together (exit code 0) Produces the executable ./CR. Smoke test: echo "?" | ./CR Coco/R (WinTel) - Compiler-Compiler V1.53 Released by Pat Terry 17 September 2002 (COCOR ? gives short help screen) Grammar[.atg] ? : File not found. Grammar[.atg] ? : The executable starts and behaves as expected. ---------------------------------------------------------------------- Files changed (this session) ---------------------------------------------------------------------- CRS.mod - removed unused ORDL wrapper; restored CurrentCh; moved CharAt/CapChAt before GetString/GetName; extracted Equal/CheckLiteral to module level. CRA.mod - renamed MatchDFA local variable s -> st. build.sh - fixed module list for the link step (drop CRQ.o, add CRX.o). fix.txt - this file. ---------------------------------------------------------------------- 8. IsoFrames adapted to FileIO (ISO build works fully, 2026) ---------------------------------------------------------------------- Goal: make the ISO-flavoured frames (IsoFrames/) regenerate CRS, CRP and the driver such that the whole build compiles and LINKS with `gm2 -fiso`. Why it broke before: - iso scanner frame emitted `src, lst: IOChan.ChanId` in its DEFINITION MODULE, but the support modules CRA/CRT/CRX call `FileIO.WriteString (CRS.lst, ...)`, which requires `CRS.lst: FileIO.File`. The types are incompatible (FileIO.File is a POINTER TO FileRec wrapping its own hidden ChanId). - iso parser frame emitted only `IMPORT ;`; the grammar's semantic actions (CR.atg) use FileIO.INT32/Long0/Long2/INTL/INT, so the generated parser missed the FileIO import. - iso compiler/compile2/gpm frames used `Storage.ALLOCATE(nextErr, SYSTEM.TSIZE(ErrDesc))` INSIDE a NESTED module (ListHandler). In ISO mode gm2 rejects that: "Storage looks like a module which has not been globally imported". The working pattern (proved with testS3.mod) is to import inside the nested module: FROM Storage IMPORT ALLOCATE; FROM SYSTEM IMPORT TSIZE; and then call ALLOCATE(p, TSIZE(T)) unqualified. Changes to IsoFrames: - scanner.frm / scanner2.frm / scannerc.frm * DEFINITION: `IMPORT FileIO;` + `INT32 = FileIO.INT32;` + `src, lst: FileIO.File;` * IMPLEMENTATION: `IMPORT FileIO, Storage;` (scannerc also keeps SYSTEM for TSIZE); Reset reads via FileIO.ReadBytes instead of IOChan.RawRead: read := BlkSize; FileIO.ReadBytes(src, buf[i]^, read); * scanner2 uses FileIO.err + FileIO.ReadBytes in its single-buffer Reset. - parser.frm: `IMPORT -->scanner;` -> `IMPORT -->scanner, FileIO;` so the generated parser gets FileIO as well as the scanner's range (matches the classic frame behaviour `IMPORT FileIO, CRS;`). - compiler.frm / compile2.frm / compiler.gpm (rudimentary drivers): * ListHandler nested module now does FROM FileIO IMPORT CR, LF, EOF, WriteString, Write, WriteLn, WriteInt, Long0; FROM Storage IMPORT ALLOCATE; FROM SYSTEM IMPORT TSIZE; and calls ALLOCATE/TSIZE unqualified; the old CONST CR/LF/EOF have been removed (they come from FileIO now). * main module switched from ProgramArgs/StdChans/SeqFile/TextIO/ WholeIO to FileIO: FileIO.NextParameter, FileIO.Open(src,...,FALSE), FileIO.Open(lst,...,TRUE), FileIO.StdOut/FileIO.err, FileIO.WriteString/WriteInt/WriteLn, FileIO.Close. NOTE: FileIO.NextParameter is a PROCEDURE with no return value, so it must NOT be used in `IF FileIO.NextParameter(...)`; pass a local buffer and test `IF buf[0] = 0C`. Critical gotcha found while linking: - gm2 fails during whole-program checking (pass 3) with CRP.mod: error: In procedure : too many errors in pass 3 whenever the generated parser contains ACTIVE `...FORWARD;` declarations. The classic build avoids this by generating the parser with the multipass option: CR -m CR.atg which makes GenForwardRefs (CRX.mod:539) wrap the FORWARD block in (* ----- FORWARD not needed in multipass compilers ... ----- *) - Symptom was independent of directory/timestamps; only commenting the FORWARD block out fixed it. Verified full ISO build (fresh dir, gm2 -fiso): CRFRAMES=/IsoFrames /CR -m CR.atg # compiles: CRA CRC CRG CRT CRX CRS CRP FileIO Sets -> all EXIT 0 gm2 -fiso -o coco CRA.o CRC.o CRG.o CRT.o CRX.o CRS.o CRP.o \ FileIO.o Sets.o CR.mod # link OK; ./coco CR.atg -> "Parsed correctly" Support modules CRA/CRT/CRX remained unchanged (FileIO already). ---------------------------------------------------------------------- 9. Pascal build from pascal.atg + gm2 whole-program link bug (2026) ---------------------------------------------------------------------- Goal: generate a Pascal compiler from pascal.atg using the same FileIO-based frames and link it with gm2 -fiso. Generation command (CRFRAMES points to directory containing the frames): CRFRAMES= CR -m -C pascal.atg - -m suppresses FORWARD declarations (multipass-safe code) - -C generates the driver (Pascal.mod from compiler.frm) Without -C, only PascalS.mod and PascalP.mod are generated; Pascal.mod is NOT written (gated on CRT.ddt["C"] at CR.mod:463-466). The grammar is intentionally not LL(1) so CR always ends with "Compilation ended with LL(1) errors." — this is expected and harmless; PascalS/PascalP/Pascal are still written. Compilation: for m in FileIO PascalS PascalP Pascal; do gm2 -fiso -c $m.mod done All four modules compile cleanly (EXIT 0). Link problem: gm2 -fiso -o Pascal PascalS.o PascalP.o FileIO.o Pascal.mod fails with: RndFile.def:27: error: In definition module RndFile: the constructor type is FlagSet and this is different from the constant constant set which has a type FlagSet (same for TermFile.def:31, ChanConsts.def:33) Root cause: gm2's default link step performs whole-program pass-3 analysis: it re-parses ALL imported definition modules (including its own bundled m2iso: RndFile.def, TermFile.def, ChanConsts.def) and type-checks every constant set-constructor. When the total number of identifiers/symbols in the program exceeds an internal threshold, a buffer overflow corrupts the type of FlagSet inside gm2 itself, producing the spurious "constructor type is FlagSet" error on the m2iso library's own code. This is the same class of bug reported on the gm2 mailing list (2023-05, msg00039/msg00045, by Michael Riedl, with workaround explained by Gaius Mulley). Trigger is symbol-count threshold, not a specific procedure: adding any sufficiently large dummy procedure to a working module causes the same failure; removing any ~50 identifiers from the failing module drops below threshold and restores success. Workaround — two-step link using module list: gm2 -fiso -fgen-module-list=modules.lst \ -o /dev/null PascalS.o PascalP.o FileIO.o Pascal.mod gm2 -fiso -fuse-list=modules.lst \ -o Pascal PascalS.o PascalP.o FileIO.o Pascal.mod The first invocation generates the module list (despite printing the RndFile error; the list is still written and the error is non-fatal). The second uses the cached list and AVOIDS the whole-program re-analysis entirely — link succeeds, binary works. Verified full end-to-end build (fresh dir): cp /Test/{FileIO.mod,FileIO.def,compiler.frm,parser.frm, scanner.frm,compiler.gpm,pascal.atg} / CRFRAMES=. CR -m -C pascal.atg for m in FileIO PascalS PascalP Pascal; do gm2 -fiso -c $m.mod; done gm2 -fiso -fgen-module-list=modules.lst -o /dev/null \ PascalS.o PascalP.o FileIO.o Pascal.mod gm2 -fiso -fuse-list=modules.lst -o Pascal \ PascalS.o PascalP.o FileIO.o Pascal.mod # ./Pascal /tmp/test.pas -> "Parsing Parsed correctly" (exit 0)