From c60c9225e55eb3ac7db94f3467cfb94b562bca72 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 9 Nov 2026 21:59:12 +0100 Subject: [PATCH 1/5] chunkers: accumulate chunking_time in a double ChunkerBase.chunking_time was a C float. It adds up one small time increment per chunk, and a 32-bit float stops growing once an increment is smaller than half an ulp of the sum: with 50 us per chunk at 1024 s at the latest, with 20 us at 512 s. "Time spent in chunking" of a long backup of many small files thus came out too low. The fixed and failing chunkers already use Python floats (doubles). Co-Authored-By: Claude Fable 5.2 --- src/borg/chunkers/base.pxd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/chunkers/base.pxd b/src/borg/chunkers/base.pxd index a99f040eb3..8ecaeddd33 100644 --- a/src/borg/chunkers/base.pxd +++ b/src/borg/chunkers/base.pxd @@ -18,7 +18,7 @@ cdef class ChunkerBase: cdef int done, eof cdef size_t min_size, buf_size, remaining, position, last cdef long long bytes_read, bytes_yielded - cdef readonly float chunking_time + cdef readonly double chunking_time # a sum of many small increments, a C float would stop growing cdef object file_reader cdef size_t reader_block_size cdef bint sparse From 96f61c97d666c88008df3a184d0b7a1f636bfc20 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 9 Nov 2026 21:59:50 +0100 Subject: [PATCH 2/5] chunkers: ValueError for invalid params, reject negative window size The CDC chunkers checked their constructor parameters with asserts, but their scan code relies on these checks to stay inside the buffer, and python -O drops asserts: e.g. an AES chunker created with chunk_min_exp 4 then read 48 bytes in front of its buffer. They raise ValueError now. A negative hash_window_size was not checked at all - not by the buzhash chunkers, where it became a huge size_t, and not by ChunkerParams, so that e.g. --chunker-params buzhash64,10,16,14,-5,0 crashed borg. Both reject it now. Co-Authored-By: Claude Fable 5.2 --- src/borg/chunkers/base.pyx | 15 +++++++---- src/borg/chunkers/buzhash.pyx | 7 ++++-- src/borg/chunkers/buzhash64.pyx | 7 ++++-- src/borg/chunkers/phte_chunker.pyx | 3 ++- src/borg/helpers/parseformat.py | 10 +++++--- src/borg/testsuite/chunkers/fastcdc_test.py | 25 +++++++++++++++++++ .../testsuite/helpers/parseformat_test.py | 2 ++ 7 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/borg/chunkers/base.pyx b/src/borg/chunkers/base.pyx index 89ae15cf7a..a15045e903 100644 --- a/src/borg/chunkers/base.pyx +++ b/src/borg/chunkers/base.pyx @@ -50,8 +50,12 @@ cdef class ChunkerBase: """ min_size = 1 << chunk_min_exp max_size = 1 << chunk_max_exp - assert max_size <= len(zeros) - assert min_size + 1 <= max_size, "too small max_size" + # The parameter checks here and in the subclasses raise ValueError rather than + # assert: the scan code relies on them for memory safety and python -O drops asserts. + if max_size > len(zeros): + raise ValueError("too big max_size") + if min_size + 1 > max_size: + raise ValueError("too small max_size") self.algo = algo self.chunk_mask = _mask_bits(hash_mask_bits, high_masks) @@ -61,9 +65,10 @@ cdef class ChunkerBase: # probability). This concentrates chunk sizes around the target and reduces # chunk-size variance. nc_level == 0 disables it, keeping behavior byte-identical # to the single-mask chunker. - assert nc_level >= 0 - assert hash_mask_bits - nc_level >= 1, "nc_level too large for hash_mask_bits" - assert hash_mask_bits + nc_level <= 48, "nc_level too large for hash_mask_bits" + if nc_level < 0: + raise ValueError("nc_level must not be negative") + if hash_mask_bits - nc_level < 1 or hash_mask_bits + nc_level > 48: + raise ValueError("nc_level too large for hash_mask_bits") self.nc_level = nc_level if nc_level: self.mask_s = _mask_bits(hash_mask_bits + nc_level, high_masks) diff --git a/src/borg/chunkers/buzhash.pyx b/src/borg/chunkers/buzhash.pyx index 6548fffdf1..2b93dd8737 100644 --- a/src/borg/chunkers/buzhash.pyx +++ b/src/borg/chunkers/buzhash.pyx @@ -138,7 +138,10 @@ cdef class Chunker(ChunkerBase): def __cinit__(self, int seed, int chunk_min_exp, int chunk_max_exp, int hash_mask_bits, int hash_window_size, bint sparse=False): self.table = NULL # see process, first while loop condition, first term must be able to get True: - assert hash_window_size + (1 << chunk_min_exp) + 1 <= (1 << chunk_max_exp), "too small max_size" + if hash_window_size < 0: + raise ValueError("hash_window_size must not be negative") + if hash_window_size + (1 << chunk_min_exp) + 1 > (1 << chunk_max_exp): + raise ValueError("too small max_size") self.window_size = hash_window_size self.table = buzhash_init_table(seed & 0xffffffff) # buzhash is single-mask (no normalized chunking): nc_level 0 makes @@ -167,7 +170,7 @@ cdef class Chunker(ChunkerBase): else: raise Exception("chunkifier byte count mismatch") - while self.remaining < min_size + window_size + 1 and not self.eof: # see assert in Chunker init + while self.remaining < min_size + window_size + 1 and not self.eof: # see check in __cinit__ self.fill() # Here we either are at eof... diff --git a/src/borg/chunkers/buzhash64.pyx b/src/borg/chunkers/buzhash64.pyx index 942a668379..9569814e54 100644 --- a/src/borg/chunkers/buzhash64.pyx +++ b/src/borg/chunkers/buzhash64.pyx @@ -150,7 +150,10 @@ cdef class ChunkerBuzHash64(ChunkerBase): self.table = NULL self.table_rot = NULL # see process, first while loop condition, first term must be able to get True: - assert hash_window_size + (1 << chunk_min_exp) + 1 <= (1 << chunk_max_exp), "too small max_size" + if hash_window_size < 0: + raise ValueError("hash_window_size must not be negative") + if hash_window_size + (1 << chunk_min_exp) + 1 > (1 << chunk_max_exp): + raise ValueError("too small max_size") self.window_size = hash_window_size self.table = buzhash64_init_table(key) @@ -204,7 +207,7 @@ cdef class ChunkerBuzHash64(ChunkerBase): else: raise Exception("chunkifier byte count mismatch") - while self.remaining < min_size + window_size + 1 and not self.eof: # see assert in Chunker init + while self.remaining < min_size + window_size + 1 and not self.eof: # see check in __cinit__ self.fill() # Here we either are at eof... diff --git a/src/borg/chunkers/phte_chunker.pyx b/src/borg/chunkers/phte_chunker.pyx index beeab9613e..f2b90451f7 100644 --- a/src/borg/chunkers/phte_chunker.pyx +++ b/src/borg/chunkers/phte_chunker.pyx @@ -33,7 +33,8 @@ cdef class ChunkerPHTE(ChunkerBase): # the rolling window needs 64 bytes of in-chunk history at the first # possible cut position (min_size), so the minimum chunk size must be # at least the window size: - assert chunk_min_exp >= 6, "chunk_min_exp must be >= 6 (2^6 = 64 = window size)" + if chunk_min_exp < 6: + raise ValueError("chunk_min_exp must be >= 6 (2^6 = 64 = window size)") self.kernel_str = "unknown" # the AES output is uniform, so contiguous low-bit masks are fine return self._setup_common(algo, chunk_min_exp, chunk_max_exp, hash_mask_bits, diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 57dad2dd2a..889472770d 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -336,8 +336,10 @@ def ChunkerParams(s): raise ArgumentTypeError("min. chunk size exponent must not be less than 6 (2^6 = 64B min. chunk size)") if chunk_max > 23: raise ArgumentTypeError("max. chunk size exponent must not be more than 23 (2^23 = 8MiB max. chunk size)") + if window_size < 0: + raise ArgumentTypeError("window_size must not be negative") # the chunker needs the hash window plus one byte beyond the min. chunk size to fit into - # a max. chunk size sized buffer, see the assertion in ChunkerBuzHash64.__cinit__. + # a max. chunk size sized buffer, see the check in ChunkerBuzHash64.__cinit__. if window_size + 2**chunk_min + 1 > 2**chunk_max: raise ArgumentTypeError("required: window_size + 2^chunk_min + 1 <= 2^chunk_max") # normalized chunking switches the mask at the target size; it needs room below and above @@ -364,7 +366,7 @@ def ChunkerParams(s): if chunk_max > 23: raise ArgumentTypeError("max. chunk size exponent must not be more than 23 (2^23 = 8MiB max. chunk size)") # the chunker needs one byte beyond the min. chunk size to fit into a max. chunk size - # sized buffer, see the assertion in ChunkerFastCDC.__cinit__. + # sized buffer, see the check in ChunkerBase._setup_common. if chunk_min >= chunk_max: raise ArgumentTypeError("required: chunk_min < chunk_max") if not (0 <= nc_level and chunk_mask - nc_level >= 1 and chunk_mask + nc_level <= 48): @@ -409,10 +411,12 @@ def ChunkerParams(s): raise ArgumentTypeError("min. chunk size exponent must not be less than 6 (2^6 = 64B min. chunk size)") if chunk_max > 23: raise ArgumentTypeError("max. chunk size exponent must not be more than 23 (2^23 = 8MiB max. chunk size)") + if window_size < 0: + raise ArgumentTypeError("window_size must not be negative") if window_size % 2 == 0: raise ArgumentTypeError("window_size must be an uneven (odd) number") # the chunker needs the hash window plus one byte beyond the min. chunk size to fit into - # a max. chunk size sized buffer, see the assertion in Chunker.__cinit__ (buzhash.pyx). + # a max. chunk size sized buffer, see the check in Chunker.__cinit__ (buzhash.pyx). if window_size + 2**chunk_min + 1 > 2**chunk_max: raise ArgumentTypeError("required: window_size + 2^chunk_min + 1 <= 2^chunk_max") return CH_BUZHASH, chunk_min, chunk_max, chunk_mask, window_size diff --git a/src/borg/testsuite/chunkers/fastcdc_test.py b/src/borg/testsuite/chunkers/fastcdc_test.py index d99d5a6424..058d9f8474 100644 --- a/src/borg/testsuite/chunkers/fastcdc_test.py +++ b/src/borg/testsuite/chunkers/fastcdc_test.py @@ -270,6 +270,31 @@ def expected_default_kernel(envvar, make, key, monkeypatch): raise AssertionError(f"none of {preference} is usable for {envvar} on {machine}") +@pytest.mark.parametrize( + "algo, params", + [ + (CH_FASTCDC, (10, 10, 10, 0)), # min. chunk size + 1 does not fit into the max. chunk size + (CH_FASTCDC, (10, 25, 14, 0)), # max. chunk size exceeds MAX_DATA_SIZE + (CH_FASTCDC, (10, 16, 14, -1)), # negative nc_level + (CH_FASTCDC, (10, 16, 14, 14)), # nc_level leaves no bits for the loose mask + (CH_BUZHASH, (10, 12, 11, 4095)), # window + min. chunk size + 1 do not fit into the max. chunk size + (CH_BUZHASH, (10, 16, 14, -4095)), # negative window size + (CH_BUZHASH64, (10, 12, 11, 4095, 0)), # as for buzhash + (CH_BUZHASH64, (10, 16, 14, -4095, 0)), # as for buzhash + (CH_TOEPLITZ_AES, (5, 16, 14, 0)), # min. chunk size smaller than the 64-byte window + (CH_RABIN_AES, (5, 16, 14, 0)), + (CH_GOLDILOCKS_AES, (5, 16, 14, 0)), + ], +) +def test_chunkers_reject_invalid_params(algo, params): + # The chunkers' own parameter checks protect their scan code from reading outside + # the buffer, so they must raise even when asserts are off (python -O) and for + # callers that do not go through ChunkerParams. An assert would fail this test with + # an AssertionError. + with pytest.raises(ValueError): + get_chunker(algo, *params, sparse=False) + + @pytest.mark.parametrize("envvar", ["BORG_FASTCDC_KERNEL", "BORG_BUZHASH64_KERNEL", "BORG_AES_CHUNKER_KERNEL"]) def test_kernel_env_rejects_unusable(envvar, monkeypatch): # A kernel that cannot run here must fail loudly instead of silently diff --git a/src/borg/testsuite/helpers/parseformat_test.py b/src/borg/testsuite/helpers/parseformat_test.py index 0facb3e85f..857615006c 100644 --- a/src/borg/testsuite/helpers/parseformat_test.py +++ b/src/borg/testsuite/helpers/parseformat_test.py @@ -919,10 +919,12 @@ def test_valid_chunkerparams(chunker_params, expected_return): "buzhash,19,23,21,4096", # even window size "buzhash,19,23,21", # missing window_size (must not fall into old-style compat mode) "buzhash,10,12,11,4095", # window_size + 2^chunk_min + 1 > 2^chunk_max + "buzhash,19,23,21,-4095", # negative window size "19,19,19,4095", # dito, chunk_min == chunk_max (old-style compat mode) "buzhash64,20,20,20,4095,2", # window_size + 2^chunk_min + 1 > 2^chunk_max "buzhash64,19,19,19,4095,2", # dito, chunk_min == chunk_max "buzhash64,19,23,21,4095", # missing nc_level + "buzhash64,19,23,21,-4095,2", # negative window size "fastcdc,20,20,20,2", # chunk_min == chunk_max "fastcdc,19,23,21", # missing nc_level (must not fall into old-style compat mode) "fixed,63", # too small block size From c6a305dfd3bf8b6966d6d143be33fe5f883ea221 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 9 Nov 2026 22:00:00 +0100 Subject: [PATCH 3/5] chunkers: cap the scan at the nc transition by length, not by position To stop a scan at the strict->loose mask transition of normalized chunking, process() computed last + normal_size and compared that as a pointer into the buffer. normal_size is a caller-given size_t: for a huge value the sum wrapped around, the "transition" ended up in front of the scan position and the scan span went negative, i.e. far out of the buffer (normal_size = 2^64 - 4096 crashed with a bus error). Compare lengths instead: the scan is capped if normal_size - chunk_len is less than what is left to scan. Cut points are unchanged. No borg command passes normal_size, this only affected API users. Co-Authored-By: Claude Fable 5.2 --- src/borg/chunkers/base.pyx | 18 ++++++-------- src/borg/chunkers/buzhash64.pyx | 27 +++++++++------------ src/borg/testsuite/chunkers/fastcdc_test.py | 22 +++++++++++++++++ 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/borg/chunkers/base.pyx b/src/borg/chunkers/base.pyx index a15045e903..e8de19c153 100644 --- a/src/borg/chunkers/base.pyx +++ b/src/borg/chunkers/base.pyx @@ -157,10 +157,9 @@ cdef class ChunkerBase: cdef uint64_t digest = 0, mask, mask_s = self.mask_s, mask_l = self.mask_l cdef int nc_level = self.nc_level cdef size_t n, old_last, min_size = self.min_size - cdef size_t normal_size = self.normal_size, normal_pos, chunk_len, span, did + cdef size_t normal_size = self.normal_size, chunk_len, span, did cdef int64_t r cdef uint8_t* p - cdef uint8_t* stop if self.done: if self.bytes_read == self.bytes_yielded: @@ -207,14 +206,13 @@ cdef class ChunkerBase: continue p = self.data + self.position - stop = p + self.remaining - if nc_level and chunk_len < normal_size: - # do not scan past the strict->loose transition; re-evaluate the mask there - normal_pos = self.last + normal_size - if (self.data + normal_pos) < stop: - stop = self.data + normal_pos - - span = stop - p + span = self.remaining + if nc_level and chunk_len < normal_size and normal_size - chunk_len < span: + # do not scan past the strict->loose transition; re-evaluate the mask there. + # This compares lengths, not positions: normal_size is a caller-given size_t + # and can be anything, self.last + normal_size might not even be representable. + span = normal_size - chunk_len + r = self._scan(p, span, &digest, mask) if r == -2: raise Exception(f"{self.algo}: OpenSSL EVP encryption failed") diff --git a/src/borg/chunkers/buzhash64.pyx b/src/borg/chunkers/buzhash64.pyx index 9569814e54..6103cc5cc0 100644 --- a/src/borg/chunkers/buzhash64.pyx +++ b/src/borg/chunkers/buzhash64.pyx @@ -194,10 +194,8 @@ cdef class ChunkerBuzHash64(ChunkerBase): cdef uint64_t mask_s = self.mask_s, mask_l = self.mask_l cdef int nc_level = self.nc_level cdef size_t n, old_last, min_size = self.min_size, window_size = self.window_size - cdef size_t normal_size = self.normal_size, normal_pos + cdef size_t normal_size = self.normal_size, chunk_len cdef uint8_t* p - cdef uint8_t* stop_at - cdef uint8_t* nc_stop cdef size_t did_bytes, span cdef int kernel_id = self.kernel_id @@ -234,29 +232,28 @@ cdef class ChunkerBuzHash64(ChunkerBase): # Normalized chunking: pick the mask based on how far we are into the current chunk. # While below normal_size use the strict mask (lower cut probability), afterward the # loose mask (higher cut probability). The mask is re-evaluated at the top of every - # iteration, so the transition is honored exactly at normal_pos. When nc is disabled, - # mask_s == mask_l == chunk_mask and the normal_pos cap is not applied, so this reduces + # iteration, so the transition is honored exactly at normal_size. When nc is disabled, + # mask_s == mask_l == chunk_mask and the normal_size cap is not applied, so this reduces # to the original single-mask behavior. mask = mask_s - normal_pos = 0 + chunk_len = 0 while True: if nc_level: - normal_pos = self.last + normal_size - mask = mask_s if self.position < normal_pos else mask_l + chunk_len = self.position - self.last + mask = mask_s if chunk_len < normal_size else mask_l if not (self.remaining > window_size and (sum & mask) and not (self.eof and self.remaining <= window_size)): break p = self.data + self.position - stop_at = p + self.remaining - window_size + span = self.remaining - window_size - if nc_level and self.position < normal_pos: - # do not scan past the strict->loose transition; re-evaluate the mask there - nc_stop = self.data + normal_pos - if nc_stop < stop_at: - stop_at = nc_stop + if nc_level and chunk_len < normal_size and normal_size - chunk_len < span: + # do not scan past the strict->loose transition; re-evaluate the mask there. + # This compares lengths, not positions: normal_size is a caller-given size_t + # and can be anything, self.last + normal_size might not even be representable. + span = normal_size - chunk_len - span = stop_at - p with nogil: did_bytes = bz64_scan(self.table, self.table_rot, p, p + window_size, span, &sum, mask, kernel_id) diff --git a/src/borg/testsuite/chunkers/fastcdc_test.py b/src/borg/testsuite/chunkers/fastcdc_test.py index 058d9f8474..d668d09555 100644 --- a/src/borg/testsuite/chunkers/fastcdc_test.py +++ b/src/borg/testsuite/chunkers/fastcdc_test.py @@ -295,6 +295,28 @@ def test_chunkers_reject_invalid_params(algo, params): get_chunker(algo, *params, sparse=False) +def test_huge_normal_size(): + # A normal_size that no chunk can reach means that the strict mask is used throughout, + # which is the same as a single-mask chunker with nc_level more mask bits. + # normal_size is a size_t and may be anything up to SIZE_MAX: the chunkers must not + # derive a buffer position from it, that would wrap around and scan outside the buffer. + from ...chunkers import ChunkerBuzHash64, ChunkerToeplitzAES + + huge = sys.maxsize * 2 + 1 - 4096 # close to SIZE_MAX + data = os.urandom(1024 * 1024) + + def sizes(chunker): + return [c.meta["size"] for c in chunker.chunkify(BytesIO(data))] + + assert sizes(ChunkerFastCDC(key0, 10, 16, 12, 2, normal_size=huge)) == sizes(ChunkerFastCDC(key0, 10, 16, 14, 0)) + assert sizes(ChunkerBuzHash64(key0, 10, 16, 12, 4095, 2, normal_size=huge)) == sizes( + ChunkerBuzHash64(key0, 10, 16, 14, 4095, 0) + ) + assert sizes(ChunkerToeplitzAES(key0, 10, 16, 12, 2, normal_size=huge)) == sizes( + ChunkerToeplitzAES(key0, 10, 16, 14, 0) + ) + + @pytest.mark.parametrize("envvar", ["BORG_FASTCDC_KERNEL", "BORG_BUZHASH64_KERNEL", "BORG_AES_CHUNKER_KERNEL"]) def test_kernel_env_rejects_unusable(envvar, monkeypatch): # A kernel that cannot run here must fail loudly instead of silently From e71d1e435aac50f1d776ebb681b3ef72e46da71a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 9 Nov 2026 22:00:20 +0100 Subject: [PATCH 4/5] AES chunkers: declare the tables parameter of *_new as in the headers The headers declare it as "const uint64_t *tables", the definitions said "const uint64_t tables[N * 256]". That is the same type, but gcc >= 11 warns about the mismatched bound (-Warray-parameter, part of -Wall). Co-Authored-By: Claude Fable 5.2 --- src/borg/chunkers/goldilocks_aes_impl.c | 2 +- src/borg/chunkers/rabin_aes_impl.c | 2 +- src/borg/chunkers/toeplitz_aes_impl.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/borg/chunkers/goldilocks_aes_impl.c b/src/borg/chunkers/goldilocks_aes_impl.c index e3868a9a9b..0bdd561f55 100644 --- a/src/borg/chunkers/goldilocks_aes_impl.c +++ b/src/borg/chunkers/goldilocks_aes_impl.c @@ -171,7 +171,7 @@ uint64_t gl_digest64(const GL_CTX *c, const uint8_t *q) #define PH_DIGEST64 gl_digest64 #include "phte_scan.h" -GL_CTX *gl_new(const uint64_t tables[GL_TABLES * 256], uint64_t k1, uint64_t k2, +GL_CTX *gl_new(const uint64_t *tables, uint64_t k1, uint64_t k2, const uint8_t aes_key[16], int kernel) { GL_CTX *c = calloc(1, sizeof(GL_CTX)); diff --git a/src/borg/chunkers/rabin_aes_impl.c b/src/borg/chunkers/rabin_aes_impl.c index af3ab6d1b1..e8481b0dff 100644 --- a/src/borg/chunkers/rabin_aes_impl.c +++ b/src/borg/chunkers/rabin_aes_impl.c @@ -66,7 +66,7 @@ uint64_t ra_digest64(const RA_CTX *c, const uint8_t *q) #define PH_DIGEST64 ra_digest64 #include "phte_scan.h" -RA_CTX *ra_new(const uint64_t tables[RA_TABLES * 256], const uint8_t aes_key[16], int kernel) +RA_CTX *ra_new(const uint64_t *tables, const uint8_t aes_key[16], int kernel) { RA_CTX *c = calloc(1, sizeof(RA_CTX)); if (c == NULL) diff --git a/src/borg/chunkers/toeplitz_aes_impl.c b/src/borg/chunkers/toeplitz_aes_impl.c index 36797d60d3..433a12adfe 100644 --- a/src/borg/chunkers/toeplitz_aes_impl.c +++ b/src/borg/chunkers/toeplitz_aes_impl.c @@ -84,7 +84,7 @@ uint64_t tp_digest64(const TP_CTX *c, const uint8_t *q) #define PH_DIGEST64 tp_digest64 #include "phte_scan.h" -TP_CTX *tp_new(const uint64_t tables[TP_TABLES * 256], const uint8_t aes_key[16], int kernel) +TP_CTX *tp_new(const uint64_t *tables, const uint8_t aes_key[16], int kernel) { TP_CTX *c = calloc(1, sizeof(TP_CTX)); if (c == NULL) From 94ac75b19b67875b953c69a10d390bcf46bd65c0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 9 Nov 2026 22:00:29 +0100 Subject: [PATCH 5/5] chunkers: fix stale comments, remove unreachable NOTBUILT branches - phte_core.h said that everything in it is static, but it also defines the three phte_kernel_*() functions with external linkage. - fc_kernel_select() / bz64_kernel_select() had a "compiler too old" branch for avx512 under #ifndef FC_KIND_512 / BZ_KIND_512, inside a block that always defines these macros. Remove it and say in the headers that *_KSEL_NOTBUILT is never returned by these two selectors. - ChunkerParams: there are three UHF-then-PRF chunkers, not "both". Co-Authored-By: Claude Fable 5.2 --- src/borg/chunkers/buzhash64_impl.c | 4 ---- src/borg/chunkers/buzhash64_impl.h | 2 +- src/borg/chunkers/fastcdc_impl.c | 4 ---- src/borg/chunkers/fastcdc_impl.h | 11 +++++++---- src/borg/chunkers/phte_core.h | 9 ++++++--- src/borg/helpers/parseformat.py | 2 +- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/borg/chunkers/buzhash64_impl.c b/src/borg/chunkers/buzhash64_impl.c index 321e9635fc..9b1dca6d34 100644 --- a/src/borg/chunkers/buzhash64_impl.c +++ b/src/borg/chunkers/buzhash64_impl.c @@ -428,14 +428,10 @@ int bz64_kernel_select(const char *name, int *out_id) return BZ_KSEL_OK; } if (strcmp(name, "avx512") == 0) { -#ifndef BZ_KIND_512 - return BZ_KSEL_NOTBUILT; /* compiler too old for the target attribute */ -#else if (!__builtin_cpu_supports("avx512f")) return BZ_KSEL_NOCPU; *out_id = BZ_K_VECTOR512; return BZ_KSEL_OK; -#endif } #endif return BZ_KSEL_UNKNOWN; diff --git a/src/borg/chunkers/buzhash64_impl.h b/src/borg/chunkers/buzhash64_impl.h index d995b07d5b..ae04eeba4b 100644 --- a/src/borg/chunkers/buzhash64_impl.h +++ b/src/borg/chunkers/buzhash64_impl.h @@ -32,7 +32,7 @@ /* Results of bz64_kernel_select(). */ #define BZ_KSEL_OK 0 #define BZ_KSEL_UNKNOWN 1 /* not a kernel name on this platform */ -#define BZ_KSEL_NOTBUILT 2 /* known, but not compiled into this binary */ +#define BZ_KSEL_NOTBUILT 2 /* known, but not compiled into this binary (never returned, see fastcdc_impl.h) */ #define BZ_KSEL_NOCPU 3 /* known and built, but this CPU cannot run it */ /* Resolve a kernel name for this build; see fc_kernel_select(). */ diff --git a/src/borg/chunkers/fastcdc_impl.c b/src/borg/chunkers/fastcdc_impl.c index 9209487c43..14bff3b3ed 100644 --- a/src/borg/chunkers/fastcdc_impl.c +++ b/src/borg/chunkers/fastcdc_impl.c @@ -409,14 +409,10 @@ int fc_kernel_select(const char *name, int *out_id) return FC_KSEL_OK; } if (strcmp(name, "avx512") == 0) { -#ifndef FC_KIND_512 - return FC_KSEL_NOTBUILT; /* compiler too old for the target attribute */ -#else if (!__builtin_cpu_supports("avx512f")) return FC_KSEL_NOCPU; *out_id = FC_K_VECTOR512; return FC_KSEL_OK; -#endif } #endif return FC_KSEL_UNKNOWN; diff --git a/src/borg/chunkers/fastcdc_impl.h b/src/borg/chunkers/fastcdc_impl.h index de72227ca4..e4e918780d 100644 --- a/src/borg/chunkers/fastcdc_impl.h +++ b/src/borg/chunkers/fastcdc_impl.h @@ -28,13 +28,16 @@ /* Results of fc_kernel_select(). */ #define FC_KSEL_OK 0 #define FC_KSEL_UNKNOWN 1 /* not a kernel name on this platform */ -#define FC_KSEL_NOTBUILT 2 /* known, but not compiled into this binary */ +#define FC_KSEL_NOTBUILT 2 /* known, but not compiled into this binary (see below) */ #define FC_KSEL_NOCPU 3 /* known and built, but this CPU cannot run it */ /* Resolve a kernel name for this build. On FC_KSEL_OK the id is stored in - * *out_id, otherwise *out_id is left alone. The three failures are kept apart - * because they need different fixes: a typo, too old a compiler, or the wrong - * CPU. */ + * *out_id, otherwise *out_id is left alone. The failures are kept apart + * because they need different fixes: a typo or the wrong CPU. + * FC_KSEL_NOTBUILT (too old a compiler) is never returned: a compiler that can + * build this file at all builds all of a platform's kernels. The code exists + * because the kernel selectors share their result codes (see kernel_env.py) + * and the AES chunkers' selector does return it. */ int fc_kernel_select(const char *name, int *out_id); /* Comma-separated list of the kernel names this build accepts, for error diff --git a/src/borg/chunkers/phte_core.h b/src/borg/chunkers/phte_core.h index 1df426b221..b8cb1be3e3 100644 --- a/src/borg/chunkers/phte_core.h +++ b/src/borg/chunkers/phte_core.h @@ -5,9 +5,12 @@ * context plumbing are identical. This header holds everything that does not * depend on the rolling hash; phte_scan.h holds the scan template that does. * - * Everything here is static (or static inline): each kernel is a separate - * extension module and gets its own copy, so there is no cross-kernel call, - * no indirection and no code-generation difference versus hand-copied code. + * Everything here is static (or static inline), except for the three + * phte_kernel_*() selection functions declared in phte_kernel.h, which the + * Cython modules call: each kernel is a separate extension module and gets its + * own copy, so there is no cross-kernel call, no indirection and no + * code-generation difference versus hand-copied code. For the same reason, + * two of the kernels can not be linked into one binary (duplicate symbols). */ #ifndef BORG_PHTE_CORE_H diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index 889472770d..fa4dfed3cf 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -376,7 +376,7 @@ def ChunkerParams(s): return CH_FASTCDC, chunk_min, chunk_max, chunk_mask, nc_level if algo in (CH_RABIN_AES, CH_GOLDILOCKS_AES, CH_TOEPLITZ_AES): # , chunk_min, chunk_max, chunk_mask, nc_level - # both UHF-then-PRF chunkers have a fixed 64-byte window, so there is no window_size field. + # the UHF-then-PRF chunkers have a fixed 64-byte window, so there is no window_size field. # nc_level is required; use nc_level 0 to disable normalized chunking. if count != 5: raise ArgumentTypeError(f"{algo} chunker params must be: {algo},chunk_min,chunk_max,chunk_mask,nc_level")