fix(nondet-log): unsigned rows carried a duplicated value column - #62
Merged
Conversation
nondet_log_to_file wrote the value with two independent `if`s where the first
had no `else`:
if (type == UNSIGNED) fprintf("%u;", ...);
if (type == DOUBLE) fprintf("%lf;", ...);
else fprintf("%d;", ...);
so an UNSIGNED row fell through into the %d arm as well and printed its value
twice. Reproduced against the installed binary before touching anything:
0;3;17;main;0;5;5;5 eight fields, where the format has seven
It parsed by accident. The value column is first, so it still landed at index
5 and readNonDetLog kept reading the right number -- which is why the emitted
test suites were correct and nothing caught this. What moved is the type
column, to index 7, so any consumer of it gets a copy of the value instead.
Only __VERIFIER_nondet_unsigned reaches the UNSIGNED enumerator.
__VERIFIER_nondet_uint is tagged UINT (13), a different one, and always took
the arm that was right. That is why it survived the whole baseline.
Two assertions added where the format is actually produced: every row has
exactly seven fields, and the type survives as the last one. They run in their
own subdirectory because --debug keeps the scratch directory and the cleanup
assertion below them looks for exactly that at depth 1.
Verified against a full build: emission suite 14/14.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Found while reading
nondet_log_to_fileto plan H1.3 (per-input logs).The defect
The first
ifhas noelse, so anUNSIGNEDrow falls through into the%darm as well and prints its value twice. Reproduced against the installed binary before changing anything:Why nothing caught it
It parses by accident. The value column is first, so it still lands at index 5 and
readNonDetLogkept reading the right number — the emitted test suites were correct. What moved is the type column, to index 7, so any consumer of it reads a copy of the value instead.And the live path is narrow: only
__VERIFIER_nondet_unsignedreaches theUNSIGNEDenumerator.__VERIFIER_nondet_uintis taggedUINT(13), a different enumerator, and always took the arm that was right. That is how it survived the entire v5/v6 baseline.The fix
else if, plus two assertions where the format is actually produced — the CSV is written by the C runtime and nothing else in the repository checked its shape:They run in their own subdirectory:
--debugkeeps the scratch directory, and the cleanup assertion below them looks for exactly that at depth 1 of$WORK.Verification
0;3;17;main;0;5;5;5(8 fields) with the already-installed binary — no rebuild, so this is the unmodified code.