Conversation
src/lz4_decode.cuh picks a 32-bit overlap gather under seq.match_len <= UINT32_MAX and keeps the 64-bit one above it. Nothing in the suite allocated the 4 GiB a match past 2^32 needs, so nothing selected the 64-bit arm, and deleting the test left every run green - a guard the suite could not prove. tests/huge_match_gpu.cu reaches it. One valid LZ4 block: three literals, one match of 2^32 + 4096 bytes at offset 3, a five-byte literals-only tail. The output is "ABC" repeating, and 2^32 mod 3 == 1, so a gather that truncates its loop index to 32 bits shifts the phase of the window at the 4 GiB mark; a 64-byte window astride that mark carries the verdict without a 4 GiB compare. The block's well-formedness is liblz4's verdict rather than an assertion about the format: the same construction at match length 4096 is decoded by LZ4_decompress_safe first, and the large one is only built if that agrees. Registered only under -DCUDEC_HUGE_TESTS=ON, and built by every CUDA configure so the target cannot rot. ctest has no build-side way to keep an entry out of a bare `ctest` run - CONFIGURATIONS is ignored when no -C is given, DISABLED hides the entry from -L as well - so the registration is the gate. Its label is `huge` rather than `gpu`, which keeps a 4 GiB single-warp decode out of the four-tool sanitizer sweep, and CONTRIBUTING carries the invocation beside the container command. A device that cannot hold the allocation FAILS with the shortfall in the message rather than skipping, because a skip on the only test that reaches an arm reports the same silence as a pass. Measured on the RTX 3080 in the pinned container. With the guard present the window is clean and the run takes 19s; with the arm made unconditional the test reds at "window [4294967267, 4294967331): 32 of 64 bytes differ" while the other 36 entries stay green, which is the shape the issue recorded. The capacity refusal was reached the same way, by raising the requirement.
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.
Closes #331.
The arm and why it was unreachable
src/lz4_decode.cuhruns the closed-form overlap gather at two arithmeticwidths and picks the 32-bit one under
seq.match_len <= UINT32_MAX. Nothingin the suite allocated the 4 GiB a match past 2^32 needs, so nothing ever
selected the 64-bit arm, and the guard could be deleted without a single test
noticing.
What lands
tests/huge_match_gpu.cu. One valid LZ4 block: three literals, one match of2^32 + 4096 bytes at offset 3, a five-byte literals-only tail. Destination
capacity 4294971400, source 16843037 bytes, almost all of it the match-length
continuation run.
No 4 GiB comparison is needed. The output is
ABCrepeating and 2^32 mod 3== 1, so a gather that truncates its loop index to 32 bits shifts the phase of
the window at the 4 GiB mark. A 64-byte window at [4294967267, 4294967331)
carries the whole verdict; the head and the tail are checked too, so a decode
that wrote nothing cannot pass by agreeing with poison.
The block's well-formedness is the reference's verdict rather than an
assertion about the format. The same construction at match length 4096 goes
through
LZ4_decompress_safefirst, and the large block is built only if theoracle agrees byte for byte.
How it runs, and how it does not
Built by every CUDA configure, so the target cannot rot; registered as a ctest
entry only under
-DCUDEC_HUGE_TESTS=ON. ctest has no build-side way to keepan entry out of a bare
ctestrun, and both plausible mechanisms were triedrather than assumed:
CONFIGURATIONSis ignored when no-Cis given, andDISABLEDwould hidethe entry from
-Las well. So the registration is the gate.The label is
huge, notgpu:scripts/sanitize-gpu.shdiscovers itstargets with
ctest -L gpu -Nand runs four tools over every one of them, anda 4 GiB single-warp decode is not that sweep's business.
CONTRIBUTING gains that block, in a subsection beside the container command,
with the rule the next capacity-selected arm should follow.
Refused, never skipped
A device that cannot hold the allocation fails with the shortfall in the
message. Reached by raising the requirement in a throwaway edit, not in the
tree:
A skip on the only test that reaches an arm reports the same silence as a
pass, which is how the arm got here.
Proof that it bites
RTX 3080, driver 560.94, in the pinned container
(
nvidia/cuda:12.6.2-devel-ubuntu24.04, nvcc V12.6.77,-arch=sm_86).Guard present, the whole opt-in tree:
The arm made unconditional (
if (true)in place of the test, not in thetree):
32 of 64 differing is the count the issue recorded, so the shape reproduces
rather than merely failing.
The default tree is unchanged in what it runs:
Formatting:
Not covered
The Compute Sanitizer sweep over this change has NOT been run. The four tools
cannot attach to the device on this route, which issue #258 holds; this test
carries the
hugelabel and would be outside the sweep's discovery in anycase.
No second person read this change. The evidence above stands in place of one.