Skip to content

Conversation

@devnexen
Copy link
Contributor

@devnexen devnexen commented Feb 9, 2026

The bounds check was evaluated after the array access due to short-circuit evaluation order, and used the wrong upper bound, allowing the index to exceed the suffixes array length.

@github-actions
Copy link

github-actions bot commented Feb 9, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/tail/tail-n0f is now passing!

@sylvestre
Copy link
Contributor

please add test to make sure we don't regress in the future
thanks

@sylvestre
Copy link
Contributor

i was more thinking about the error that you could trigger from df or dd directly ?

@github-actions
Copy link

github-actions bot commented Feb 9, 2026

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/tail/tail-n0f is now passing!

The bounds check was evaluated after the array access due to
short-circuit evaluation order, and used the wrong upper bound,
allowing the index to exceed the suffixes array length.
@devnexen devnexen force-pushed the dd_df_out_of_bounds branch from 7c8e7db to de3186f Compare February 9, 2026 21:56
@github-actions
Copy link

github-actions bot commented Feb 9, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 10, 2026

Merging this PR will improve performance by 3.87%

⚡ 1 improved benchmark
✅ 283 untouched benchmarks
⏩ 38 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sort_ascii_utf8_locale 18 ms 17.3 ms +3.87%

Comparing devnexen:dd_df_out_of_bounds (6703990) with main (ec7e81e)

Open in CodSpeed

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ChrisDryden
Copy link
Collaborator

If my understanding is correct, this would only trigger when a file was 999 Yottabytes, so I don't think we will be able to make a regression test for this one 😄

@github-actions
Copy link

GNU testsuite comparison:

Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants