Skip to content

strip_test_modules: a char literal and a */ inside a line comment each blank production code from the recorder guards #303

Description

@fcostaoliveira

strip_test_modules's comment stripper has two over-strip cases that blank production code from the recorder-coverage guards. Both are latent — verified that no env:: call is currently lost and every long blanked run today is a genuine //////! block — but the trigger for the second is present in seven files, including the stripper's own source.

Over-stripping is the dangerous direction: it silently removes production code from the guard's view, so a real violation in the hidden region passes.

C-2 — a '"' char literal desyncs the string tracker

strip_block_comments (effective_config.rs:1752) tracks in_str on " and has no char-literal handling:

if in_str {
    if esc { esc = false; }
    else if b[i] == '\\' { esc = true; }
    else if b[i] == '"' { in_str = false; }
} else if b[i] == '"' { in_str = true; }

'"' is three chars: ', ", '. The " sets in_str = true and the closing ' does not clear it, so string parity is inverted from that point on. A later /* inside a string literal is then seen with in_str == false, opens a phantom block comment, and blanks to the next */ or to EOF.

Trigger shapes on master (git grep "'\"'" -- src/), 7 files:

src/bin/vector_db_benchmark/config.rs:2138,2150,2197,2204
src/bin/vector_db_benchmark/effective_config.rs:1730,1733,1780,1783

Note lines 1780 and 1783 — strip_block_comments compares b[i] == '"', so the scanner's own source carries the shape that desyncs it.

C-1 — */ inside a // line comment

/*
// disabled */
fn production() { … }

is legal Rust — line comments do not nest inside block comments, so the block ends at that */. The stripper removes line comments first, which deletes the */, leaving the block open to EOF.

This one was introduced by the fix for the previous instance. Stripping line comments first was the fix for a /// doc comment containing /* (which had blanked ~1955 lines of config.rs, including project_root's env::current_dir()); that fix created this case.

Why the existing test does not catch either

The pin added alongside the last fix covers the shapes that had been demonstrated: */ in a string literal, nested block comments, #[cfg(test)] in a macro body, CRLF, BOM, a raw string with a closed #[cfg(test)] mod, and a /// containing */. All pass. Neither case above is in the set.

This is the guards-one-notch-weak shape: the pin constrains the dimensions the known bugs varied, and both remaining cases vary a different one.

Suggested direction

Handle char literals in the tracker — treat ' as opening a char literal, consume its (possibly escaped) contents, and skip the closer without touching in_str. Raw strings (r#"…"#) are a third form the tracker also does not model, and the last fix's own stated limits acknowledge it.

Better, if it is worth the dependency: this is a lexing problem and the stripper is a hand-written character scanner — the fifth hand-written scanner in this file's history, each of which needed several rounds. proc_macro2/syn would tokenise correctly and make all of these unrepresentable rather than handled.

Whichever way, add both cases to the pin. A weakened stripper should fail loudly.

Provenance

Found by an adversarial verification pass against PR #264 at 93bb97b and re-confirmed against merged master 466b6b9, where strip_test_modules survives unchanged. The pass's other findings (PROSE_KEYS, SAFE_PARAM_KEYS, is_simple_param_value, is_connection_like) are all moot — those constants and functions were deleted in later rounds of that PR — but this one outlived them.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions