fix: initialize PreTypes sentinel to fix uninitialized read on stray #endif - #895
Open
Exberg wants to merge 1 commit into
Open
fix: initialize PreTypes sentinel to fix uninitialized read on stray #endif#895Exberg wants to merge 1 commit into
Exberg wants to merge 1 commit into
Conversation
…#endif An extra #endif (and likewise #else/#elseif/#endprocedure with an empty preprocessor nesting stack) made DoEndif and friends read AP.PreTypes[AP.NumPreTypes] == PreTypes[0]. That sentinel slot was never initialized -- Malloc1 wraps malloc without zeroing -- so the error branch depended on heap garbage. Valgrind reported "Conditional jump or move depends on uninitialised value(s)" in DoEndif, and depending on the garbage value the extra #endif was sometimes silently ignored. Initialize PreTypes[0] to PRETYPENONE at allocation time in startup, mirroring the existing PreIfStack[0] and PreSwitchModes[0] sentinels, so the stack is deterministic with or without garbage on the heap. Add a deterministic regression test (check/fixes.frm, Issue242): a stray #endif must always be diagnosed as "#endif without corresponding #if". Fixes form-dev#242
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An extra
#endif(no matching#if) made the preprocessor's terminationhandlers (
DoEndif, and the same peek pattern inDoElse,DoElseif,DoEnddo,DoEndprocedure, ...) readAP.PreTypes[AP.NumPreTypes]with anempty stack — i.e.
PreTypes[0], a sentinel slot that was neverinitialized.
Malloc1wrapsmallocwithout zeroing (tools.c), so the!= PRETYPEIFbranch depended on heap garbage:Conditional jump or move depends on uninitialised value(s)inDoEndif(pre.c), exactly as in this issue's report.#endifcould also be silently ignored (the non-reproducible case mentioned in the issue).Fix
Initialize the sentinel at allocation time in
startup.c:This mirrors the existing sentinel initialization of the sibling stacks
(
PreIfStack[0] = EXECUTINGIFin setfile.c,PreSwitchModes[0] = EXECUTINGPRESWITCHin pre.c).AddToPreTypescopies0..MaxPreTypeswhenit grows the array, so the sentinel is preserved across reallocations. No
behavioral change for correctly nested input: every handler pushes before the
slot can be read, and every matching pop restores
NumPreTypes == 0.With the fix, the exact reproducer from the issue now fails deterministically
and identically on every run:
Regression test
Added
Issue242tocheck/fixes.frm: a stray#endifmust always bediagnosed (
assert preprocess_error?("#endif without corresponding #if")).Test plan
Built locally on macOS arm64 (
./configure --disable-float && make -C sources form; MPFR not available on this machine — see note below)../check.rb ./sources/form Issue242— 1 test, 1 assertion, passes#StartFloat,mzv_,Evaluate,chop,torat, ...), reproduced identically (same 54 tests, diffed) on a pristinemasterworktree built with the same--disable-floatconfiguration. They are a pre-existing consequence of building without GMP+MPFR on this machine, unrelated to this change. GitHub CI builds with the full feature set.Note for reviewers: I could not run the Valgrind matrix locally (Linux-only
runner feature); the fix removes the uninitialized read by construction and
the deterministic test covers the observable behavior.
Closes #242