Repository navigation
Fix id_type to uint32_t in vendored darts.h; add legacy 64-bit OCD reader with full validation - #1373
Merged
Merged
Conversation
…ader with full validation
Inspection of OPENCCDARTS1 files produced by opencc_dict 1.3.2 on 64-bit
platforms confirms that the high 32 bits of every unit are zero, so the
logical double-array structure is identical to the 32-bit layout. Switching
id_type from size_t to uint32_t is therefore safe, makes all future output
platform-independent, and keeps the vendored copy as close to upstream
darts-clone 0.32h as possible (one typedef change plus one added include).
A compatibility layer in DartsDict auto-detects old 64-bit files at load
time via a zero-bytes heuristic and reads them without altering the magic
string or on-disk format.
Detailed Changes:
- **deps/darts-clone-0.32h/include/darts.h**:
- Add `#include <cstdint>`.
- Change `typedef size_t id_type` → `typedef uint32_t id_type`; all other
code is left untouched to stay in sync with upstream.
- **src/DartsDict.cpp**:
- Add `LegacyUnit64` struct mirroring `DoubleArrayUnit` bit operations on
`uint64_t` units.
- Add `Legacy64ExactMatch` mirroring `exactMatchSearch` with the correct
three-way `pos ^= offset ^ c` traversal and `pos ^ offset` leaf access.
- Add `Legacy64PrefixSearch` mirroring `commonPrefixSearch`: continues
traversal to `maxLen` even after the result buffer is full, always
returning the true total match count so the caller's re-search path can
allocate a correctly-sized buffer and retrieve the true longest match.
- Add `ValidateLegacy64` mirroring `DoubleArrayImpl::validate` to check
root sanity, offset bounds, and leaf value bounds before any traversal.
- `NewFromFile`: read an 8-byte probe after the magic; bytes [4..7] all
zero → old 64-bit layout (uint64_t dartsSize + 8-byte units, loaded into
`legacyArray64` and validated); otherwise → 32-bit layout (uint32_t
dartsSize + 4-byte units, seek back 4, existing path).
- `MatchPrefix` legacy branch: restructure re-search to read `maxVal` from
`rematchedResults`, not the original stack-allocated `results[64]` (was
out-of-bounds and returned the wrong longest match).
- `SerializeToFile`: write dartsSize as `uint32_t` instead of `size_t` so
output is platform-independent; throw `InvalidFormat` early when called
on a legacy-loaded dict whose `doubleArray` is nullptr.
- **src/DartsDictTest.cpp**:
- Update `WriteMalformedDartsFile` / `CorruptDartsRootUnit` /
`CorruptFirstDartsValue` to use `uint32_t` sizes.
- Add `SerializeAsLegacy64` helper that zero-extends each 32-bit unit to
64 bits to reproduce old build output.
- Add `LoadsLegacy64bitFormat`: round-trip correctness via `TestDict` and
an explicit Chinese-key lookup.
- Add `RejectsInvalidLegacy64Root`: zeroed root unit must throw.
- Add `Legacy64SerializeToFileThrows`: `SerializeToFile` on a
legacy-loaded dict must throw `InvalidFormat` rather than segfault.
- Add `Legacy64PrefixSearchBeyond64Matches`: 65-entry purely-nested dict
round-tripped through legacy 64-bit OCD; `MatchPrefix` must return "v65"
(the true longest match), not "v64" as it did when traversal stopped as
soon as the result buffer was full.
- Total DartsDictTest count: 12.
frankslin
added a commit
that referenced
this pull request
Jul 9, 2026
…t OCD output (#1414) OCD files embed a BinaryDict section whose integer fields were written as native size_t, so 32-bit and 64-bit builds produced different bytes for the same dictionary, and files did not load across word sizes. All fields are now written as uint32_t (matching what 32-bit builds already produced), and the deserializer detects the legacy 8-byte layout, so files written by old 64-bit builds still load, on any platform. Investigation: - PR #1372/#1373 made the darts trie section word-size independent: vendored darts.h id_type was fixed to uint32_t, the dartsSize field became uint32_t, and a legacy 64-bit reader with validation was added. Those changes covered only the darts array; the trailing BinaryDict section (key/value buffers plus offset tables) kept writing native size_t fields, a layout that dates back to 641bfe9 which deliberately chose size_t to silence MSVC x64 warnings. - As a result, .ocd files produced on 32-bit platforms (including wasm32 and armv7) differed byte-wise from 64-bit output for the same input, and could not be loaded across word sizes at all: BinaryDict::NewFromFile read sizeof(size_t) fields of whatever platform was running. - The existing legacy-format tests could not catch this because SerializeAsLegacy64 copied the BinaryDict section verbatim. On a 64-bit host the fixture carried 8-byte fields and the reader consumed 8-byte fields, so the test was self-consistent yet word-size dependent; the same fixture would have failed on a 32-bit host. - Layout detection reuses the high-bytes-zero probe already proven for dartsSize: an 8-byte numItems always has zero bytes at [4..7] for any realistic dictionary, while the fixed-width layout places keyTotalLength there, which is non-zero whenever the dictionary has entries. The only all-zero-probe fixed-width file is an empty dictionary, exactly 12 bytes long, which is disambiguated by size. - Endianness is intentionally out of scope: the whole OCD/OCD2 family is native-endian (darts array, marisa trie, SerializedValues), so this change keeps BinaryDict consistent with the rest of the format on big-endian platforms rather than fixing one section in isolation. Verification: - bazel test //src/... : all 18 targets pass, including the new binary_dict_test cases (fixed-width layout size assertion, empty-dict round trip, legacy 64-bit loading, out-of-bounds rejection in both layouts) and darts_dict_test with the widened legacy fixture. - dict_converter round trip text -> ocd -> text reproduces the input, and a hex dump of the generated .ocd confirms 4-byte numItems/keyTotalLength/ valueTotalLength and per-entry fields after the darts section. Detailed Changes: - **src/BinaryDict.cpp**: - Write all integer fields as uint32_t; reject values above UINT32_MAX. - Detect the field width on load via DetectFieldWidth and parse both layouts with a single bounds-checked reader. - Route NewFromFile through NewFromBuffer to share the detection logic. - **src/DartsDict.cpp**: - Update layout-detection comments; no behavior change. - **Tests**: - Make crafted BinaryDict files explicit about field width instead of depending on the host size_t, and cover both layouts. - Assert the serialized layout size is fully determined by content. - Cover the empty-dictionary round trip and legacy 64-bit loading. - Widen the BinaryDict tail in SerializeAsLegacy64 so legacy OCD fixtures match real files from old 64-bit builds end to end.
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.
Inspection of OPENCCDARTS1 files produced by opencc_dict 1.3.2 on 64-bit platforms confirms that the high 32 bits of every unit are zero, so the logical double-array structure is identical to the 32-bit layout. Switching id_type from size_t to uint32_t is therefore safe, makes all future output platform-independent, and keeps the vendored copy as close to upstream darts-clone 0.32h as possible (one typedef change plus one added include). A compatibility layer in DartsDict auto-detects old 64-bit files at load time via a zero-bytes heuristic and reads them without altering the magic string or on-disk format.
Detailed Changes:
#include <cstdint>.typedef size_t id_type→typedef uint32_t id_type; all other code is left untouched to stay in sync with upstream.LegacyUnit64struct mirroringDoubleArrayUnitbit operations onuint64_tunits.Legacy64ExactMatchmirroringexactMatchSearchwith the correct three-waypos ^= offset ^ ctraversal andpos ^ offsetleaf access.Legacy64PrefixSearchmirroringcommonPrefixSearch: continues traversal tomaxLeneven after the result buffer is full, always returning the true total match count so the caller's re-search path can allocate a correctly-sized buffer and retrieve the true longest match.ValidateLegacy64mirroringDoubleArrayImpl::validateto check root sanity, offset bounds, and leaf value bounds before any traversal.NewFromFile: read an 8-byte probe after the magic; bytes [4..7] all zero → old 64-bit layout (uint64_t dartsSize + 8-byte units, loaded intolegacyArray64and validated); otherwise → 32-bit layout (uint32_t dartsSize + 4-byte units, seek back 4, existing path).MatchPrefixlegacy branch: restructure re-search to readmaxValfromrematchedResults, not the original stack-allocatedresults[64](was out-of-bounds and returned the wrong longest match).SerializeToFile: write dartsSize asuint32_tinstead ofsize_tso output is platform-independent; throwInvalidFormatearly when called on a legacy-loaded dict whosedoubleArrayis nullptr.WriteMalformedDartsFile/CorruptDartsRootUnit/CorruptFirstDartsValueto useuint32_tsizes.SerializeAsLegacy64helper that zero-extends each 32-bit unit to 64 bits to reproduce old build output.LoadsLegacy64bitFormat: round-trip correctness viaTestDictand an explicit Chinese-key lookup.RejectsInvalidLegacy64Root: zeroed root unit must throw.Legacy64SerializeToFileThrows:SerializeToFileon a legacy-loaded dict must throwInvalidFormatrather than segfault.Legacy64PrefixSearchBeyond64Matches: 65-entry purely-nested dict round-tripped through legacy 64-bit OCD;MatchPrefixmust return "v65" (the true longest match), not "v64" as it did when traversal stopped as soon as the result buffer was full.