Repository navigation
CI validation: --filelist CRLF fix (Windows-only strip) - #3
Open
jaypatrickhoward wants to merge 121 commits into
Open
jaypatrickhoward wants to merge 121 commits into
jaypatrickhoward wants to merge 121 commits into
Conversation
ZSTD_compressSequences() validates every block through
determine_blockSize(), which in explicit-delimiter mode rejects a block
whose regenerated size exceeds cctx->blockSizeMax:
if (explicitBlockSize > blockSize)
RETURN_ERROR(externalSequences_invalid,
"sequences incorrectly define a too large block");
ZSTD_compressSequencesAndLiterals_internal() only supports explicit
delimiters, but does not perform that check. It calls
ZSTD_get1BlockSummary() and validates block.litSize against litSize
only, so sequences describing an over-sized block are accepted and the
function returns success while emitting a block that exceeds the
maximum block size the format allows.
The two APIs disagree on identical input:
ZSTD_compressSequences -> External sequences are not valid
ZSTD_compressSequencesAndLiterals -> SUCCESS (cSize = 36)
For a large enough block the resulting frame is not decodable, so the
caller receives a success return for bytes that cannot be read back.
The guard was added for the sibling path in 87dcd33 (2022). The
AndLiterals pipeline was split out later, in 14a21e4 (2024), without
carrying it over.
basicUnitTests() already declares size_t cSize, so the inner declaration tripped -Wshadow and failed builds using MOREFLAGS=-Werror.
jaypatrickhoward
force-pushed
the
filelist-crlf
branch
8 times, most recently
from
September 7, 2026 13:34
078603b to
e74a51a
Compare
The Clang-specific LIKELY and UNLIKELY annotations in ZSTD_decodeSequence() were removed because they hurt cache locality in PGO builds. The comments that justified those annotations remained and now appear in both architecture-specific implementations. Remove both stale comments now that they no longer describe the code. Signed-off-by: Usama Arif <usama.arif@linux.dev>
… not enabled Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>
…efDDict(NULL) When ZSTD_d_refMultipleDDicts is used, the DCtx keeps a hashset of DDict pointers. ZSTD_DCtx_reset(session_and_parameters) and ZSTD_DCtx_refDDict(dctx, NULL) cleared the dictionary but left the set populated, so later references were added to a set that still held pointers to DDicts the caller may have freed. A frame asking for the dictID of one of those then made ZSTD_DCtx_selectFrameDDict() read the freed DDict. Free the set in both places, and add a test that decodes a frame after the referenced DDict has been freed.
… use With nbWorkers >= 1, a job of an abandoned frame can still be running when the context frees memory it uses: - ZSTD_freeCCtx() frees the dictionary before ZSTDMT_freeCCtx() joins the workers; - ZSTD_CCtx_reset(session_and_parameters), or a session reset followed by loading another dictionary, frees it while the job runs; - with ZSTD_CCtx_refThreadPool(), ZSTDMT_freeCCtx() does not stop the shared pool and frees the jobs table and CCtx pool under the worker; - after a session reset, raising nbWorkers resizes the CCtx pool before the abandoned jobs are waited for. Move the "previous compression not correctly finished" wait of ZSTDMT_initCStream_internal() into ZSTDMT_waitForPendingJobs(), and call it on session reset, at the start of ZSTDMT_freeCCtx(), and before ZSTDMT_resize(). Free the MT context before the dictionaries. Fixes meta#4862 Signed-off-by: Eshan05 <nahareshan05@gmail.com>
The macos-14 runner image is no longer available: the job was cancelled before starting, with "The configuration 'macos-14-arm64-us-default' is not supported". macos-latest is an ARM64 (Apple Silicon) runner.
ci: run the macOS ARM64 CMake job on macos-latest
--memory (128 MiB by default) only limited the decompression of zstd frames: liblzma was initialized without any limit. Pass it to liblzma as well. When a file needs more, report how much memory it needs and suggest a --memory= value, as for zstd frames. .xz and .lzma files that need more than 128 MiB to decode now require --memory=, like zstd frames with a large window.
Apply --memory to .xz and .lzma decompression
reject out-of-range numFrames in seekable seek table loader
…rflow programs: fix `zstd -l` infinite loop on a large skippable frame
The CLI does not follow symbolic links by default (only -f does), but on Windows UTIL_isLink() always returned 0, and the recursive directory walk entered symbolic links and junctions like ordinary directories. -r could then compress, and with --rm delete, files outside the given directory. Treat name surrogate reparse points, i.e. symbolic links and junctions, as links, like lstat() does on POSIX. Other reparse points, such as OneDrive placeholders or deduplicated files, remain regular files and directories. Fixes meta#4853. The test comes from meta#4860. Co-authored-by: Nmd-sketch <revusarlic@gmail.com>
programs: don't follow symbolic links and junctions in -r on Windows
ZSTD_seekable_decompress() limited len with offset + len > eos. An offset well past the end made eos - offset underflow into a huge byte count that ZSTD_isError() did not flag. An offset + len that wrapped around skipped the limit, and the loop never exited. Return parameter_outOfBound for an offset past the end, and compare len against eos - offset. Reading at the end still returns 0.
…ocks When a block's literals are split between dst and litExtraBuffer, the first part of the literals is stored at the end of the block's area in dst. A block that decodes to more than that area could overwrite these literals before reading them, and decode to wrong output: - ZSTD_safecopyDstBeforeSrc() used ZSTD_wildcopy() when src was more than 16 bytes ahead of dst, but ZSTD_wildcopy() can write up to 31 bytes past its length, onto source bytes not yet copied. Only use it when src is at least WILDCOPY_OVERLENGTH bytes ahead. - ZSTD_execSequenceEndSplitLitBuffer() only detected op starting inside the literals being copied. Reject any sequence whose output runs past the end of the literals it consumes, which also covers matches overwriting the literals of the next sequences. - ZSTD_decompressFrame() didn't check that a block decodes to at most blockSizeMax, unlike ZSTD_decompressContinue(). One-shot decompression accepted such blocks, including raw and RLE blocks larger than ZSTD_d_maxBlockSize. Valid blocks never decode to more than MIN(blockSizeMax, dstCapacity), so the output stays at least 64 KB - 32 bytes behind the literals in dst, and isn't affected. All writes were within dst, so this was not a memory safety issue. Frames with blocks larger than the window are now rejected by the one-shot API, as they already were by streaming. Add tests with a crafted block for the block and frame APIs, and for ZSTD_d_maxBlockSize with raw and RLE blocks.
…e data The return value is smaller than dstSize when the data ends first, 0 when offset is the end of the data, and an error past it.
…st-end seekable_format: Reject offsets past the end in decompress
The output file was opened with O_CREAT|O_TRUNC, which follows a symbolic link. stat() fails on a dangling link, so it was not seen as an existing file: zstd created its target and wrote the output there, even without -f. A new output file is now created with O_EXCL, which fails on any symbolic link, even a dangling one, including a link that appears after the existence check or after an existing file was removed. A dangling link now counts as an existing file: it is not overwritten without -f, and -f replaces the link, as it already did for a link to an existing file. Existing files that are not regular files, such as /dev/null, /dev/stdout or pipes, are still opened as they are. They are no longer truncated, and they are refused if they turn out to be regular files once opened. A regular file that could not be removed is still truncated in place.
Keep the single-DDict state empty while a DDict set exists. Resolve each ordinary frame into a local DDict pointer before installing decoder state, using no dictionary when ID 0 has no registered entry, and leave explicit-dictionary APIs independent of the set. Preserve trace metadata without retaining a borrowed DDict. Reject implicit set-based legacy decoding, retain explicit legacy decoding, and cover immediate release after disabling a set, raw ID 0, dictionary-free frames, and unchanged local-prefix behavior.
Invalidate the cached frame before seeking, and record the new frame only after its input and decoder state are initialized. Otherwise a failed seek can leave a new object with a NULL input pointer, and retrying within the same frame bypasses initialization. Add a malformed 65-byte archive regression covering in-memory and custom sources, repeated failed reads, and recovery to a valid frame.
Reinitialization overwrote the previous seek-table allocation. Read failures while loading a new table could also leak it, and failed reinitialization could leave old decoder state paired with the new source. Release the previous table and reset decoder state before initialization, free partially loaded tables on I/O errors, and reject decompression until initialization succeeds. Document the reset contract and add regression tests for reuse, failure, and recovery across all input APIs.
libzstd.mk adds -z cet-report=error when the compiler accepts it, but it tests this in 64-bit mode. zstd32, fullbench32, fuzzer32 and zstreamtest32 add -m32 afterwards, and 32-bit system objects usually have no CET properties: with gcc on Ubuntu 24.04 and 26.04, `make -C programs zstd32` fails with "missing IBT and SHSTK properties". The sanitizer targets hid it, because they forced the gold linker, which ignores this option. These targets now link with LDFLAGS_32, which leaves the option out. 64-bit links keep it.
The ASan runtime of gcc-8 loops on AddressSanitizer:DEADLYSIGNAL when the kernel uses 32 bits of mmap randomization. The ubuntu-24.04 image sets vm.mmap_rnd_bits=28; ubuntu-26.04 does not, and gcc-8-asan-ubsan-fuzz printed 102 million such lines there. Both gcc-8 jobs now set it themselves. Newer ASan runtimes (gcc 15, clang) are not affected.
LDFLAGS=-fuse-ld=gold was added in 2017 for Travis CI (f58ac79). The sanitizers link with the default linker, and gold is deprecated in binutils: it is not installed on ubuntu-26.04 runners, so these targets failed there. The 15 sanitizer jobs, including clang-asan-ubsan-cli-tests, return to ubuntu-latest.
qemu-user-static is only a virtual package on ubuntu-26.04, so the ARM and PowerPC install targets failed there. arminstall and ppcinstall now install qemu-user, which exists from Ubuntu 20.04 to 26.04 and on Debian 12, and does not remove an installed qemu-user-static. qemu-user names its binaries qemu-<arch>, without the -static suffix. The targets call qemu-<arch> when qemu-arm is installed, and qemu-<arch>-static otherwise, as before: a machine with only qemu-user-static keeps working. The tests call every cross-compiled binary through QEMU_SYS, so no binfmt registration is needed. armbuild and armfuzz return to ubuntu-latest.
short-tests-1 installed lib64gcc-13-dev-powerpc-cross, which only matches the GCC 13 cross compiler of ubuntu-24.04: with GCC 15 on ubuntu-26.04, ppc64build could not find -lgcc. gcc-multilib-powerpc-linux-gnu installs the 64-bit libraries of the default cross compiler, whatever its version. short-tests-1 returns to ubuntu-latest.
GCC on ubuntu-26.04 runners targets x86-64-v3 by default. With BMI2 enabled, v1.3.1 does not compile: its threading.h defines pthread_mutex_t as a macro, which then clashes with the glibc type pulled in by <immintrin.h>. The versions test now builds with -march=x86-64 on x86-64 hosts. Verified with GCC 15 on Ubuntu 26.04 and x86-64-v3 as default target: the 46 versions build and the test passes. versions-compatibility returns to ubuntu-latest.
The size limits had margins between 1% and 100%. On ubuntu-26.04 runners, GCC 15 targets x86-64-v3 by default, which makes libzstd.so 21% larger (1105472 bytes instead of 909760), above the 1100000 limit. The test now builds with -march=x86-64, so the measure does not depend on the default target of the compiler. Each limit is the size measured with GCC 15.2, plus about 30%, rounded up: this still catches a jump such as meta#3066. check_size.py prints every size, to make the next measure easy. The workflow documents the rule. libzstd-size returns to ubuntu-latest.
GCC on ubuntu-26.04 runners targets x86-64-v3, which enables BMI2 by default. The dynamic-bmi2 cases without -mbmi2 then still had BMI2, so the generic code path and the runtime dispatch were no longer tested. Each case now starts from -march=x86-64.
On 32-bit builds, ZSTDMT_initCStream_internal() computed the round buffer size in size_t. The total can reach or exceed 4 GiB and wrap: - windowLog 30 with btultra2 and LDM disabled makes the overlap 1 GiB, which raises the job size to 1 GiB after the job size cap is applied. The 4 GiB total wraps to 0, the allocation is skipped, and the first streaming copy writes through a NULL buffer. - Many workers with large jobs wrap to a smaller non-zero size, so the buffer is allocated but is too small for the configuration. Compute the slack/section totals in 64 bits and reject the configuration with memory_allocation when the total exceeds SIZE_MAX, before allocating. Add a 32-bit-only fuzzer regression test that expects both cases to fail cleanly with memory_allocation. Co-authored-by: Kevin Zhang <kevz8@fb.com>
On 32-bit builds, a level 22 stream with nbWorkers=1 needs a 2 GiB round buffer (4 x 512 MB), which malloc cannot provide in a 32-bit address space, so compression fails with memory_allocation. Lower ZSTDMT_JOBLOG_MAX from 29 to 28 and ZSTDMT_JOBSIZE_MAX from 512 MB to 256 MB on 32-bit builds, so the same case needs 1 GiB. 64-bit builds are unchanged. Addresses meta#4628. Co-authored-by: Kevin Zhang <kevz8@fb.com>
List mode read the skippable length before checking that all eight header bytes were available, allowing stale stack bytes to affect a printed seek position. Reject truncated headers before reading the length and cover all magic values and short lengths, while preserving the signed 64-bit seek regression.
…mode - -T# default is ZSTDCLI_NBTHREADS_DEFAULT, based on logical cores (UTIL_countLogicalCores() / 4, since meta#4211), not physical cores. - ZSTDMT_NBWORKERS_MAX is 64 on 32-bit and 256 on 64-bit, not 200. - Benchmark mode uses 1 worker thread by default since 725a152. Signed-off-by: YoungjuKim <fjvbn2003@gmail.com>
ci: run the pinned jobs on ubuntu-26.04
The four jobs that test gcc-7 and gcc-8 install them from the focal archive. They also added ppa:ubuntu-toolchain-r/test through make gcc7install and gcc8install, but no package came from it: it only cost about 10 s per job, and a Launchpad SSL error failed one job on meta#4949. The jobs now install the same packages with apt-get directly. The make targets are unchanged, for the machines that still need the PPA.
ci: install gcc-7 and gcc-8 without the toolchain PPA
This avoids a lot of name collision issues with other projects. See openzfs/zfs@a2ac9cd which made the same change downstream.
Bound the cumulative sample count by INT_MAX and the maximum allocatable size_t table length before narrowing or allocation. This prevents small-chunk training inputs from wrapping the allocation size on 32-bit builds. Add a fast sparse-file regression covering all trainers, partial chunks, and counts accumulated across files.
Some fullbench scenarios (convertSequences, get1BlockSummary) do not need a destination buffer, so their preparation leaves dst == NULL and dstCapacity == 0. BMK_benchFunction() then called memset(NULL, 0xE5, 0). Passing a null pointer to memset() is undefined behavior even when the length is 0, and UBSan reports it through glibc's nonnull annotation: programs/benchfn.c:116:13: runtime error: null pointer passed as argument 1, which is declared to never be null Skip the warm-up memset when the destination capacity is 0. Fixes meta#4776
Both functions were deprecated in the experimental API, documented as misleading with very limited utility. Remove them along with the static ZSTD_copyCCtx_internal() helper, the manual entry, and the tests that exercised them. In the fuzzer, drop the context-duplication tests, and have the bufferless streaming test initialize its context directly instead of copying from a reference context.
qemu-consistency was pinned to ubuntu-24.04 in 2025, to get a newer GCC than ubuntu-22.04 for the RISC-V vector tests. It now runs on ubuntu-latest: - qemu-user replaces qemu-user-static, which is only a virtual package on ubuntu-26.04, and the emulators lose their -static suffix; - qemu-user-binfmt keeps the binfmt registration that qemu-user-static provided, because the tests run the cross-compiled datagen directly. On ubuntu-26.04, its handlers are only loaded when systemd-binfmt restarts, so the job restarts it and checks the handler of its target; - the qemu-system-* emulators and qemu-utils are no longer installed, since the job only uses user-mode emulation; - RISC-V uses the default cross compiler (GCC 15 on ubuntu-26.04) instead of gcc-14. Its libc targets the RVA23 profile, which the generic rv64 CPU model of qemu lacks, so the vector tests use -cpu max with each vlen; - ubuntu-26.04 has no MIPS cross compiler, so MIPS builds and runs its tests in a debian:13 container (GCC 14.2). The binfmt handlers of the runner use the F flag, so they also work in the container.
ci: run qemu-consistency on ubuntu-latest
`--filelist` stopped accepting Windows CRLF line endings in 165e52c ("first implementation supporting Process Substitution", meta#4349). That rewrite reads the list into a buffer opened in binary mode and splits it on '\n' alone. Previously the list was opened in text mode, where the Windows CRT translated CRLF to LF before the parser saw it, so stripping '\n' was sufficient. In binary mode the CR survives into every path: zstd: can't stat a.txt : No such file or directory -- ignored The '\r' is invisible in that message, which makes it awkward to diagnose. Strip a trailing '\r' when the line pointers are built, but only on Windows, where CRLF is the native line separator and a '\r' cannot be used in a filename. Elsewhere '\r' is a legal filename byte and is left alone, so behaviour on those platforms is unchanged. This restores the pre-regression behaviour exactly, on every platform, without adding a new one. Adds a playTests case, guarded to Windows, that fails without the fix.
Cyan4973
force-pushed
the
filelist-crlf
branch
from
November 15, 2026 04:58
e74a51a to
9776f6d
Compare
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.
Fork-only PR to run full CI on 060ea1e before the upstream draft is marked ready. Do not merge.