diff --git a/docs/notes/setranslator-exception-labels.md b/docs/notes/setranslator-exception-labels.md new file mode 100644 index 00000000..6e4f5eb1 --- /dev/null +++ b/docs/notes/setranslator-exception-labels.md @@ -0,0 +1,78 @@ +# Working note — Fix seTranslator switch fall-through (mislabeled crash diagnostics) + +Branch: `fix/setranslator-exception-labels` +Type: Bug / Reliability + +## Goal (restated) + +`seTranslator` (`src/blitzrc/bbruntime_dll/bbruntime_dll.cpp:55-70`) is the Windows +structured-exception translator: it maps a CPU fault code to a human-readable +panic message. Its `switch` has **no `break` statements**, so every case falls +through and `panicStr` always ends as `"Stack overflow!"`. Result: every native +crash — divide-by-zero, the very common null/dangling-object access violation, +illegal instruction — is reported to the user as "Stack overflow!", sending them +down the wrong debugging path. Directly contradicts INTENT #6 ("Trust the +runtime… instrumented enough to diagnose"). Confirmed by recon ~5×; the top +deferred Bug candidate. + +## Approach + +Add `break;` after each `case` in the switch so each exception code maps to its +correct message. Minimal correctness fix — no new codes, no pipeline redesign. + +```cpp +switch( u ){ +case EXCEPTION_INT_DIVIDE_BY_ZERO: panicStr = "Integer divide by zero"; break; +case EXCEPTION_ACCESS_VIOLATION: panicStr = "Memory access violation"; break; +case EXCEPTION_ILLEGAL_INSTRUCTION:panicStr = "Illegal instruction"; break; +case EXCEPTION_STACK_OVERFLOW: panicStr = "Stack overflow!"; break; +} +``` + +Message flow: `seTranslator → bbruntime_panic → RTEX → bbEx → debugError → +Debugger::debugMsg`. In test mode (`-t`) the DummyDebugger prints `"Error: " + msg` +to stdout and `exit(1)`; in normal runs it shows the message box. + +## Verification + +`-t` mode routes the panic message to stdout (DummyDebugger), so it is capturable: +a `.bb` that triggers a runtime integer divide-by-zero (divisor in a variable to +avoid constant folding), run with `blitzcc -t`, should print +`Error: Integer divide by zero` (post-fix) vs `Error: Stack overflow!` (pre-fix). +Plan: confirm this empirically against the rebuilt binary; if capturable, add a +CLI-contract check to `test.bat`/`test.sh` using a crash fixture generated OUTSIDE +`tests/` (a panic calls `exit(1)`, so it must NOT be a normal `-t` suite file — +that would abort the whole run). If the harness can't cleanly capture it, fall +back to documented manual repro + the obvious correctness of the four `break`s. + +## Files touched + +- `src/blitzrc/bbruntime_dll/bbruntime_dll.cpp` — `seTranslator` (4 `break`s). +- `test.bat` / `test.sh` — CLI crash-diagnostic contract (if capturable). +- `docs/notes/setranslator-exception-labels.md` — this note. + +Windows-only by nature (`_set_se_translator`); macOS has its own signal path +(out of scope). No ABI/codegen/parser change. + +## Acceptance criteria + +- A runtime integer divide-by-zero reports "Integer divide by zero", not + "Stack overflow!" (verified empirically; ideally pinned by a CLI check). +- Unknown codes still report "Unknown runtime exception". +- `test.bat` green; build 0 errors; no regression. + +## Trade-offs / rejected + +- **Skipped: adding more exception codes** (FLT_DIVIDE_BY_ZERO, PRIV_INSTRUCTION) — + scope creep beyond the missing-break bug; keep one-purpose. +- A normal `-t` Test block can't assert this (the panic `exit(1)`s mid-run) — hence + the standalone-fixture CLI-contract approach. + +## Deferred follow-ups (high priority, surfaced this iteration) + +- **GC doc false citation (my own #67 error):** `help/language/lang_ref_modern.html` + cites `tests/GarbageCollectionTest.bb` as proof of the "reference cycles leak — + break them yourself" contract, but that test has NO cycle case. Either add the + cycle/aliasing test cases (GC acceptance suite — a recon pitch this iteration) or + correct the citation. Worth doing soon (shipped-doc correctness). +- Contextual-keyword Phase 2 (`Release` + expression-position keywords). diff --git a/scripts/fixtures/divzero.bb b/scripts/fixtures/divzero.bb new file mode 100644 index 00000000..15caa0bd --- /dev/null +++ b/scripts/fixtures/divzero.bb @@ -0,0 +1,19 @@ +Strict +EnableGC + +; Deliberate runtime integer divide-by-zero. Used by test.bat / test.sh to verify +; the Windows structured-exception translator (seTranslator in bbruntime_dll.cpp) +; labels the fault correctly as "Integer divide by zero" -- not the pre-fix +; mislabel "Stack overflow!" caused by missing `break`s in its switch. +; +; This program PANICS and exits non-zero by design, so it lives OUTSIDE tests/ +; (it must not run in the `blitzcc -t` suite loop) and outside samples/tutorials/ +; games/ (the corpus compile-sweep). The harness invokes it explicitly with -t and +; asserts the panic message. +; +; The divisor is a variable so the divide is not constant-folded at compile time. +Test divideByZeroFault() + Local denom = 0 + Local result = 10 / denom + Assert( result = 0 ) +End Test diff --git a/src/blitzrc/bbruntime_dll/bbruntime_dll.cpp b/src/blitzrc/bbruntime_dll/bbruntime_dll.cpp index b8aa924c..d9657361 100644 --- a/src/blitzrc/bbruntime_dll/bbruntime_dll.cpp +++ b/src/blitzrc/bbruntime_dll/bbruntime_dll.cpp @@ -58,12 +58,16 @@ static void _cdecl seTranslator( unsigned int u,EXCEPTION_POINTERS* pExp ){ switch( u ){ case EXCEPTION_INT_DIVIDE_BY_ZERO: panicStr = "Integer divide by zero"; + break; case EXCEPTION_ACCESS_VIOLATION: panicStr = "Memory access violation"; + break; case EXCEPTION_ILLEGAL_INSTRUCTION: panicStr = "Illegal instruction"; + break; case EXCEPTION_STACK_OVERFLOW: panicStr = "Stack overflow!"; + break; } bbruntime_panic( panicStr.c_str() ); diff --git a/test.bat b/test.bat index 4256030c..0e110a23 100644 --- a/test.bat +++ b/test.bat @@ -23,6 +23,10 @@ call :expect_failure "EOF error names end of file, not a control byte" "end of f call :expect_ide_format "IDE machine format preserved under blitzide" "!DIAG_BB!" del /q "!DIAG_BB!" >nul 2>&1 +rem --- runtime crash diagnostic: native faults are labeled by type, not all "Stack overflow!" --- +rem The fixture panics and exits non-zero by design, so it lives outside tests\. +call :expect_failure "native divide-by-zero is labeled correctly" "Integer divide by zero" -t "%ROOTDIR%\scripts\fixtures\divzero.bb" + cd /d "%ROOTDIR%\tests" for /R %%f in (*.bb) do ( diff --git a/test.sh b/test.sh index cdbd3811..da237617 100755 --- a/test.sh +++ b/test.sh @@ -169,6 +169,26 @@ if grep -q "error:" "${TEST_TMPDIR}/diag-ide.out"; then FAILED=1 fi +# --- runtime crash diagnostic: native faults are labeled by type --- +# The seTranslator must map each structured exception to its own message rather +# than reporting every fault as "Stack overflow!". The fixture deliberately +# integer-divides by zero; in -t mode the panic prints "Error: " and exits +# non-zero, so it lives outside tests/ and is invoked explicitly. (Windows-only +# path; on hosts without the structured-exception translator this is a no-op +# check that simply won't see the message -- so only assert it where it applies.) +CRASH_BB="${ROOTDIR}/scripts/fixtures/divzero.bb" +if [[ -f "${CRASH_BB}" ]]; then + set +e + "${BLITZCC}" "${TARGET_FLAG[@]}" "${EXEC_FLAG[@]}" -t "${CRASH_BB}" > "${TEST_TMPDIR}/crash.out" 2>&1 + crash_rc=$? + set -e + if grep -q "Stack overflow" "${TEST_TMPDIR}/crash.out" && ! grep -q "Integer divide by zero" "${TEST_TMPDIR}/crash.out"; then + echo "crash diagnostic FAILED: divide-by-zero mislabeled as Stack overflow" >&2 + cat "${TEST_TMPDIR}/crash.out" >&2 + FAILED=1 + fi +fi + while IFS= read -r -d '' f; do if ! "${BLITZCC}" "${TARGET_FLAG[@]}" "${EXEC_FLAG[@]}" -t "${f}"; then echo "\"${f}\" failed at least one test"