diff --git a/.agents/roadmap_v1.md b/.agents/roadmap_v1.md index 6bc28960d..7325ad3ca 100644 --- a/.agents/roadmap_v1.md +++ b/.agents/roadmap_v1.md @@ -164,6 +164,8 @@ issue is not yet placed. Keyed record: update in place, never append. | [#670](https://github.com/mudler/vllm.cpp/issues/670) | `BACKEND-TENSTORRENT-MISTRAL` | Tenstorrent: allowlist `MistralForCausalLM` and gate it on-device; goldens are `transformers`-teacher-forced because vLLM has no TT backend, spec [`tenstorrent-mistral.md`](specs/tenstorrent-mistral.md) | feature | | [#773](https://github.com/mudler/vllm.cpp/issues/773) | `GATE-FORK-ANCESTRY` | `check-pr-size` and `check-commit-trailers` abort on every fork PR because the base SHA stops being an ancestor once main moves, so CI has never validated an outside contributor's trailers, spec [`gate-fork-ancestry.md`](specs/gate-fork-ancestry.md) | bug | | [#774](https://github.com/mudler/vllm.cpp/issues/774) | `ENG-RELEASE-WINDOWS` | `check-windows-portability.py:1710` asserted the MSVC warning policy with `token in warnings`, and `"/WX" in "/WX-"` is `True` — `/WX-` is MSVC's spelling for DISABLE warnings-as-errors, so the gate was blind to its own inversion. Measured on PR #640 commit `74ba3823f`, which shipped `/WX-` on the CXX arm while the only bare `/WX` left was on `$` — Objective-C++, the Metal backend, which never compiles under MSVC. Two further blindnesses fell out of the same `in`: `/W44996` answers for `/W4`, and `CMakeLists.txt:30`'s `#` comment satisfies the whole policy on its own. Repaired to a token-boundary match over the flags that reach the C/C++ compile, plus the negating spellings `/WX-` `/W0` `/w`; spec [`windows-msvc-warning-policy-tokens.md`](specs/windows-msvc-warning-policy-tokens.md) | bug | +| [#772](https://github.com/mudler/vllm.cpp/issues/772) | `FIX-UNALIGNED-LOADERS-772` | **FIXED 2026-08-14, `row/FIX-UNALIGNED-LOADERS-772`, spec [`unaligned-safetensors-loaders.md`](specs/unaligned-safetensors-loaders.md).** FOURTH recurrence of the unaligned-safetensors class after [#301](https://github.com/mudler/vllm.cpp/issues/301), [#627](https://github.com/mudler/vllm.cpp/issues/627) and [#674](https://github.com/mudler/vllm.cpp/issues/674): `voxtral.cpp:51`/`:344`, `qwen3_vl.cpp:78` and `qwen3_5_mtp.cpp:71` each formed a `const uint16_t*` over an mmap'd safetensors payload, whose offset (`8 + + `) carries no alignment guarantee. TWO OF THE FOUR ARE INVISIBLE TO UBSAN BY CONSTRUCTION — `PermuteQKBf16` and `CopyRawNK` launder every access through `std::memcpy`, so `-fsanitize=alignment` never fires, and all three earlier recurrences were UBSan finds. Fixed by `vt::LoadUnaligned` at the two scalar-load sites and by `unsigned char` byte arithmetic at the two bulk-copy sites; `minimax_h3_vae_loader.cpp:96-101`, which hand-rolled the same `memcpy` instead of calling the seam, folded in. Gated by `test_loader_unaligned_offsets`, which FORCES an odd payload offset with one space of JSON header padding and REQUIREs the mapped-address parity | bug | +| [#807](https://github.com/mudler/vllm.cpp/issues/807) | `FIX-UNALIGNED-LOADERS-772` | **FIXED IN FLOW 2026-08-14, `row/FIX-UNALIGNED-LOADERS-772`.** `docs/FEATURES.md:83-84` carried the row `Safetensors direct load, no conversion` TWICE under the identical key, with two rows that did not agree: one implied the unaligned-read class was handled, the other named three loaders still owed. `fc903b8dd` (the #674 fix) ADDED a row while the second already existed, and adding a row next to an edit of a different row merges cleanly — the "two relocations auto-merge into a duplicate" shape on a keyed projection. Nothing checks `FEATURES.md` for duplicate row keys; `check-public-doc-tables.py` validates structure, not key uniqueness. Collapsed into one accurate row by the #772 fix, which is what made the second row's claim false | bug | | [#238](https://github.com/mudler/vllm.cpp/issues/238) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprobs_mode`: three of four modes are runtime-refused stubs | bug | | [#264](https://github.com/mudler/vllm.cpp/issues/264) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprob_token_ids`: generative scoring over an explicit token set is unported | feature | | [#365](https://github.com/mudler/vllm.cpp/issues/365) | `PERF-27B-DENSE-MARLIN-GATEUP` | 27B gap decomposed vs vLLM's own decode profile; dense W4A16 MLP bypassed the fused gate_up seam (spec `specs/perf-27b-dense-marlin-gateup.md`) | bug | diff --git a/.agents/specs/unaligned-safetensors-loaders.md b/.agents/specs/unaligned-safetensors-loaders.md new file mode 100644 index 000000000..181d71260 --- /dev/null +++ b/.agents/specs/unaligned-safetensors-loaders.md @@ -0,0 +1,209 @@ +# Unaligned safetensors loads — the fourth recurrence, and the two UBSan cannot see + +Identity: `FIX-UNALIGNED-LOADERS-772` + +Issue: [#772](https://github.com/mudler/vllm.cpp/issues/772) + +Class predecessors: [#301](https://github.com/mudler/vllm.cpp/issues/301) +(closed; it left the `vt::LoadUnaligned` seam), +[#627](https://github.com/mudler/vllm.cpp/issues/627) (`qwen3_5_weights.cpp`; +rescoped to the grep-able checker this file's finding argues for), and +[#674](https://github.com/mudler/vllm.cpp/issues/674) / PR #688 +(`ltx2_loader.cpp`, whose forced-odd-offset case is the pattern mirrored here). + +Status: `ACTIVE`. Base `af026e5241e16e1d2a89da001b978ffcb3e0f634`. + +## Scope + +Remove the four remaining `reinterpret_cast(.data)` formations named in #772, and route +`minimax_h3_vae_loader.cpp:96-101` — which hand-rolls the same `std::memcpy` +instead of calling the shared seam — through `vt::LoadUnaligned`. Add the gate +that forces an odd payload offset for each of them. + +Change no loaded value, no shape rule, no dtype rule, no error message and no +public behavior. The only surface addition is two declarations in a new INTERNAL +header under `src/`, without which the gate cannot drive the production code +(see Design). Nothing under `include/` is touched, so the public ABI is +unchanged. + +Explicitly excluded: `scripts/check-pr-size.py`, +`scripts/check-commit-trailers.py` (PR #782), +`scripts/check-windows-portability.py` (#774), `scripts/audit-live-rows.py`, +`.github/workflows/ci.yml` (#726); the checker #627 now proposes, which is that +issue's own work and needs its own spec; and the two pre-existing `sanitize-cpu` +reds, [#775](https://github.com/mudler/vllm.cpp/issues/775) and +[#776](https://github.com/mudler/vllm.cpp/issues/776). + +## Observed baseline and root cause + +A safetensors tensor begins at `8 + + `. None of those three terms is required to be even, so a BF16 +tensor that follows an odd-length tensor — or that sits behind a header of odd +length — starts on an odd byte. That is an ordinary file, not a corrupt one. + +Four loaders formed a `const uint16_t*` over that address: + +| Site | Shape | Sanitizer-visible? | +|---|---|---| +| `voxtral.cpp:51` `StBf16ToF32` | cast, then `src[i]` | YES | +| `voxtral.cpp:344` `PermuteQKBf16` | cast, then row address + `memcpy` | **NO** | +| `qwen3_vl.cpp:78` `LoadVisionF32` | cast, then `p[i]` in `Bf16BitsToF32` | YES | +| `qwen3_5_mtp.cpp:71` `CopyRawNK` | cast, `+ offset`, then `memcpy` | **NO** | + +**The two `NO` rows are the finding.** Forming and advancing a misaligned +`uint16_t*` is undefined in its own right, but both of them then launder every +access through `std::memcpy`, which reads bytes — so `-fsanitize=alignment` +never fires. All three earlier recurrences of this class were found by UBSan. +These two would have survived every one of those sweeps, and would survive the +next one. That is the argument for #627's checker, and it is why the gate below +is a value gate rather than another sanitizer run. + +All four were latent by luck: the offsets happen to be even in the fixtures we +run. That is a property of the files we load, not of the code. + +## Design + +Two shapes of fix, because the two shapes of defect are different. + +**Scalar loads** (`StBf16ToF32`, `LoadVisionF32`) go through +`vt::LoadUnaligned` over an `unsigned char` base — the seam #301 left, +already used by `dense_loaders::TransposeBf16` and by +`minimax_h3_vae_loader.cpp`. It is free: verified during the #688 review that at +`-O2` it compiles instruction-for-instruction identically to the raw cast +(`movzwl (%rdi,%rax,2)`), emits no `memcpy` call, and is exhaustively +value-correct over all 65536 bit patterns × byte offsets 0-7. + +**Bulk copies** (`PermuteQKBf16`, `CopyRawNK`) drop the typed pointer entirely +and do the arithmetic in `unsigned char`. `vt::LoadUnaligned` would be the wrong +tool: the payload is already moved by `memcpy`, and there is no scalar to load. +The one thing this rewrite must not lose is the `sizeof(uint16_t)` the pointer +type used to supply — `CopyRawNK`'s `offset` counts BF16 **elements**, not bytes +— so that factor is what the gate's mutation arm targets. + +`minimax_h3_vae_loader.cpp` is **not** a defect: it already did the byte-wise +load and already carried the reason in prose. It is in scope because it +hand-rolled `std::memcpy` where the shared seam exists, which is the parallel +path AGENTS.md's shared-seam rule is about — and it is the file that carried the +repair while three other loaders shipped the cast anyway. + +**Why two declarations exist at all, and why they are INTERNAL.** +`StBf16ToF32` and `PermuteQKBf16` live in an anonymous namespace, and the only +entry point above them is `LoadVoxtralWeights`, which is unreachable at test +scale: `VoxtralEncoderConfig()` is fixed at 32 layers of `d_model` 1280 / `ffn` +5120, so a synthetic checkpoint that satisfies it is ~1.2 GiB. The alternative to +declaring them is a gate that tests a copy of the code rather than the code, +which proves nothing. + +They go in `src/vllm/model_executor/models/voxtral_loader_internal.h`, **not** +`include/`. They are loader internals, not a shipped capability, and putting them +in the public tree would overstate what they are — AGENTS.md's ABI rule is about +capabilities reaching `include/vllm.h`, and these reach nothing. The tree already +has this shape: the NemotronH MIXED_PRECISION resolver header is internal until a +loader consumes it, and its two suites reach it through +`-I${CMAKE_SOURCE_DIR}/src` (`tests/CMakeLists.txt:80-96`). The functions move out +of the anonymous namespace unchanged and gain a `Voxtral` prefix, since the bare +names are generic enough to collide. + +The other three sites need no such change: `LoadQwen3VLVisionWeights`, +`LoadQwen3_5MTP` and `LoadMiniMaxH3AudioVaeWeights` are already public and are +reachable with a checkpoint of a few hundred bytes. + +## Tests and RED evidence + +One new suite, `tests/vllm/models/test_loader_unaligned_offsets.cpp`, with one +case per site, mirroring `test_ltx2_video.cpp`'s "ODD safetensors payload offset +(#674)" case: + +- The safetensors file is written by the test, so the header length — and with + it the payload parity — is ours to choose. Every BF16 tensor is an even number + of bytes, so the parity of the first payload byte is the parity of every + tensor in the file, and one space of padding inside the counted JSON header + flips all of them at once. Trailing whitespace is legal JSON, and padding the + header is exactly how real writers align their payloads. +- The file is written **both** ways and whichever lands odd is kept, so no case + depends on the exact length of the generated JSON. `REQUIRE` proves the two + spellings really differ in parity, so padding could not become inert silently. +- The mapped **address** parity is `REQUIRE`d, not inferred. A fixture edit that + makes the address even fails the REQUIRE instead of passing while covering + nothing. +- Every case checks the loaded **values** against the fixture's own bit + patterns, not merely that the load returned — because for two of the four + sites the values are the only witness there is. + +RED, at base + the `voxtral.h` export and nothing else, under +`-DVLLM_CPP_SANITIZE='address,undefined'`: + +- `voxtral.cpp` — `runtime error: load of misaligned address … for type 'const + short unsigned int', which requires 2 byte alignment`, in + `vllm::VoxtralStBf16ToF32`. +- `qwen3_vl.cpp:58` — the same diagnostic, in `Bf16BitsToF32` under + `LoadVisionF32`. +- `PermuteQKBf16`, `CopyRawNK` and the minimax loop — **pass**, which is the + point being pinned: the sanitizer cannot see them. + +Because the lane builds with `-fno-sanitize-recover=all`, the first finding +aborts the process, so each case is run in its own process to witness its own +site. + +For the two sanitizer-invisible sites the RED is a **mutation**: dropping the +`* sizeof(uint16_t)` from the byte arithmetic — the exact mistake this rewrite +can make — must red the case. + +## Gates + +1. RED: the two sanitizer-visible cases report the misaligned-load diagnostic at + base; the two invisible ones pass, and that asymmetry is recorded. +2. GREEN: all five cases pass under `address,undefined` with no new finding. +3. Mutation: `* sizeof(uint16_t)` dropped at `qwen3_5_mtp.cpp` and at + `voxtral.cpp` must red their cases. Restore verified by checksum. +4. Inertness: the object code of the four changed functions, compiled at the + project's normal optimization level, before and after. +5. Full `ctest` on a clean CPU build, and `scripts/agent-preflight.sh --staged`, + and `python3 -m pytest tests/scripts/`. + +Baseline to subtract: `windows-msvc-*` is PR-only and red on every PR (#584); +`test_cpu_threadpool` flakes under load (#631); `sanitize-cpu` on `main` is red +on #775 and #776; `agent-record` fails via `audit-live-rows` (#726). + +## Risks and stop conditions + +The risk is a rewrite that is UB-clean and quietly wrong — an element offset +read as a byte offset loads the wrong expert's rows and still returns a +plausible tensor. Gate arm 3 exists for exactly that, and every expectation in +the suite is derived from the fixture's own stacked layout so a confusion +between neighbouring slices shows up as specific wrong values rather than as +"something differs". + +Stop with `NEEDS_DECISION` if any loaded value moves anywhere: this change is +required to be inert on every already-aligned checkpoint, which is all of them +today. + +## Outcome + +**The two shapes of defect need two different fixes, and conflating them would +have been a bug.** #772 proposes `vt::LoadUnaligned` for all four sites. That is +right for the two scalar loads and wrong for the two bulk copies: +`PermuteQKBf16` copies a whole row per iteration and `CopyRawNK` copies a whole +`[N,K]` slice, so there is no scalar to load and wrapping them would have meant +rewriting a `memcpy` as an element loop. Both were fixed by moving the pointer +arithmetic into `unsigned char` instead, which is what the issue itself +prescribes for `CopyRawNK` alone. + +**The asymmetry #772 predicted was reproduced exactly.** At base, the two +sanitizer-visible sites reported `load of misaligned address … requires 2 byte +alignment` and the two laundered ones passed clean — on the identical fixture, +at the identical odd offset, in the same binary. The class is provably not fully +discoverable by the lane that caught its first three recurrences. + +**The mutation arm is not vacuous.** Dropping `* sizeof(uint16_t)` from +`CopyRawNK`'s byte advance red 48 of 93 assertions in the MTP case; dropping it +from `PermuteQKBf16`'s row address red 21 of 33. Both reverted to a byte-identical +tree, verified by `md5sum` rather than by `git status`. + +**What could not be proved here.** The 27B / 35B / Coder token gates #627 asks +for need the GB10 box and staged checkpoints, and other work was live on that +GPU; they are not claimed. What is claimed instead is object-code inertness +(gate arm 4) plus the full CPU suite, which is the strongest evidence available +without taking the GPU. Voxtral's own e2e gate additionally requires a +checkpoint that is not staged here and exits 77. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 2c0eb3650..11a6309d4 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -80,8 +80,7 @@ are our reading of their documented behavior, not measurements. | Merged fp8 projection folds per-column alpha in the GEMM epilogue | ◐ `VT_FP8_ALPHA_VEC_EPILOGUE`, CUDA only, default off, ungated; refuses split-K under a bf16-D equivalence claim (`claims_splitk1_premise`, default off) | n/a | n/a | n/a | | `vt::MulColVecF32` carries a bf16 store width | ✅ f32 arm byte-identical; bf16 arm rounds once; CPU + CUDA | n/a | ☐ | ☐ | | bf16 / fp16 | ✅ | ✅ | ✅ | ✅ | -| Safetensors direct load, no conversion | ✅ a payload offset carries no alignment guarantee, so reads of the mapping go byte-wise ([#674](https://github.com/mudler/vllm.cpp/issues/674) VAE loader; [#627](https://github.com/mudler/vllm.cpp/issues/627) the rest) | ✅ | ✅ | ☐ | -| Safetensors direct load, no conversion | ✅ at ANY tensor byte offset: the format aligns nothing, so the `*_weights.cpp` loaders never form a typed pointer into the mapping (#627). `voxtral.cpp`, `qwen3_vl.cpp`, `qwen3_5_mtp.cpp` still do and are OWED | ✅ | ✅ | ☐ | +| Safetensors direct load, no conversion | ✅ at ANY tensor byte offset: the format aligns nothing, so no loader forms a typed pointer into the mapping. Last three fixed by #772; a checker is still owed on #627 | ✅ | ✅ | ☐ | | Weights uploaded straight from the file mapping (no host copy first) | ◐ verbatim tensors only (37.8% of 27B BF16); arbitrary-offset reads are defined, including Laguna graph staging. Merged/transposed and merged FP4 weights still copy | ✅ | ✅ | ✅ mmap | ## Model coverage diff --git a/src/vllm/model_executor/models/minimax_h3_vae_loader.cpp b/src/vllm/model_executor/models/minimax_h3_vae_loader.cpp index 65eb78571..80a6484ea 100644 --- a/src/vllm/model_executor/models/minimax_h3_vae_loader.cpp +++ b/src/vllm/model_executor/models/minimax_h3_vae_loader.cpp @@ -32,6 +32,7 @@ #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vt/dtype.h" +#include "vt/unaligned.h" // LoadUnaligned — mmap'd payloads have no alignment guarantee namespace vllm { namespace { @@ -91,13 +92,18 @@ std::vector MiniMaxH3ReadSafetensorF32(const StTensor& tensor) { // happened to pad; the format does not require it. The cast was UB on such a // file and UBSan caught it ("load of misaligned address ... requires 2 byte // alignment") the first time a checkpoint with an odd header reached this - // path. memcpy has no alignment precondition and compiles to the same load - // where the address does happen to be aligned. + // path. `vt::LoadUnaligned` has no alignment precondition and compiles to + // the same load where the address does happen to be aligned. + // + // It is the SHARED seam for this (include/vt/unaligned.h, born of #301): + // this loop used to hand-roll its own std::memcpy, which is the parallel + // path AGENTS.md's shared-seam rule exists to prevent — and the reason the + // three loaders in #772 could ship the cast next to a file that had already + // carried the repair in prose. const auto* bytes = static_cast(tensor.data); const bool bf16 = (tensor.dtype == "BF16"); for (int64_t i = 0; i < numel; ++i) { - uint16_t bits; - std::memcpy(&bits, bytes + static_cast(i) * 2, sizeof(bits)); + const auto bits = vt::LoadUnaligned(bytes + static_cast(i) * 2); out[static_cast(i)] = bf16 ? Bf16ToF32(bits) : F16ToF32(bits); } } else { diff --git a/src/vllm/model_executor/models/qwen3_5_mtp.cpp b/src/vllm/model_executor/models/qwen3_5_mtp.cpp index fa1a8e15f..f941da5fb 100644 --- a/src/vllm/model_executor/models/qwen3_5_mtp.cpp +++ b/src/vllm/model_executor/models/qwen3_5_mtp.cpp @@ -68,7 +68,17 @@ OwnedTensor CopyRawNK(const StTensor& source, int64_t offset, int64_t n, "qwen3_5 MTP: stacked slice out of bounds for " + name); OwnedTensor out = MakeOwned(vt::DType::kBF16, {n, k}); out.nk = true; - const auto* begin = reinterpret_cast(source.data) + offset; + // Byte arithmetic, NOT `reinterpret_cast(source.data) + + // offset` (issue #772). `source.data` points into the safetensors mmap, whose + // payload offset carries no alignment guarantee, and forming — let alone + // advancing — a misaligned `uint16_t*` is undefined even though the payload is + // then moved by memcpy and never dereferenced as a uint16_t. That memcpy + // laundering is why UBSan never reported this site: it is the one of the four + // in #772 that a sanitizer sweep provably cannot find. `offset` counts BF16 + // ELEMENTS, so the byte advance carries the `sizeof(uint16_t)` the pointer + // type used to supply. + const auto* begin = static_cast(static_cast(source.data)) + + static_cast(offset) * sizeof(uint16_t); std::memcpy(out.bytes.data(), begin, out.bytes.size()); return out; } diff --git a/src/vllm/model_executor/models/qwen3_vl.cpp b/src/vllm/model_executor/models/qwen3_vl.cpp index f5cc33508..1aeadf39a 100644 --- a/src/vllm/model_executor/models/qwen3_vl.cpp +++ b/src/vllm/model_executor/models/qwen3_vl.cpp @@ -26,6 +26,7 @@ #include "vt/backend.h" #include "vt/dtype.h" #include "vt/ops.h" +#include "vt/unaligned.h" // LoadUnaligned — mmap'd payloads have no alignment guarantee namespace vllm { namespace { @@ -69,13 +70,20 @@ void RoundToBf16(std::vector& v) { } // ---- vision weight loader: model.visual.* bf16 -> host f32 (matches M2a dump) ---- +// `t.data` points into the safetensors mmap, whose payload offset carries NO +// alignment guarantee (issue #772), so the bytes are read through +// vt::LoadUnaligned rather than handed to Bf16BitsToF32 as a `const uint16_t*`. +// Bf16BitsToF32 keeps that signature for its OTHER callers, which pass +// std::vector::data() and are suitably aligned by construction. std::vector LoadVisionF32(const TensorResolver& get, const std::string& name) { const StTensor& t = get(name); VT_CHECK(t.dtype == "BF16", "qwen3-vl vision: expected BF16 for " + name); - const int64_t n = static_cast(t.nbytes / sizeof(uint16_t)); - std::vector out = - Bf16BitsToF32(reinterpret_cast(t.data), n); + const auto n = static_cast(t.nbytes / sizeof(uint16_t)); + const auto* src = static_cast(static_cast(t.data)); + std::vector out(n); + for (size_t i = 0; i < n; ++i) + out[i] = vt::BF16ToF32(vt::LoadUnaligned(src + i * 2)); MaybeReleaseSourcePages(t.data, t.nbytes); return out; } diff --git a/src/vllm/model_executor/models/voxtral.cpp b/src/vllm/model_executor/models/voxtral.cpp index cdaa8604b..95dcf3784 100644 --- a/src/vllm/model_executor/models/voxtral.cpp +++ b/src/vllm/model_executor/models/voxtral.cpp @@ -25,9 +25,11 @@ #include "vllm/model_executor/models/dense_attn_block.h" // AttnBlock, BuildStepInputs, ResidentWeight #include "vllm/model_executor/models/dense_weight_loaders.h" #include "vllm/model_executor/models/qwen3_vl_text.h" // Qwen3VLMergeMultimodal (modality-agnostic merge) +#include "vllm/model_executor/models/voxtral_loader_internal.h" // the two mmap-reading loader steps (#772) #include "vt/dtype.h" #include "vt/ops.h" #include "vt/tensor.h" +#include "vt/unaligned.h" // LoadUnaligned — mmap'd payloads have no alignment guarantee namespace vllm { namespace { @@ -45,16 +47,6 @@ using vt::Queue; using vt::Tensor; using v1::CommonAttentionMetadata; -// --- bf16 StTensor -> host f32 vector (encoder + adapter weights). -------------- -std::vector StBf16ToF32(const StTensor& t) { - VT_CHECK(t.dtype == "BF16", "voxtral: expected BF16 tensor"); - const auto* src = reinterpret_cast(t.data); - const size_t n = t.nbytes / sizeof(uint16_t); - std::vector out(n); - for (size_t i = 0; i < n; ++i) out[i] = vt::BF16ToF32(src[i]); - return out; -} - std::vector Bf16BitsToF32(const uint16_t* p, int64_t n) { std::vector o(static_cast(n)); for (int64_t i = 0; i < n; ++i) o[static_cast(i)] = vt::BF16ToF32(p[i]); @@ -301,60 +293,35 @@ void LoadEncoderWeights(const TensorResolver& get, const std::vector& emb const multimodal::WhisperAudioEncoderConfig& cfg, multimodal::WhisperAudioEncoderWeights& w) { const std::string E = "mm_whisper_embeddings.whisper_encoder."; - w.conv1_w = StBf16ToF32(get(E + "conv_layers.0.weight")); - w.conv1_b = StBf16ToF32(get(E + "conv_layers.0.bias")); - w.conv2_w = StBf16ToF32(get(E + "conv_layers.1.weight")); - w.conv2_b = StBf16ToF32(get(E + "conv_layers.1.bias")); + w.conv1_w = VoxtralStBf16ToF32(get(E + "conv_layers.0.weight")); + w.conv1_b = VoxtralStBf16ToF32(get(E + "conv_layers.0.bias")); + w.conv2_w = VoxtralStBf16ToF32(get(E + "conv_layers.1.weight")); + w.conv2_b = VoxtralStBf16ToF32(get(E + "conv_layers.1.bias")); w.embed_positions_w = embed_positions; - w.final_ln_w = StBf16ToF32(get(E + "transformer.norm.weight")); - w.final_ln_b = StBf16ToF32(get(E + "transformer.norm.bias")); + w.final_ln_w = VoxtralStBf16ToF32(get(E + "transformer.norm.weight")); + w.final_ln_b = VoxtralStBf16ToF32(get(E + "transformer.norm.bias")); w.layers.resize(static_cast(cfg.num_layers)); for (int64_t l = 0; l < cfg.num_layers; ++l) { const std::string p = E + "transformer.layers." + std::to_string(l) + "."; multimodal::WhisperEncoderLayerWeights& lw = w.layers[static_cast(l)]; - lw.attn_ln_w = StBf16ToF32(get(p + "attention_norm.weight")); - lw.attn_ln_b = StBf16ToF32(get(p + "attention_norm.bias")); - lw.final_ln_w = StBf16ToF32(get(p + "ffn_norm.weight")); - lw.final_ln_b = StBf16ToF32(get(p + "ffn_norm.bias")); - lw.q_w = StBf16ToF32(get(p + "attention.wq.weight")); - lw.q_b = StBf16ToF32(get(p + "attention.wq.bias")); - lw.k_w = StBf16ToF32(get(p + "attention.wk.weight")); // k_proj: NO bias - lw.v_w = StBf16ToF32(get(p + "attention.wv.weight")); - lw.v_b = StBf16ToF32(get(p + "attention.wv.bias")); - lw.out_w = StBf16ToF32(get(p + "attention.wo.weight")); - lw.out_b = StBf16ToF32(get(p + "attention.wo.bias")); - lw.fc1_w = StBf16ToF32(get(p + "feed_forward.w1.weight")); - lw.fc1_b = StBf16ToF32(get(p + "feed_forward.w1.bias")); - lw.fc2_w = StBf16ToF32(get(p + "feed_forward.w2.weight")); - lw.fc2_b = StBf16ToF32(get(p + "feed_forward.w2.bias")); + lw.attn_ln_w = VoxtralStBf16ToF32(get(p + "attention_norm.weight")); + lw.attn_ln_b = VoxtralStBf16ToF32(get(p + "attention_norm.bias")); + lw.final_ln_w = VoxtralStBf16ToF32(get(p + "ffn_norm.weight")); + lw.final_ln_b = VoxtralStBf16ToF32(get(p + "ffn_norm.bias")); + lw.q_w = VoxtralStBf16ToF32(get(p + "attention.wq.weight")); + lw.q_b = VoxtralStBf16ToF32(get(p + "attention.wq.bias")); + lw.k_w = VoxtralStBf16ToF32(get(p + "attention.wk.weight")); // k_proj: NO bias + lw.v_w = VoxtralStBf16ToF32(get(p + "attention.wv.weight")); + lw.v_b = VoxtralStBf16ToF32(get(p + "attention.wv.bias")); + lw.out_w = VoxtralStBf16ToF32(get(p + "attention.wo.weight")); + lw.out_b = VoxtralStBf16ToF32(get(p + "attention.wo.bias")); + lw.fc1_w = VoxtralStBf16ToF32(get(p + "feed_forward.w1.weight")); + lw.fc1_b = VoxtralStBf16ToF32(get(p + "feed_forward.w1.bias")); + lw.fc2_w = VoxtralStBf16ToF32(get(p + "feed_forward.w2.weight")); + lw.fc2_b = VoxtralStBf16ToF32(get(p + "feed_forward.w2.bias")); } } -// Permute the rows of a bf16 [n_heads*head_dim, K] q/k weight from the Meta- -// interleaved rope layout (mistral consolidated) to the HF NeoX layout vLLM's -// rotary_emb (is_neox_style=True) expects — the EXACT transform vLLM applies on -// the mistral load path (verified bit-exact: permute(wq)==vLLM q_proj). Row map -// per head: out(j*hd2 + i) <- in(2i + j), hd2 = head_dim/2. Pure byte reorder of -// bf16 values (no arithmetic) => bit-exact. wv/wo are NOT permuted. -std::vector PermuteQKBf16(const StTensor& t, int64_t n_heads) { - VT_CHECK(t.dtype == "BF16" && t.shape.size() == 2, "voxtral: q/k permute needs 2-D BF16"); - const int64_t d1 = t.shape[0], K = t.shape[1]; - const int64_t hd = d1 / n_heads, hd2 = hd / 2; - VT_CHECK(hd * n_heads == d1 && hd2 * 2 == hd, "voxtral: q/k permute head split mismatch"); - const auto* src = reinterpret_cast(t.data); - std::vector out(static_cast(d1) * K); - for (int64_t h = 0; h < n_heads; ++h) - for (int64_t i = 0; i < hd2; ++i) - for (int64_t j = 0; j < 2; ++j) { - const int64_t out_row = h * hd + j * hd2 + i; - const int64_t in_row = h * hd + 2 * i + j; - std::memcpy(&out[static_cast(out_row) * K], - &src[static_cast(in_row) * K], - static_cast(K) * sizeof(uint16_t)); - } - return out; -} - // Build the merged qkv OwnedTensor [Hq*Dh + 2*Hkv*Dh, K] (rows q|k|v) with q/k // rope-permuted and v raw — the mistral-format analog of LoadMergedBf16RawNK. OwnedTensor BuildPermutedQKV(const TensorResolver& get, const std::string& b, @@ -364,8 +331,8 @@ OwnedTensor BuildPermutedQKV(const TensorResolver& get, const std::string& b, const StTensor& wv = get(b + "attention.wv.weight"); const int64_t K = wq.shape[1]; const int64_t qd = wq.shape[0], kd = wk.shape[0], vd = wv.shape[0]; - std::vector pq = PermuteQKBf16(wq, config.num_attention_heads); - std::vector pk = PermuteQKBf16(wk, config.num_key_value_heads); + std::vector pq = VoxtralPermuteQKBf16(wq, config.num_attention_heads); + std::vector pk = VoxtralPermuteQKBf16(wk, config.num_key_value_heads); OwnedTensor m = dense_loaders::MakeOwned(DType::kBF16, {qd + kd + vd, K}); auto* dst = reinterpret_cast(m.bytes.data()); std::memcpy(dst, pq.data(), static_cast(qd) * K * sizeof(uint16_t)); @@ -399,6 +366,56 @@ void LoadTextWeights(const TensorResolver& get, const HfConfig& config, } // namespace +// --- bf16 StTensor -> host f32 vector (encoder + adapter weights). -------------- +// `t.data` points into the safetensors mmap, whose payload offset carries NO +// alignment guarantee (issue #772), so the bytes are read through +// vt::LoadUnaligned rather than a `const uint16_t*` — the same treatment +// dense_loaders::TransposeBf16 and minimax_h3_vae_loader.cpp already use, and +// free: at -O2 it emits the identical `movzwl` and no call. +std::vector VoxtralStBf16ToF32(const StTensor& t) { + VT_CHECK(t.dtype == "BF16", "voxtral: expected BF16 tensor"); + const auto* src = static_cast(static_cast(t.data)); + const size_t n = t.nbytes / sizeof(uint16_t); + std::vector out(n); + for (size_t i = 0; i < n; ++i) + out[i] = vt::BF16ToF32(vt::LoadUnaligned(src + i * 2)); + return out; +} + +// Permute the rows of a bf16 [n_heads*head_dim, K] q/k weight from the Meta- +// interleaved rope layout (mistral consolidated) to the HF NeoX layout vLLM's +// rotary_emb (is_neox_style=True) expects — the EXACT transform vLLM applies on +// the mistral load path (verified bit-exact: permute(wq)==vLLM q_proj). Row map +// per head: out(j*hd2 + i) <- in(2i + j), hd2 = head_dim/2. Pure byte reorder of +// bf16 values (no arithmetic) => bit-exact. wv/wo are NOT permuted. +// +// The source row address is computed in `unsigned char` (issue #772): the +// safetensors payload has no alignment guarantee, and forming +// `reinterpret_cast(t.data)` — let alone indexing it — is +// undefined even though every access here is a memcpy that never dereferences +// it as a uint16_t. That memcpy laundering is exactly why UBSan never reported +// this site while it reported its two siblings. NOT vt::LoadUnaligned: this is a +// bulk row copy, not a scalar load, so the `* sizeof(uint16_t)` that used to be +// implicit in the pointer type is now explicit in the byte offset. +std::vector VoxtralPermuteQKBf16(const StTensor& t, int64_t n_heads) { + VT_CHECK(t.dtype == "BF16" && t.shape.size() == 2, "voxtral: q/k permute needs 2-D BF16"); + const int64_t d1 = t.shape[0], K = t.shape[1]; + const int64_t hd = d1 / n_heads, hd2 = hd / 2; + VT_CHECK(hd * n_heads == d1 && hd2 * 2 == hd, "voxtral: q/k permute head split mismatch"); + const auto* src = static_cast(static_cast(t.data)); + std::vector out(static_cast(d1) * K); + for (int64_t h = 0; h < n_heads; ++h) + for (int64_t i = 0; i < hd2; ++i) + for (int64_t j = 0; j < 2; ++j) { + const int64_t out_row = h * hd + j * hd2 + i; + const int64_t in_row = h * hd + 2 * i + j; + std::memcpy(&out[static_cast(out_row) * K], + src + static_cast(in_row) * K * sizeof(uint16_t), + static_cast(K) * sizeof(uint16_t)); + } + return out; +} + // ─── VoxtralDecodeGraph (BF16 Mistral/Llama full-attention decode CUDA-graph) ── // The Voxtral-text sibling of Qwen3MoeDecodeGraph (qwen3_moe.cpp) and // Qwen3_5DenseDecodeGraph (qwen3_5.cpp): the SAME cold -> warm -> capture -> replay @@ -621,8 +638,8 @@ VoxtralWeights LoadVoxtralWeights(const SafetensorsFile& st, w.downsample_factor = 4; w.text_hidden = text_config.hidden_size; LoadEncoderWeights(get, embed_positions, w.encoder_cfg, w.encoder); - w.adapter_w_in = StBf16ToF32(get("mm_whisper_embeddings.audio_language_projection.0.weight")); - w.adapter_w_out = StBf16ToF32(get("mm_whisper_embeddings.audio_language_projection.2.weight")); + w.adapter_w_in = VoxtralStBf16ToF32(get("mm_whisper_embeddings.audio_language_projection.0.weight")); + w.adapter_w_out = VoxtralStBf16ToF32(get("mm_whisper_embeddings.audio_language_projection.2.weight")); LoadTextWeights(get, text_config, w.text); return w; } diff --git a/src/vllm/model_executor/models/voxtral_loader_internal.h b/src/vllm/model_executor/models/voxtral_loader_internal.h new file mode 100644 index 000000000..8f795253d --- /dev/null +++ b/src/vllm/model_executor/models/voxtral_loader_internal.h @@ -0,0 +1,33 @@ +// INTERNAL header (src/, not include/): the two Voxtral loader steps that read +// the mmap'd BF16 safetensors payload directly. Not part of the public ABI and +// deliberately not reachable from `include/vllm.h` — nothing outside this TU and +// its gate calls them. +// +// Exposed at all for the ODD-OFFSET loader gate (issue #772). A safetensors +// payload carries NO alignment guarantee — a tensor's first byte is +// `8 + + ` and not one +// of those terms is required to be even — so both of these must work over a BF16 +// tensor that begins on an odd address. They cannot be reached through +// `LoadVoxtralWeights` at test scale: `VoxtralEncoderConfig()` is FIXED at 32 +// layers of d_model 1280 / ffn 5120, so a synthetic checkpoint that satisfies it +// is ~1.2 GiB. Declaring them is what lets the gate drive the production code +// rather than a copy of it. Same shape as the internal resolver header +// `test_modelopt_mixed_precision` reaches through `-I${CMAKE_SOURCE_DIR}/src`. +#pragma once + +#include +#include + +#include "vllm/model_executor/model_loader/safetensors_reader.h" // StTensor + +namespace vllm { + +// BF16 StTensor -> host f32 vector (encoder + adapter weights). +std::vector VoxtralStBf16ToF32(const StTensor& t); + +// Permute the rows of a BF16 [n_heads*head_dim, K] q/k weight from the +// Meta-interleaved rope layout (mistral consolidated) to the HF NeoX layout +// vLLM's rotary_emb (is_neox_style=True) expects. +std::vector VoxtralPermuteQKBf16(const StTensor& t, int64_t n_heads); + +} // namespace vllm diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cc470617c..6f6bfaee9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -325,6 +325,18 @@ foreach(_gateup_lever VT_DENSE_MARLIN_GATEUP VT_NVFP4_MARLIN VT_MOE_FUSED_W13) PROPERTIES ENVIRONMENT "${_gateup_lever}=0") endforeach() vllm_cpp_add_test(test_qwen35_plain_weights vllm/models/test_qwen35_plain_weights.cpp) +# UNALIGNED SAFETENSORS PAYLOADS (#772, fourth recurrence after #301/#627/#674). +# The four loaders that formed a `const uint16_t*` over an mmap'd tensor, driven +# over checkpoints whose payload parity is FORCED odd by one space of JSON header +# padding. Two of the four launder every access through `memcpy`, so UBSan cannot +# see them and only these value checks can — which is why this is a suite of its +# own and not another sanitizer sweep. +vllm_cpp_add_test(test_loader_unaligned_offsets + vllm/models/test_loader_unaligned_offsets.cpp) +# The two Voxtral loader steps are declared in an INTERNAL header (src/, not +# include/): they are not public ABI and must not become so for a test's sake. +target_include_directories(test_loader_unaligned_offsets PRIVATE + ${CMAKE_SOURCE_DIR}/src) vllm_cpp_add_test(test_qwen3_8_text_only vllm/models/test_qwen3_8_text_only.cpp) # The PUBLISHED `Qwen/Qwen3.8-2.4T-A95B` config.json, committed VERBATIM, so the # config-resolution case reads the real document (nested `rope_parameters`, 92 diff --git a/tests/vllm/models/test_loader_unaligned_offsets.cpp b/tests/vllm/models/test_loader_unaligned_offsets.cpp new file mode 100644 index 000000000..89c421e2c --- /dev/null +++ b/tests/vllm/models/test_loader_unaligned_offsets.cpp @@ -0,0 +1,449 @@ +// ─── safetensors payloads have NO alignment guarantee (issue #772) ─────────── +// +// A tensor's first byte sits at `8 + + `. Not one of those three terms is required to be +// even, so a BF16 tensor beginning on an ODD address is an ordinary safetensors +// file, not a corrupt one. Four loaders formed a `const uint16_t*` over that +// address anyway — undefined behaviour on every target, and a real fault on the +// strict-alignment ones this project builds for (`build-test-cpu-arm64`, +// Jetson/Orin sm_110): +// +// voxtral.cpp:51 StBf16ToF32 — cast then INDEXED (UBSan sees this) +// voxtral.cpp:344 PermuteQKBf16 — cast then row-address + memcpy +// qwen3_vl.cpp:78 LoadVisionF32 — cast then INDEXED (UBSan sees this) +// qwen3_5_mtp.cpp:71 CopyRawNK — cast, `+ offset`, then memcpy +// +// This is the FOURTH recurrence of one class: #301 (closed, and the source of +// the `vt::LoadUnaligned` seam), #627 (`qwen3_5_weights.cpp`) and #674 / +// PR #688 (`ltx2_loader.cpp`) are the others. +// +// TWO OF THESE FOUR ARE INVISIBLE TO THE SANITIZER, WHICH IS WHY THIS FILE +// EXISTS RATHER THAN ANOTHER UBSAN SWEEP. `PermuteQKBf16` and `CopyRawNK` form +// and do arithmetic on the misaligned `uint16_t*` but then LAUNDER every access +// through `std::memcpy`, which reads bytes — so `-fsanitize=alignment` never +// fires on them. All three previous recurrences were found by UBSan; these two +// would have survived every one of those sweeps. For them the guarantee this +// file pins is VALUE CORRECTNESS at an odd offset: the fix rewrites element +// arithmetic (`uint16_t* + n`) into byte arithmetic (`unsigned char* + 2n`), and +// dropping that factor of two is exactly the mistake a reviewer must be able to +// see fail. Every case below therefore checks the LOADED VALUES, not merely that +// the load returned. +// +// EVERY CASE FORCES THE ODD OFFSET AND THEN ASSERTS IT. Mirroring +// `test_ltx2_video.cpp`'s "ODD safetensors payload offset (#674)" case: the JSON +// header is padded by one space (trailing whitespace is legal JSON, and padding +// the header is exactly how real writers align their payloads) until the payload +// lands on an odd byte, and the mapped ADDRESS parity is REQUIREd rather than +// inferred. A fixture edit that makes the address even fails the REQUIRE instead +// of passing while covering nothing. + +#include +#include +#include +#include + +#include +#include + +#include + +#include "vllm/model_executor/model_loader/safetensors_reader.h" +#include "vllm/model_executor/models/minimax_h3.h" +#include "vllm/model_executor/models/qwen3_5_mtp.h" +#include "vllm/model_executor/models/qwen3_vl.h" +#include "vllm/model_executor/models/qwen3_vl_vision.h" +// INTERNAL header, reached through -I${CMAKE_SOURCE_DIR}/src: the two Voxtral +// loader steps are not public ABI and must not become so for a test's sake. +#include "vllm/model_executor/models/voxtral_loader_internal.h" +#include "vt/dtype.h" + +namespace { + +// A bare temp directory. Deliberately NOT any model's Workspace fixture: these +// cases must not depend on a fixture whose tensor sizes could silently change +// the parity they exist to force. +struct TempDir { + std::string root; + TempDir() { + static int counter = 0; + root = "/tmp/vllm_unaligned_" + std::to_string(::getpid()) + "_" + + std::to_string(counter++); + ::mkdir(root.c_str(), 0755); + } + ~TempDir() { + const int rc = std::system(("rm -rf '" + root + "'").c_str()); + (void)rc; + } +}; + +struct Spec { + std::string name; + std::vector shape; +}; + +int64_t Numel(const std::vector& shape) { + int64_t n = 1; + for (const int64_t d : shape) n *= d; + return n; +} + +// The BF16 bit pattern this fixture stores at flat element index `i` of tensor +// `t`. Deliberately a BIT PATTERN and not a float: the expectation is then +// `vt::BF16ToF32` of the very bits on disk, so a case can assert EQUALITY rather +// than a tolerance, and a read shifted by one byte is a hard failure instead of +// something a band could absorb. The low bits vary per element so a row-address +// error moves the value; 0x3d00 keeps every pattern a small finite positive. +uint16_t FixtureBits(size_t t, size_t i) { + return static_cast(0x3d00U + ((t * 37U + i * 7U) & 0x1ffU)); +} + +std::string U64Le(uint64_t v) { + std::string s; + for (int i = 0; i < 8; ++i) s.push_back(static_cast((v >> (8 * i)) & 0xFFU)); + return s; +} + +// Build an all-BF16 safetensors file from `specs`, padding the counted JSON +// header with `header_pad` spaces. Returns the file bytes; `*payload_at` gets +// the absolute file offset of the FIRST tensor's first byte. +// +// Every BF16 tensor is an even number of bytes, so the parity of the first +// payload byte is the parity of EVERY tensor in the file — which is what lets +// one space of padding put all of them on odd addresses at once. +std::string BuildBf16Safetensors(const std::vector& specs, + size_t header_pad, size_t* payload_at) { + std::string header = "{"; + std::string body; + uint64_t offset = 0; + for (size_t i = 0; i < specs.size(); ++i) { + const int64_t n = Numel(specs[i].shape); + const auto nbytes = static_cast(n) * 2; + if (i != 0) header += ","; + header += "\"" + specs[i].name + "\":{\"dtype\":\"BF16\",\"shape\":["; + for (size_t d = 0; d < specs[i].shape.size(); ++d) { + if (d != 0) header += ","; + header += std::to_string(specs[i].shape[d]); + } + header += "],\"data_offsets\":[" + std::to_string(offset) + "," + + std::to_string(offset + nbytes) + "]}"; + offset += nbytes; + + const size_t at = body.size(); + body.resize(at + static_cast(nbytes)); + for (size_t e = 0; e < static_cast(n); ++e) { + const uint16_t v = FixtureBits(i, e); + std::memcpy(body.data() + at + e * 2, &v, 2); + } + } + header += "}"; + header.append(header_pad, ' '); + *payload_at = 8 + header.size(); + return U64Le(header.size()) + header + body; +} + +void WriteFileBytes(const std::string& path, const std::string& bytes) { + FILE* f = std::fopen(path.c_str(), "wb"); + REQUIRE(f != nullptr); + REQUIRE(std::fwrite(bytes.data(), 1, bytes.size(), f) == bytes.size()); + REQUIRE(std::fclose(f) == 0); +} + +// Write `specs` twice — unpadded, and padded by one space — and keep whichever +// lands the payload on an ODD byte. Doing it both ways means the case never +// depends on the exact length of the generated JSON above: a rename that changes +// the header length flips which file is kept, not whether the offset is odd. +// The REQUIRE proves the two spellings really do differ in parity, so a future +// change that made padding inert could not pass silently. +std::string WriteOddOffsetSafetensors(const TempDir& ws, const std::string& stem, + const std::vector& specs) { + const std::string a = ws.root + "/" + stem + "_a.safetensors"; + const std::string b = ws.root + "/" + stem + "_b.safetensors"; + size_t off_a = 0; + size_t off_b = 0; + WriteFileBytes(a, BuildBf16Safetensors(specs, 0, &off_a)); + WriteFileBytes(b, BuildBf16Safetensors(specs, 1, &off_b)); + REQUIRE((off_a % 2) != (off_b % 2)); + return (off_a % 2 == 1) ? a : b; +} + +// The fixture really is what every case here claims: this tensor's MAPPED +// address is odd, so no loader below can satisfy a `uint16_t`'s alignment by +// luck. mmap bases are page-aligned, so file-offset parity IS address parity — +// but assert the address rather than infer it. +void RequireOddlyMapped(const vllm::SafetensorsFile& file, const std::string& name) { + INFO("tensor: " << name); + REQUIRE((reinterpret_cast(file.Get(name).data) % 2) == 1); +} + +// Expected f32 for element `i` of the `t`-th tensor in a spec list. +float Expected(size_t t, size_t i) { return vt::BF16ToF32(FixtureBits(t, i)); } + +} // namespace + +// ─── voxtral.cpp:51 — StBf16ToF32 ─────────────────────────────────────────── +// +// UBSan-VISIBLE: the cast is followed by `src[i]`, so under +// `-fsanitize=alignment` this case reports "load of misaligned address ... for +// type 'const uint16_t', which requires 2 byte alignment" before the fix. +// +// Driven through the internal declaration rather than `LoadVoxtralWeights`, +// which cannot be reached at test scale: `VoxtralEncoderConfig()` is FIXED at 32 +// layers of d_model 1280 / ffn 5120, so a synthetic checkpoint satisfying it is +// ~1.2 GiB. See voxtral_loader_internal.h. +TEST_CASE("voxtral StBf16ToF32 reads a BF16 tensor at an ODD offset (#772)") { + const TempDir ws; + const std::vector specs = {{"adapter.weight", {3, 5}}}; + const std::string path = WriteOddOffsetSafetensors(ws, "voxtral_st", specs); + + const vllm::SafetensorsFile file = vllm::SafetensorsFile::Open(path); + RequireOddlyMapped(file, "adapter.weight"); + + const std::vector got = vllm::VoxtralStBf16ToF32(file.Get("adapter.weight")); + REQUIRE(got.size() == 15U); + for (size_t i = 0; i < got.size(); ++i) { + INFO("element " << i); + CHECK(got[i] == doctest::Approx(Expected(0, i)).scale(0.0)); + } +} + +// ─── voxtral.cpp:344 — PermuteQKBf16 ──────────────────────────────────────── +// +// UBSan-INVISIBLE: every access is a `memcpy`, so only the VALUES can witness +// the fix. The permutation is `out(h*hd + j*hd2 + i) <- in(h*hd + 2i + j)`, so +// each output row must equal a DIFFERENT, identifiable input row — an off-by-one +// row address (the failure mode of rewriting `&src[row*K]` into a byte offset +// without the `*2`) lands on the wrong row and every element of it changes. +TEST_CASE("voxtral PermuteQKBf16 permutes a BF16 tensor at an ODD offset (#772)") { + const TempDir ws; + constexpr int64_t kHeads = 2; + constexpr int64_t kHeadDim = 4; // hd2 = 2 + constexpr int64_t kK = 3; + const std::vector specs = {{"wq.weight", {kHeads * kHeadDim, kK}}}; + const std::string path = WriteOddOffsetSafetensors(ws, "voxtral_permute", specs); + + const vllm::SafetensorsFile file = vllm::SafetensorsFile::Open(path); + RequireOddlyMapped(file, "wq.weight"); + + const std::vector got = + vllm::VoxtralPermuteQKBf16(file.Get("wq.weight"), kHeads); + REQUIRE(got.size() == static_cast(kHeads * kHeadDim * kK)); + + constexpr int64_t kHd2 = kHeadDim / 2; + for (int64_t h = 0; h < kHeads; ++h) { + for (int64_t i = 0; i < kHd2; ++i) { + for (int64_t j = 0; j < 2; ++j) { + const int64_t out_row = h * kHeadDim + j * kHd2 + i; + const int64_t in_row = h * kHeadDim + 2 * i + j; + for (int64_t c = 0; c < kK; ++c) { + INFO("out_row " << out_row << " col " << c << " <- in_row " << in_row); + CHECK(got[static_cast(out_row * kK + c)] == + FixtureBits(0, static_cast(in_row * kK + c))); + } + } + } + } +} + +// ─── qwen3_vl.cpp:78 — LoadVisionF32 ──────────────────────────────────────── +// +// UBSan-VISIBLE (cast then `p[i]` inside Bf16BitsToF32). Driven through the +// PRODUCTION entry point `LoadQwen3VLVisionWeights`: with `depth = 0` and no +// deepstack indexes the tower needs exactly the nine tensors below, which is the +// smallest checkpoint that reaches the loader without a copy of it. Qwen3.6-27B +// really does ship an EMPTY deepstack_visual_indexes, so depth aside this is a +// shape the loader is expected to accept. +TEST_CASE("qwen3-vl vision loader reads BF16 tensors at an ODD offset (#772)") { + const TempDir ws; + const std::string v = "model.visual."; + const std::vector specs = { + {v + "patch_embed.proj.weight", {4, 3}}, + {v + "patch_embed.proj.bias", {4}}, + {v + "pos_embed.weight", {5, 2}}, + {v + "merger.norm.weight", {4}}, + {v + "merger.norm.bias", {4}}, + {v + "merger.linear_fc1.weight", {3, 4}}, + {v + "merger.linear_fc1.bias", {3}}, + {v + "merger.linear_fc2.weight", {2, 3}}, + {v + "merger.linear_fc2.bias", {2}}, + }; + const std::string path = WriteOddOffsetSafetensors(ws, "qwen3vl_vision", specs); + + std::vector shards; + shards.push_back(vllm::SafetensorsFile::Open(path)); + for (const Spec& s : specs) RequireOddlyMapped(shards[0], s.name); + + vllm::multimodal::Qwen3VLVisionConfig vc; + vc.depth = 0; + vc.deepstack_visual_indexes.clear(); + const vllm::multimodal::Qwen3VLVisionWeights vw = + vllm::LoadQwen3VLVisionWeights(shards, vc); + + // Spot every tensor the loader read, each against ITS OWN spec index, so a + // read that drifted into a neighbouring tensor is caught rather than absorbed. + struct Check { + size_t spec_index; + const std::vector* got; + }; + const std::vector checks = { + {0, &vw.patch_proj_w}, {1, &vw.patch_proj_b}, + {2, &vw.pos_embed_w}, {3, &vw.merger.norm_w}, + {4, &vw.merger.norm_b}, {5, &vw.merger.fc1_w}, + {6, &vw.merger.fc1_b}, {7, &vw.merger.fc2_w}, + {8, &vw.merger.fc2_b}, + }; + for (const Check& c : checks) { + INFO("tensor: " << specs[c.spec_index].name); + REQUIRE(c.got->size() == static_cast(Numel(specs[c.spec_index].shape))); + for (size_t i = 0; i < c.got->size(); ++i) { + INFO("element " << i); + CHECK((*c.got)[i] == doctest::Approx(Expected(c.spec_index, i)).scale(0.0)); + } + } +} + +// ─── qwen3_5_mtp.cpp:71 — CopyRawNK ───────────────────────────────────────── +// +// THE ONE A SANITIZER SWEEP CANNOT REACH. `CopyRawNK` forms +// `reinterpret_cast(source.data) + offset` and then memcpy's +// from it: the misaligned pointer is formed and advanced, but never +// dereferenced as a `uint16_t`, so `-fsanitize=alignment` stays silent. Only +// reading the code — or this case — finds it. +// +// What the case pins is the ELEMENT-vs-BYTE arithmetic. `offset` counts BF16 +// ELEMENTS, so the byte-pointer rewrite must advance `offset * 2` bytes; the +// two-expert MoE below gives expert 1 a non-zero `offset` for all three of +// gate/up/down, so a rewrite that dropped the `* 2` reads expert 0's rows into +// expert 1 (and the up-projection slice, whose offset is `gu_base + +// intermediate*hidden`, lands on the gate rows). Each expected value is derived +// from the fixture's own stacked layout, so those confusions are visible as +// specific wrong values and not merely as "something differs". +TEST_CASE("qwen3.5 MTP stacked-expert slice at an ODD offset (#772)") { + const TempDir ws; + constexpr int64_t kHidden = 4; + constexpr int64_t kHeadDim = 2; + constexpr int64_t kHeads = 2; // q_proj packs q|gate => [2*Q, H] + constexpr int64_t kKvHeads = 1; + constexpr int64_t kExperts = 2; + constexpr int64_t kMoeInter = 3; + constexpr int64_t kShared = 2; + constexpr int64_t kQ = kHeads * kHeadDim; + constexpr int64_t kKv = kKvHeads * kHeadDim; + + const std::string b = "mtp.layers.0."; + const std::string sa = b + "self_attn."; + const std::string mlp = b + "mlp."; + const std::vector specs = { + {"mtp.fc.weight", {kHidden, 2 * kHidden}}, + {"mtp.pre_fc_norm_embedding.weight", {kHidden}}, + {"mtp.pre_fc_norm_hidden.weight", {kHidden}}, + {"mtp.norm.weight", {kHidden}}, + {b + "input_layernorm.weight", {kHidden}}, + {b + "post_attention_layernorm.weight", {kHidden}}, + {sa + "q_proj.weight", {2 * kQ, kHidden}}, + {sa + "k_proj.weight", {kKv, kHidden}}, + {sa + "v_proj.weight", {kKv, kHidden}}, + {sa + "o_proj.weight", {kHidden, kQ}}, + {sa + "q_norm.weight", {kHeadDim}}, + {sa + "k_norm.weight", {kHeadDim}}, + {mlp + "gate.weight", {kExperts, kHidden}}, + {mlp + "shared_expert_gate.weight", {1, kHidden}}, + {mlp + "experts.gate_up_proj", {kExperts, 2 * kMoeInter, kHidden}}, + {mlp + "experts.down_proj", {kExperts, kHidden, kMoeInter}}, + {mlp + "shared_expert.gate_proj.weight", {kShared, kHidden}}, + {mlp + "shared_expert.up_proj.weight", {kShared, kHidden}}, + {mlp + "shared_expert.down_proj.weight", {kHidden, kShared}}, + }; + constexpr size_t kGateUpSpec = 14; + constexpr size_t kDownSpec = 15; + REQUIRE(specs[kGateUpSpec].name == mlp + "experts.gate_up_proj"); + REQUIRE(specs[kDownSpec].name == mlp + "experts.down_proj"); + + const std::string path = WriteOddOffsetSafetensors(ws, "qwen35_mtp", specs); + std::vector shards; + shards.push_back(vllm::SafetensorsFile::Open(path)); + RequireOddlyMapped(shards[0], specs[kGateUpSpec].name); + RequireOddlyMapped(shards[0], specs[kDownSpec].name); + + vllm::HfConfig config; + config.hidden_size = kHidden; + config.head_dim = kHeadDim; + config.num_attention_heads = kHeads; + config.num_key_value_heads = kKvHeads; + config.num_experts = kExperts; + config.num_experts_per_tok = 1; + config.moe_intermediate_size = kMoeInter; + config.shared_expert_intermediate_size = kShared; + + const vllm::Qwen3_5MTPWeights w = + vllm::LoadQwen3_5MTP(shards, config, vllm::Qwen3_5MTPKind::kMoe); + REQUIRE(w.NumLayers() == 1); + const vllm::MoeBlockWeights& moe = w.moe_layers[0].moe; + REQUIRE(moe.expert_gate.size() == static_cast(kExperts)); + + // The stacked source layouts are gate_up[E, 2I, H] and down[E, H, I]; the + // loader slices gate at `e*2*I*H`, up at `e*2*I*H + I*H`, down at `e*H*I`. + const auto* gate_up_bytes = shards[0].Get(specs[kGateUpSpec].name).data; + const auto* down_bytes = shards[0].Get(specs[kDownSpec].name).data; + REQUIRE(gate_up_bytes != nullptr); + REQUIRE(down_bytes != nullptr); + + for (int64_t e = 0; e < kExperts; ++e) { + const auto gu_base = static_cast(e * 2 * kMoeInter * kHidden); + const auto down_base = static_cast(e * kHidden * kMoeInter); + const auto rows_gu = static_cast(kMoeInter * kHidden); + const auto rows_down = static_cast(kHidden * kMoeInter); + + const auto& gate = moe.expert_gate[static_cast(e)]; + const auto& up = moe.expert_up[static_cast(e)]; + const auto& down = moe.expert_down[static_cast(e)]; + REQUIRE(gate.bytes.size() == rows_gu * 2); + REQUIRE(up.bytes.size() == rows_gu * 2); + REQUIRE(down.bytes.size() == rows_down * 2); + + for (size_t i = 0; i < rows_gu; ++i) { + uint16_t bits = 0; + std::memcpy(&bits, gate.bytes.data() + i * 2, 2); + INFO("expert " << e << " gate element " << i); + CHECK(bits == FixtureBits(kGateUpSpec, gu_base + i)); + + std::memcpy(&bits, up.bytes.data() + i * 2, 2); + INFO("expert " << e << " up element " << i); + CHECK(bits == FixtureBits(kGateUpSpec, gu_base + rows_gu + i)); + } + for (size_t i = 0; i < rows_down; ++i) { + uint16_t bits = 0; + std::memcpy(&bits, down.bytes.data() + i * 2, 2); + INFO("expert " << e << " down element " << i); + CHECK(bits == FixtureBits(kDownSpec, down_base + i)); + } + } +} + +// ─── minimax_h3_vae_loader.cpp:96-101 — the seam-routed sibling ────────────── +// +// NOT a defect: this loop already did the byte-wise load, and already carried +// the reason in prose. It is here because it hand-rolled `std::memcpy` where the +// shared `vt::LoadUnaligned` seam exists — the parallel path AGENTS.md's +// shared-seam rule is about — and routing it through the seam must be inert. +// This case is the inertness proof for that rewrite: the same odd-offset load, +// through the production entry point, must still return the exact bytes on disk. +TEST_CASE("minimax-h3 audio VAE reads a BF16 tensor at an ODD offset (#772)") { + const TempDir ws; + // `.bias`, not `.weight`: a 3-D `.weight` takes the materialized-weight-norm + // branch, which derives magnitudes and is not the plain read under test. + const std::vector specs = {{"decoder.dec_in_proj.bias", {6}}}; + const std::string path = WriteOddOffsetSafetensors(ws, "minimax_vae", specs); + + const vllm::SafetensorsFile file = vllm::SafetensorsFile::Open(path); + RequireOddlyMapped(file, "decoder.dec_in_proj.bias"); + + const vllm::MiniMaxH3AudioVaeWeights w = vllm::LoadMiniMaxH3AudioVaeWeights(file); + // `decoder.` is stripped by the loader. + REQUIRE(w.tensors.count("dec_in_proj.bias") == 1); + const std::vector& got = w.tensors.at("dec_in_proj.bias"); + REQUIRE(got.size() == 6U); + for (size_t i = 0; i < got.size(); ++i) { + INFO("element " << i); + CHECK(got[i] == doctest::Approx(Expected(0, i)).scale(0.0)); + } +}