From 56edd8904c318e324a1b29a0dac7d8fe324e8707 Mon Sep 17 00:00:00 2001 From: jackwthake Date: Thu, 23 Jul 2026 17:04:38 -0700 Subject: [PATCH 1/3] Fixed CG-12: Globals hog ZP slots that they never use --- CHANGELOG.md | 4 +++ c02-as/src/generator.c | 8 ++++-- docs/Deviations/FINDINGS_codegen.md | 8 +++++- test/codegen/globals_not_pushed_to_stack.c02 | 25 +++++++++++++++++++ .../golden/globals_not_pushed_to_stack.golden | 0 5 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 test/codegen/globals_not_pushed_to_stack.c02 create mode 100644 test/codegen/golden/globals_not_pushed_to_stack.golden diff --git a/CHANGELOG.md b/CHANGELOG.md index 8999db0..1daa0c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/) - while the project is in `0.x`, breaking changes may land in MINOR releases; PATCH releases are reserved for bug fixes only. +## [Unreleased] + +- **Fixed CG-12:** *Globals exhausting ZP slots they never occupied* Added early out in ZP building if the operand is both a variable and a global. + ## [1.7.1] 2026-07-24 ### Fixed: diff --git a/c02-as/src/generator.c b/c02-as/src/generator.c index 2377045..50f12d8 100644 --- a/c02-as/src/generator.c +++ b/c02-as/src/generator.c @@ -149,12 +149,16 @@ static int zp_map_add(emitter_t *e, zp_map_t *map, tac_operand_kind_t kind, return 1; } +// fwd decl for early return path in zp addition +static global_entry_t *lookup_global(emitter_t *e, char *name); // Register a TAC operand in the ZP map (dispatches var vs temp). static int zp_map_add_operand(emitter_t *e, zp_map_t *map, tac_operand_t *op) { - if (op->kind == OPERAND_VAR) + if (op->kind == OPERAND_VAR) { + if (lookup_global(e, op->name)) return 1; + return zp_map_add(e, map, OPERAND_VAR, op->name, 0, op->type); - if (op->kind == OPERAND_TEMP) + } if (op->kind == OPERAND_TEMP) return zp_map_add(e, map, OPERAND_TEMP, NULL, op->temp_id, op->type); return 1; } diff --git a/docs/Deviations/FINDINGS_codegen.md b/docs/Deviations/FINDINGS_codegen.md index bff51ef..85caf43 100644 --- a/docs/Deviations/FINDINGS_codegen.md +++ b/docs/Deviations/FINDINGS_codegen.md @@ -40,7 +40,7 @@ Severity: **T** over-strict rejection · **P0** silent miscompile · | [CG-9](#cg-9) | P2 | Source | ✅ Fixed | `allocate_globals` never checks RAM_TOP — globals silently run past $3FFF | none — undocumented architecture | | [CG-10](#cg-10) | P2 | Source | — | Interrupt handlers don't save the ABI zone, helper slots, or RET | none — undocumented architecture | | [CG-11](#cg-11) | P2 | Source | ✅ Fixed upstream | Struct field offsets >255 wrap in `LDY #imm` on the pointer path | none — undocumented architecture | -| [CG-12](#cg-12) | P2 | Source | — | Every referenced global gets a dead ZP slot per function — wasted ZP, earlier exhaustion | none — undocumented architecture | +| [CG-12](#cg-12) | P2 | Source | ✅ Fixed | Every referenced global gets a dead ZP slot per function — wasted ZP, earlier exhaustion | none — undocumented architecture | | [CG-13](#cg-13) | P2 | Source | — | Variable shift counts read only the low byte | none — undocumented architecture | ## Entries @@ -381,6 +381,12 @@ output; a real capacity/perf trap. **Verified:** Source. +**Resolved:** Added check in `zp_map_add_operand` for variable operands to +first lookup if the operand is a global, if it is it early returns before adding +to ZP map. Cheap fix because evervy load/store/etc goes through functions that +already check globals before ZP. The issue lied in the building of the ZP map +for each function body, not specifically in the pushing a stack frame to the stack. + ### CG-13: Variable shift counts read only the low byte `TAC_SHL/TAC_SHR` with a non-constant count do `emit_load_byte(src2, 0)` into diff --git a/test/codegen/globals_not_pushed_to_stack.c02 b/test/codegen/globals_not_pushed_to_stack.c02 new file mode 100644 index 0000000..2212cfd --- /dev/null +++ b/test/codegen/globals_not_pushed_to_stack.c02 @@ -0,0 +1,25 @@ +struct S0 { u16 a; u16 b; } +struct S1 { S0 a; S0 b; } +struct S2 { S1 a; S1 b; } +struct S3 { S2 a; S2 b; } +struct S4 { S3 a; S3 b; } +struct P16 { u8 a0; u8 a1; u8 a2; u8 a3; u8 a4; u8 a5; u8 a6; u8 a7; u8 a8; u8 a9; u8 a10; u8 a11; u8 a12; u8 a13; u8 a14; u8 a15; } +struct P15 { u8 a0; u8 a1; u8 a2; u8 a3; u8 a4; u8 a5; u8 a6; u8 a7; u8 a8; u8 a9; u8 a10; u8 a11; u8 a12; u8 a13; u8 a14; } +struct P127 { P16 a; P16 b; P16 c; P16 d; P16 e; P16 f; P15 g; } +struct S5 { S4 a; S4 b; u16 x; u16 y; u16 z; P127 padding; S0 q; S0 r; u16 s; } + +S5 fatty1; +S5 fatty2; + +fn main() -> void { + fatty1.s = 69; + fatty2.s = fatty1.s; + + // will cause any locals to be pushed to the stack, per old bug all used globals will also be pushed. + second(); +} + + +fn second() -> void { + fatty2.x = 420; +} diff --git a/test/codegen/golden/globals_not_pushed_to_stack.golden b/test/codegen/golden/globals_not_pushed_to_stack.golden new file mode 100644 index 0000000..e69de29 From 36c7b617211403eea9fc362894b472a1b00478fc Mon Sep 17 00:00:00 2001 From: jackwthake Date: Thu, 23 Jul 2026 17:34:58 -0700 Subject: [PATCH 2/3] Fixed FE-9: over-strictness in operator parsing --- CHANGELOG.md | 1 + c02-frontend/src/C02/Parser/Lexer.hs | 27 +++++++++++++++----- c02-frontend/src/C02/Parser/Parser.hs | 2 -- docs/Deviations/FINDINGS_frontend.md | 7 +++-- test/parser/adjacent_operators.c02 | 12 +++++++++ test/parser/golden/adjacent_operators.golden | 1 + 6 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 test/parser/adjacent_operators.c02 create mode 100644 test/parser/golden/adjacent_operators.golden diff --git a/CHANGELOG.md b/CHANGELOG.md index 1daa0c2..f5a97be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ releases are reserved for bug fixes only. ## [Unreleased] +- **Fixed FE-9:** *Adjacent operators produce syntax errors* relaxed strictness of the operator parser to accomodate cramped code. - **Fixed CG-12:** *Globals exhausting ZP slots they never occupied* Added early out in ZP building if the operand is both a variable and a global. ## [1.7.1] 2026-07-24 diff --git a/c02-frontend/src/C02/Parser/Lexer.hs b/c02-frontend/src/C02/Parser/Lexer.hs index 6518003..7229048 100644 --- a/c02-frontend/src/C02/Parser/Lexer.hs +++ b/c02-frontend/src/C02/Parser/Lexer.hs @@ -119,10 +119,25 @@ charLiteralParser :: Parser Int charLiteralParser = ord <$> between (char '\'') (char '\'') L.charLiteral -- Match an operator token, enforcing maximal munch: the same trick 'keyword' --- uses, but guarding against operator chars instead of identifier chars. This is --- what stops operator "<" from matching the '<' in "<<" or "<=". The inner 'try' --- keeps a partial match (e.g. "<" of an expected "<=") non-consuming so --- makeExprParser can fall through to another operator. +-- uses, but guarding only against multi-character operators that start with the +-- current operator. This prevents "<" from matching the '<' in "<<" or "<=", +-- but allows "a*-b" (multiply then negate) since *- is not a recognized token. +-- The inner 'try' keeps a partial match non-consuming so makeExprParser can +-- fall through to another operator. operator :: String -> Parser String -operator name = lexeme (try (string name <* notFollowedBy (oneOf opChars))) - where opChars = "+-*/%<>=!&|^~" :: [Char] +operator name = lexeme (try (string name <* notFollowedBy (validContinuation name))) + where + validContinuation :: String -> Parser String + validContinuation op = case op of + "+" -> choice [string "+", string "="] + "-" -> choice [string "-", string "="] + "*" -> string "=" + "/" -> string "=" + "%" -> string "=" + "<" -> choice [string "<", string "="] + ">" -> choice [string ">", string "="] + "=" -> string "=" + "!" -> string "=" + "&" -> string "&" + "|" -> string "|" + _ -> empty diff --git a/c02-frontend/src/C02/Parser/Parser.hs b/c02-frontend/src/C02/Parser/Parser.hs index 4969475..5a5a21d 100644 --- a/c02-frontend/src/C02/Parser/Parser.hs +++ b/c02-frontend/src/C02/Parser/Parser.hs @@ -1,5 +1,3 @@ -{-# OPTIONS_GHC -Wno-unused-top-binds #-} - -- | The C02 grammar: turns source text into a "C02.AST" 'Program'. Built on the -- token helpers in "C02.Lexer" and megaparsec's expression-combinator support. -- 'parseProgram' is the stage entry point the driver (and later stages) call. diff --git a/docs/Deviations/FINDINGS_frontend.md b/docs/Deviations/FINDINGS_frontend.md index ea31a76..55a7590 100644 --- a/docs/Deviations/FINDINGS_frontend.md +++ b/docs/Deviations/FINDINGS_frontend.md @@ -37,11 +37,11 @@ Severity legend (same as `DEVIATIONS_hs_impl.md`): | [FE-6](#fe-6) | G | Source | — | Included declarations are hoisted above the includer's own, erasing include position | none — undocumented architecture | | [FE-7](#fe-7) | T | Executed | — | No `asm { }` statement exists in the grammar | [§5.7](../SPEC.md#57-inline-assembly-asm) | | [FE-8](#fe-8) | G | Executed | — | Char literals: an undocumented extension, not a lexeme (`'a' + 1` fails), Haskell escape rules | [§1.3](../SPEC.md#13-integer-literals) | -| [FE-9](#fe-9) | G | Executed | — | Adjacent operators fail to lex: `a*-b`, `a<-1`, `a&&&b` are parse errors | [§1.6](../SPEC.md#16-operators--punctuation) | +| [FE-9](#fe-9) | G | Executed | ✅ Fixed | Adjacent operators fail to lex: `a*-b`, `a<-1`, `a&&&b` are parse errors | [§1.6](../SPEC.md#16-operators--punctuation) | | [FE-10](#fe-10) | G | Executed | — | Integer literals overflowing the host word silently wrap (`2^64+1` lexes as `1`) | [§1.3](../SPEC.md#13-integer-literals) | | [FE-11](#fe-11) | T | Executed | — | Compound assignment doesn't type-check as its desugaring: `p += 1` rejected on a pointer | [§5.3](../SPEC.md#53-assignment) | | [FE-12](#fe-12) | P2 | Executed | — | S-1/S-17 laxity reproduced: any `(void*)`-shaped actual is compatible with *everything* | [§3.2](../SPEC.md#32-type-compatibility) | -| [FE-13](#fe-13) | P2 | Executed | — | `&&`/`||` operands are entirely unchecked (struct `&&` struct accepted) | [§6.3](../SPEC.md#63-binary-operators) | +| [FE-13](#fe-13) | P2 | Executed | — | `&&`/`\|\|` operands are entirely unchecked (struct `&&` struct accepted) | [§6.3](../SPEC.md#63-binary-operators) | | [FE-14](#fe-14) | P0 | Executed | ✅ Fixed | `interrupt` validation missing: no warning, flag not cleared → calling the function crashes at runtime | [§4.2](../SPEC.md#42-interrupt-functions) | | [FE-15](#fe-15) | G | Executed | — | `for`-increment clause accepts a variable declaration | [§5.5](../SPEC.md#55-for-loop-clauses) | | [FE-16](#fe-16) | P2 | Source | — | Statement-position structs get no redeclaration check at all | [§7.2](../SPEC.md#72-scope-stack--shadowing) | @@ -257,6 +257,9 @@ guard.) **Verified:** Executed (`a*-b`). +**Resolved:** Refactored `operator` parser to build out characters char by char and not +be hardcoded to expect whitespace after the operator. + ### FE-10: Host-word overflow in integer literals wraps silently ```c diff --git a/test/parser/adjacent_operators.c02 b/test/parser/adjacent_operators.c02 new file mode 100644 index 0000000..e8c5339 --- /dev/null +++ b/test/parser/adjacent_operators.c02 @@ -0,0 +1,12 @@ +fn main () -> void { + i8 a = (i8)7; + i8 b = (i8)10; + i8 x = 0; + + i8 c = a*-b; + i8 d = a&~b; + + if (x<-1) { + a+=c; + } +} \ No newline at end of file diff --git a/test/parser/golden/adjacent_operators.golden b/test/parser/golden/adjacent_operators.golden new file mode 100644 index 0000000..1ecd93a --- /dev/null +++ b/test/parser/golden/adjacent_operators.golden @@ -0,0 +1 @@ +Program [] [FunctionDecl (FuncDecl {funcName = "main", funcReturnType = Void, funcReturnPtrDepth = 0, isInterrupt = False, params = [], body = Just (Nested [LocVarDecl (VarDecl {varType = I8, ptrDepth = 0, declName = "a", declInit = Just (Cast I8 0 (IntLit 7))}),LocVarDecl (VarDecl {varType = I8, ptrDepth = 0, declName = "b", declInit = Just (Cast I8 0 (IntLit 10))}),LocVarDecl (VarDecl {varType = I8, ptrDepth = 0, declName = "x", declInit = Just (IntLit 0)}),LocVarDecl (VarDecl {varType = I8, ptrDepth = 0, declName = "c", declInit = Just (Binary Mul (Var "a") (Unary Negate (Var "b")))}),LocVarDecl (VarDecl {varType = I8, ptrDepth = 0, declName = "d", declInit = Just (Binary BitAnd (Var "a") (Unary BitNot (Var "b")))}),If [(Just (Binary Lt (Var "x") (Unary Negate (IntLit 1))),[Assign (Var "a") PlusEquals (Var "c")])]])})] From 83af1411262e1442610ab2bc2de1ca6f8d0f9c8c Mon Sep 17 00:00:00 2001 From: jackwthake Date: Thu, 23 Jul 2026 18:06:31 -0700 Subject: [PATCH 3/3] Fixed FE-5: lazy file read errors on bad encoding --- CHANGELOG.md | 3 +- c02-frontend/app/Main.hs | 77 +++++++++++++++++--------- c02-frontend/c02-frontend.cabal | 1 + docs/Deviations/FINDINGS_frontend.md | 6 +- scripts/c02c.py | 11 ++-- test/parser/bad_encoding.c02 | 1 + test/parser/golden/bad_encoding.golden | 1 + 7 files changed, 67 insertions(+), 33 deletions(-) create mode 100644 test/parser/bad_encoding.c02 create mode 100644 test/parser/golden/bad_encoding.golden diff --git a/CHANGELOG.md b/CHANGELOG.md index f5a97be..2d651cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ releases are reserved for bug fixes only. ## [Unreleased] -- **Fixed FE-9:** *Adjacent operators produce syntax errors* relaxed strictness of the operator parser to accomodate cramped code. +- **Fixed FE-5:** *Lazy file read doesn't catch bad file encoding* Now forces UTF-8 file encoding +- **Fixed FE-9:** *Adjacent operators produce syntax errors* Relaxed strictness of the operator parser to accomodate cramped code. - **Fixed CG-12:** *Globals exhausting ZP slots they never occupied* Added early out in ZP building if the operand is both a variable and a global. ## [1.7.1] 2026-07-24 diff --git a/c02-frontend/app/Main.hs b/c02-frontend/app/Main.hs index 066bc03..7537e9d 100644 --- a/c02-frontend/app/Main.hs +++ b/c02-frontend/app/Main.hs @@ -37,6 +37,26 @@ import C02.Parser.AST (Program, Pos(..)) import C02.Lowering.Serialize (serializeModule) import qualified Data.ByteString.Lazy as B +import qualified Data.ByteString as BS +import qualified Data.Text as T +import qualified Data.Text.Encoding as TE + +-- | Reads a file, validates UTF-8, and returns a String or an error message. +readUtf8Safe :: FilePath -> IO (Either String String) +readUtf8Safe path = do + -- 1. Read the file safely as raw bytes + bytes <- BS.readFile path + + -- 2. Attempt to decode the bytes into Text + case TE.decodeUtf8' bytes of + -- Left indicates a UnicodeException (decoding failed) + Left err -> + return (Left ("Invalid UTF-8 encoding: " ++ show err)) + + -- Right indicates success; convert Data.Text to a standard String + Right text -> + return (Right (T.unpack text)) + writeOutput :: Program -> String -> IO () writeOutput p fp = @@ -55,33 +75,36 @@ main = do (outPath, files) = extractOutput rest case files of [path] -> do - src <- readFile path - case parseProgram path src of - Left err -> hPutStr stderr (errorBundlePretty err) >> exitFailure - Right prog - -- --dump-ast is an orthogonal reporting flag: it prints the AST to - -- stdout at whatever point we stop, so both test stages can diff a - -- text golden instead of a binary .o. --parse-only stops after parsing - -- (parser stage: no analysis, since its fixtures aren't whole programs); - -- otherwise analysis runs and a clean pass emits the IR object. - | parseOnly -> if dumpAST then print prog else pure () - | otherwise -> do - resolved <- resolveIncludes path includeDirs prog - case resolved of - Left err -> hPutStr stderr (errorBundlePretty err) >> exitFailure - Right (resolvedProg, srcs) -> - -- The root file's source isn't in @srcs@ (the resolver only - -- records files it read); add it so the renderer can show root - -- diagnostics too. - let diags = analyze resolvedProg - hasErr = any ((== Error) . severityOf . diagWhat) diags - in do - if null diags - then pure () - else hPutStr stderr (renderDiags path (Map.insert path src srcs) diags) - if hasErr - then exitFailure - else if dumpAST then print resolvedProg else writeOutput resolvedProg outPath + s <- readUtf8Safe path + case s of + Left err -> hPutStrLn stderr err >> exitFailure + Right src -> do + case parseProgram path src of + Left err -> hPutStr stderr (errorBundlePretty err) >> exitFailure + Right prog + -- --dump-ast is an orthogonal reporting flag: it prints the AST to + -- stdout at whatever point we stop, so both test stages can diff a + -- text golden instead of a binary .o. --parse-only stops after parsing + -- (parser stage: no analysis, since its fixtures aren't whole programs); + -- otherwise analysis runs and a clean pass emits the IR object. + | parseOnly -> if dumpAST then print prog else pure () + | otherwise -> do + resolved <- resolveIncludes path includeDirs prog + case resolved of + Left err -> hPutStr stderr (errorBundlePretty err) >> exitFailure + Right (resolvedProg, srcs) -> + -- The root file's source isn't in @srcs@ (the resolver only + -- records files it read); add it so the renderer can show root + -- diagnostics too. + let diags = analyze resolvedProg + hasErr = any ((== Error) . severityOf . diagWhat) diags + in do + if null diags + then pure () + else hPutStr stderr (renderDiags path (Map.insert path src srcs) diags) + if hasErr + then exitFailure + else if dumpAST then print resolvedProg else writeOutput resolvedProg outPath _ -> hPutStrLn stderr "usage: c02-frontend [--parse-only] [-I ]... [-o ] " >> exitFailure diff --git a/c02-frontend/c02-frontend.cabal b/c02-frontend/c02-frontend.cabal index 12fad88..4791bfa 100644 --- a/c02-frontend/c02-frontend.cabal +++ b/c02-frontend/c02-frontend.cabal @@ -94,6 +94,7 @@ executable c02-frontend megaparsec, containers, bytestring, + text, c02-frontend -- Driver lives in its own dir so it doesn't re-compile the library's diff --git a/docs/Deviations/FINDINGS_frontend.md b/docs/Deviations/FINDINGS_frontend.md index 55a7590..789aba5 100644 --- a/docs/Deviations/FINDINGS_frontend.md +++ b/docs/Deviations/FINDINGS_frontend.md @@ -33,7 +33,7 @@ Severity legend (same as `DEVIATIONS_hs_impl.md`): | [FE-2](#fe-2) | P1 | Executed | ✅ Fixed | Root file is never in the include visited-set: self-include / root↔header cycles duplicate the root's declarations | none — undocumented architecture | | [FE-3](#fe-3) | P2 | Executed | — | No path canonicalization: the same header via two spellings is included twice | none — undocumented architecture | | [FE-4](#fe-4) | G | Executed | — | include-not-found diagnostic loses the include site's position | none — undocumented architecture | -| [FE-5](#fe-5) | P2 | Source | — | Lazy `readFile` lets late I/O errors escape the resolver's `try` and crash the driver | none — undocumented architecture | +| [FE-5](#fe-5) | P2 | Source | ✅ Fixed | Lazy `readFile` lets late I/O errors escape the resolver's `try` and crash the driver | none — undocumented architecture | | [FE-6](#fe-6) | G | Source | — | Included declarations are hoisted above the includer's own, erasing include position | none — undocumented architecture | | [FE-7](#fe-7) | T | Executed | — | No `asm { }` statement exists in the grammar | [§5.7](../SPEC.md#57-inline-assembly-asm) | | [FE-8](#fe-8) | G | Executed | — | Char literals: an undocumented extension, not a lexeme (`'a' + 1` fails), Haskell escape rules | [§1.3](../SPEC.md#13-integer-literals) | @@ -174,6 +174,10 @@ outside the `ExceptT` channel and crashes `c02-frontend` with an uncaught **Verified:** Source — from the `try (readFile path)` shape in `Includes.hs:100`; not executed. +**Resolved:** Added `readUtf8Safe` that forces UTF-8 encoding and early errors +otherwise. Editted driver to not register temp output file unless frontend succeeeds. +Added `bad_encoding` test case. + ### FE-6: Include position within the file is erased `Program` separates the include list from the declaration list at parse time, diff --git a/scripts/c02c.py b/scripts/c02c.py index 6f91001..b7184c7 100755 --- a/scripts/c02c.py +++ b/scripts/c02c.py @@ -62,7 +62,8 @@ def clean_o_file_temps(o_files): # A temp path is registered before its file is written, so a stage that # fails early can leave a path here that never became a file — tolerate it. try: - os.remove(f) + if os.path.isfile(f): + os.remove(f) except FileNotFoundError: pass @@ -96,11 +97,12 @@ def main(): # --- 1. frontend: c02 source -> IR object ----------------------------- for src in src_files: src_out = make_temp_object() - temps += [src_out] # collect temps for deletion after pipeline - o_files += [src_out] result = subprocess.run([frontend] + frontend_args + ['-o', src_out] + [src]) if result.returncode != 0: sys.exit(result.returncode) + # only register the temp after the frontend successfully created it + temps += [src_out] # collect temps for deletion after pipeline + o_files += [src_out] if args.parse_only or args.dump_ast: sys.exit(0) @@ -115,10 +117,11 @@ def main(): if len(o_files) > 1: # more than one object: merge them linked = make_temp_object() - temps += [linked] result = subprocess.run([linker] + o_files + [ "-o", linked]) if result.returncode != 0: sys.exit(result.returncode) + # only register the linked temp after successful linking + temps += [linked] else: linked = o_files[0] # single object nothing to merge diff --git a/test/parser/bad_encoding.c02 b/test/parser/bad_encoding.c02 new file mode 100644 index 0000000..e3975c6 --- /dev/null +++ b/test/parser/bad_encoding.c02 @@ -0,0 +1 @@ +Hello ÿ World \ No newline at end of file diff --git a/test/parser/golden/bad_encoding.golden b/test/parser/golden/bad_encoding.golden new file mode 100644 index 0000000..5d1436b --- /dev/null +++ b/test/parser/golden/bad_encoding.golden @@ -0,0 +1 @@ +Invalid UTF-8 encoding: Cannot decode byte '\xff': Data.Text.Encoding: Invalid UTF-8 stream