Skip to content

adjust-mods leaks debug state to stderr and interval batching can overflow at u32 endpoints #671

Description

@SuhasSrinivasan

adjust-mods leaks debug state to stderr and interval batching can overflow at u32 endpoints

Summary

Two independent low-risk reliability defects remain in otherwise successful command paths:

  • Every otherwise-successful adjust-mods run reaching motif parsing unconditionally prints an internal dbg! line containing source location and the value of self.cpg.
  • ReferenceIntervalBatchesFeeder adds an interval size to a u32 start coordinate without overflow protection. Near u32::MAX, debug builds panic and release builds can wrap the endpoint backwards.

The fixes are kept as two separate commits so each behavior can be reviewed or reverted independently.

Severity

Severity: Low — diagnostics and extreme-coordinate reliability

Rationale: The stderr leak affects every otherwise-successful adjust-mods invocation reaching motif parsing but does not change the output BAM. The arithmetic defect can panic, regress interval coverage, or fail to terminate, but it requires an extreme coordinate and/or interval-size combination whose sum exceeds the u32 limit and is therefore an uncommon accepted-input edge case. Neither defect changes routine modification calls or statistics.

User and scientific impact

  • Affected result or workflow: scripts that parse or archive adjust-mods stderr; interval-driven commands operating near the maximum supported coordinate.
  • Direction of error: unexpected diagnostic output; panic, wrapped/repeated interval coverage, or nontermination.
  • Likely exposure: routine for the stderr line, rare for the coordinate overflow.
  • Detectability or workaround: the stderr line is visible and can be filtered; the interval failure can be avoided by choosing a start coordinate and interval size whose sum does not overflow u32.

Affected versions and environment

  • Affected release: modkit 0.6.4 contains the same faulty expressions.
  • Development revision: 5cecc3fb3a9336068d9e3c68d5c08d678153dd2c.
  • Operating system and architecture: macOS 26.6, Apple Silicon.
  • Rust toolchain: rustc/cargo 1.90.0.
  • Relevant input: a valid modBAM for adjust-mods; an in-memory reference interval ending at u32::MAX for the feeder edge case.

Steps to reproduce

1. Unconditional adjust-mods stderr

From a modkit checkout containing the repository test resources:

tmpdir=$(mktemp -d)
cargo run -p modkit -- adjust-mods \
  --ignore h \
  --suppress-progress \
  tests/resources/bc_anchored_10_reads.sorted.bam \
  "$tmpdir/adjusted.bam" \
  2>"$tmpdir/adjust.stderr"
grep -F '&self.cpg = false' "$tmpdir/adjust.stderr"

Observed behavior

The command succeeds and writes a readable BAM, but stderr contains an internal source diagnostic similar to:

[modkit-core/src/modbam_util/subcommands.rs:779:9] &self.cpg = false

The same unconditional diagnostic reports true when --cpg is used.

2. Interval endpoint overflow

Add this focused test to the existing interval_chunks_tests module:

use crate::interval_chunks::ReferenceIntervalBatchesFeeder;
use crate::util::ReferenceRecord;

#[test]
fn test_reference_interval_batches_near_u32_max() {
    let requested_start = u32::MAX - 9;
    let requested_end = u32::MAX;
    let record = ReferenceRecord::new(
        7,
        requested_start,
        requested_end - requested_start,
        "near-u32-max".to_string(),
    );
    let mut feeder = ReferenceIntervalBatchesFeeder::new(
        vec![record], 1, 8, false, None, None,
    )
    .unwrap();
    let mut intervals = Vec::new();
    let mut terminated = false;

    for _ in 0..4 {
        match feeder.next() {
            Some(Ok(batches)) => intervals.extend(
                batches
                    .into_iter()
                    .flat_map(|batch| batch.0)
                    .map(|coordinates| {
                        (coordinates.start_pos, coordinates.end_pos)
                    }),
            ),
            Some(Err(error)) => panic!("feeder failed: {error}"),
            None => {
                terminated = true;
                break;
            }
        }
    }

    assert!(terminated, "feeder did not terminate");
    assert_eq!(
        intervals,
        vec![
            (requested_start, u32::MAX - 1),
            (u32::MAX - 1, requested_end),
        ]
    );
}

Run it in debug mode:

cargo test -p mod_kit \
  interval_chunks_tests::test_reference_interval_batches_near_u32_max

The same test can be run with --release to expose wrapped/nonterminating behavior through its bounded termination and exact-interval assertions rather than a debug overflow panic.

Observed behavior

  • A debug build panics when the second interval computes start + interval_size.
  • A release build wraps the endpoint below start, violating monotonic bounded coverage and potentially preventing termination.

Control or independent oracle

The expected partition under interval size 8 is:

[u32::MAX-9, u32::MAX-1)
[u32::MAX-1, u32::MAX)

The intervals must cover exactly nine bases, remain nonempty and monotonic, never exceed the requested end, and terminate within the bounded collection loop.

Expected behavior

  • A successful adjust-mods run does not emit internal source locations or self.cpg state. Intended logging and error diagnostics remain unchanged.
  • Interval endpoint calculation cannot wrap. The edge-case feeder emits the two exact intervals above and terminates.

Root-cause evidence

  • modkit-core/src/modbam_util/subcommands.rs: Adjust::run executes dbg!(&self.cpg) unconditionally before motif parsing.
  • modkit-core/src/interval_chunks.rs: ReferenceIntervalBatchesFeeder::next_batch computes start + self.interval_size in u32 before taking the minimum with the contig end.
  • Parent-revision regressions reproduce the debug signature and the debug-build overflow. The corrected regressions additionally verify a readable nonempty BAM and exact bounded interval coverage.

Proposed fix scope

  • Remove the unintended dbg! call.
  • Use saturating endpoint addition before clipping to the contig end.
  • Add focused regressions for stderr, readable output, termination, and exact near-limit coverage.

Non-goals

  • No change to adjust/collapse/ignore/CpG semantics, MM/ML tags, BAM records, record order, or intentional logging.
  • No coordinate-type migration, broad arithmetic audit, interval-size validation, scheduling change, or performance refactor.
  • No change to extract accounting, shared writer finalization, or dependency resolution.

Acceptance criteria

  • The adjust-mods reproducer succeeds with a readable nonempty output BAM and without the internal debug signature.
  • The near-u32::MAX reproducer emits the two exact monotonic intervals and terminates.
  • Existing successful adjust-mods and interval-feeder behavior remains unchanged outside the stated scope.
  • Focused regressions and the applicable full workspace test suite pass.

Reproduction artifacts

Artifact Size SHA-256 Notes
tests/resources/bc_anchored_10_reads.sorted.bam 5,527 bytes 4441acbe1ad59caf6ab5d56bf18097d8999c08351395219c919cb863652d85f0 Existing modkit test fixture used for the stderr regression.

Related work

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