From e0ae04b1f65fca593af362e90fc9ec138391ec2d Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Wed, 21 Oct 2026 07:55:26 -0400 Subject: [PATCH 1/4] chore: prepare upstream 982 qualification with ABI evidence --- README.md | 8 ++ docs/upstream_982_qualification.md | 180 +++++++++++++++++++++++++++++ tests/native/runtime_api_test.cpp | 49 ++++++++ tests/test_runtime_api.py | 49 ++++++++ third_party/stable-diffusion.cpp | 2 +- 5 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 docs/upstream_982_qualification.md create mode 100644 tests/native/runtime_api_test.cpp create mode 100644 tests/test_runtime_api.py diff --git a/README.md b/README.md index 869d545..f58beb7 100644 --- a/README.md +++ b/README.md @@ -604,6 +604,14 @@ python3 tools/apple_xcframework.py consumer dist/stable-diffusion-native-apple-x Flutter companion does (a dynamic library that re-exports the framework), runs the macOS probe and builds the iOS device and simulator slices. +## Upstream qualification + +The local upstream 982 candidate is preparation only; see the +[ABI audit, carry inventory and outstanding gates](docs/upstream_982_qualification.md). +Its larger context struct requires regenerated consumer bindings, and Metal +batched CFG remains unqualified. `tests/test_runtime_api.py` exercises a built +runtime's defaults and component conversion without models or a GPU. + ## Release Run the `Native Build & Release` workflow with a tag such as `v0.1.0`. Tags are diff --git a/docs/upstream_982_qualification.md b/docs/upstream_982_qualification.md new file mode 100644 index 0000000..fdb999d --- /dev/null +++ b/docs/upstream_982_qualification.md @@ -0,0 +1,180 @@ +# Upstream 982 qualification — local preparation only + +This branch prepares upstream `0518b34374761cb82aeda5ad3c2f0686fdad2cec` +(`master-982-0518b34`), with ggml `dc998771e700d49bbdf9a39cede22bb5b3311767`. +It is **not ready for publication or consumer adoption**. No source workaround +has enough evidence for retirement. The public baseline remains owner +`v0.2.0-4` / `c9922d960c79f7ba7381161ca6784f3b7c1e2fce`, upstream +`3f8527a46c54ecf4cb4ed6003da8e8982283c73c` / ggml `89c4413f`. +Live owner main and upstream master were refreshed on 2026-10-21. + +## ABI and wrapper audit + +The wrappers compile unchanged against the new upstream and ggml headers. +Long operations still enter `Call` inside the native wrapper; create/free +tracking remains atomic with respect to teardown. No callback, long-running +export, object type or fallback to an unguarded upstream call was added. + +Executed on macOS arm64 with Apple clang; values below are bytes: + +| Type/member | Published baseline | Candidate | +| --- | ---: | ---: | +| `sizeof(sd_ctx_params_t)` | 320 | 336 | +| `disable_segmented_compute` | 288 | 288 | +| `batched_cfg` | absent | 289 (previous padding) | +| `linear_scale`, `attn_scale` | 292, 296 | 292, 296 | +| `tokenizer`, `sage_attn`, `conditioning_cache_size` | 304, 312, 316 | 304, 312, 316 | +| `flash_attn_mode`, `conv_direct_mode` | absent | 320, 328 | +| `sizeof(sd_img_gen_params_t)` | 552 | 552 | +| `sizeof(sd_vid_gen_params_t)` | 584 | 584 | +| `sizeof(sd_sample_params_t)`, `sd_image_t`, `sd_lora_t` | 96, 24, 16 | 96, 24, 16 | +| `sizeof(sd_preview_info_t)` | absent | 8 | + +`sd_ctx_params_init` sets `batched_cfg=true`; both mode pointers are null. +Legacy attention/convolution booleans remain false and cache size remains 4. +The added pointers make an old 320-byte Dart allocation unsafe: the candidate +initializer writes beyond it. This is a **blocking consumer ABI gate**, even +though older members keep their offsets. Bindings must be regenerated from +released headers, and the mode/batching policy must be mapped explicitly. +Null modes preserve the legacy flag route; adopting auto mode is separate. + +`convert_with_components` now appends `const sd_lora_t* loras, int lora_count`. +The new runtime test compiles that exact function-pointer type and calls the +13-argument API with no LoRAs to convert synthetic Safetensors into GGUF. +`convert` retains its six-argument signature. The two new exports are +`sd_get_preview_info` and `sd_ctx_supports_ref_image_conditioning`. +The former describes a native preview callback; it does not justify adding +Dart callbacks. Video, upscaler, adetailer, control-net hot-swap and conversion +still need native lifetime wrappers before a killable Dart isolate uses them +as long operations. Neither new export creates such public Dart support. + +`tests/test_runtime_api.py` checks real initializer defaults and a trailing +canary, null capability-query behavior, initial preview state and conversion +against a built runtime, without a model or GPU. It skips without that runtime; +set `SD_RUNTIME_API_TARGET` for a built Linux target. The source also pins the +64-bit layouts and changed conversion signature at compile time. + +## Complete owner carry inventory + +“Permanent” means part of the managed-runtime or packaging contract. “Keep” +means an upstream equivalent is absent or insufficiently qualified. Release +names describe provenance, not acceptance of a candidate on every platform. +Owner tag ancestry places page alignment in v0.1.1, SwiftPM packaging in +v0.2.0, privacy/progress/Apple teardown in v0.2.0-1, log/device tracking in +v0.2.0-2, Linux host-exit retention in v0.2.0-3 and Metal fail-loud in v0.2.0-4. + +| Carry / class | Purpose, original trigger and owner provenance | Affected revisions / upstream equivalent and earliest evidence | Tests and decision | +| --- | --- | --- | --- | +| Vendored source patches | None: the owner stores a submodule, not an applied patch stack. Upstream's ggml fork is upstream content. | Both baseline and candidate submodules are clean. | **None to remove.** Do not mistake upstream `af0fc72` removing its conv1d workaround for retirement of an owner patch. | +| Ordered native progress ring / permanent | `ea53d85` (#14): Dart callbacks after shutdown aborted; callback locks deadlocked VM shutdown; latest-only polling lost batch transitions. | All owner builds since #14. Candidate still has a process-global native callback, without a managed-runtime polling/lifetime replacement. No equivalent release. | `test_progress`, `test_progress_stress`, real CPU generation. **Keep** all history, atomics and registration lifetime. | +| Native log/error ring / permanent | `6eb1bac` (#17): upstream log threads cannot safely call Dart; failed loads return null without a reason. | All owner builds since #17. Upstream callback still borrows message storage; no equivalent. | `test_log`, missing-model/exit stand-ins. **Keep** bounded error window, UTF-8 boundaries and dropped-message accounting. | +| Log spin abandonment / mitigation | #17: Windows `ExitProcess` can kill a flag holder; a descheduled reader formerly hid load errors. Separate error flag and 100 ms bound prevent indefinite waits. | Candidate has no equivalent recorder or holder-death protocol. | `test_log` plain, ASan and TSan controls; Windows execution remains unavailable. **Keep.** | +| Native context registry and guarded load/generate/free/cancel / permanent | `fd6456e` (#15), `736430e` (#16): abandoned contexts and calls must not race exit teardown. | Candidate raw `new_sd_ctx`/`generate_image` provide no managed-isolate lifetime tracking. | `test_exit_teardown`, CPU tracked-count 1→0 and double-free no-op. **Keep**, including creation/free inside one native call. | +| Apple hidden `__cxa_atexit`, static-destructor ordering / mitigation | #15: ggml-metal aborts if buffers remain when its static destructor runs. | Candidate still has Metal-owned resources/statics; no upstream ordering contract for abandoned managed objects. | Native stand-ins and `validate_artifacts` import/export closure pass. Required real Metal release/ASan teardown remains unavailable. **Keep.** | +| Bounded teardown waits and 250 ms settle / mitigation | #15/#16/#17: isolate can disappear between calls; load/generate/query cannot always cancel promptly. 15 s work / 2 s other waits, then free nothing. | No candidate equivalent or stronger cancellation bound. | `test_exit_teardown`, including in-flight/free/query/race controls. Real-model Metal phases remain unqualified. **Keep.** | +| Linux owned-static retention and cooperative host contract / mitigation | `16df759` (#18), following #949: destructors raced active load/generation. Earlier force-exit/early-flush approach was superseded to preserve host callbacks and stdio. | Candidate has no replacement host-shutdown protocol. Applies to Linux, not Android. | Stand-in host contract is retained; Linux native/Dart runtime and ASan matrix not run here. **Keep**, never reintroduce force `_exit`. | +| Linux GCC 13 floor / build mitigation | #18: pre-13 `` injects stream initializers whose destructors owned-static retention drops. Artifact validator rejects imports. | Upstream API changes do not change this compiler/runtime constraint. | `test_tools` ELF fixtures; real Linux builds unavailable. **Keep.** | +| Static-registry device queries / permanent | #17: a query in flight crashed at exit in 2/200 trials before tracking; 0/300 after. Queries use `SdDartStaticsCall` even with no tracked context. | Candidate ggml registry is still static; no equivalent. | `test_device_memory`, `test_exit_teardown` query controls. **Keep** native wait and catch boundaries. | +| GPU build-presence/status mapping / permanent | #17: an absent backend and a compiled backend finding no device are distinct states. | Candidate ggml registry still cannot represent the build-time distinction alone. | `test_device_memory` with/without backend definitions. **Keep.** | +| Free-memory underflow/clamp / defensive mitigation | #17: Vulkan subtracts unsigned usage from budget; overuse wrapped near `SIZE_MAX`; clamp output to `[0,total]`. | ggml `65c60b239f02ab3fb35017af1e817d17a13e4952` now uses min(budget,heap), saturating subtraction. Included by SD `af0fc72` (#2147); earliest locally available containing release `master-973-f0aac52`. SD `60423ab` (master-961) reclaims before wrapped estimates; different layer. | Wrapper under/over-budget stand-ins pass. No real Vulkan comparison or artifact-level retirement proof. **Keep** cheap output invariant; upstream source overlap alone is insufficient. | +| Metal fail-loud output discard/context poison / correctness mitigation | `ea60915` (#21), owner #19: command-buffer failure could return corrupt images as success. Published v0.2.0-4. | Candidate Metal still logs the same failure/status/error-state diagnostics; no verified upstream image-result cleanup/context poison equivalent. | Existing `test_exit_teardown` controlled failures/cleanup/success and `test_log` level/overflow controls retained. CPU image success passes. Physical failure unavailable. **Keep** conservative overlap invalidation. Separate from watchdog prevention (#1047). | +| Windows short upstream build + inherited `EP_PREFIX` / build workaround | #14: Vulkan shader generator hit FTK1011 at 260-character tlog paths; short upstream path alone failed. EP prefix reduced the recorded path to 213 chars. | Candidate still nests an ExternalProject shader build. No measured default-path Windows replacement. | Source retained; Windows build unavailable. **Keep.** | +| Export allowlist / permanent | `935b70e` and #14: separate ggml copies must coexist without symbol collisions. Headers define public closure. | Candidate adds two public exports; does not remove collision risk. | Candidate macOS artifact exact export/dependency validation passes, no imported `__cxa_atexit`. Windows PE and ELF parser fixtures retained. **Keep.** | +| Windows MSVC generator / build selection | Initial pipeline: Ninja could select runner MinGW rather than MSVC. | No evidence candidate changes runner compiler selection. | Tool configuration retained, no Windows run. **Keep.** | +| Android 16 KiB alignment / platform requirement | `31bdfb7` (#7): Google Play page-size acceptance. `ANDROID_SUPPORT_FLEXIBLE_PAGE_SIZES` plus linker page size. | API28/Armv8.2 dotprod+fp16 target remains the shipped contract; no upstream update removes it. | Configuration tests retained; Android build/device unavailable. **Keep.** | +| Android CPU-only / support boundary | Initial/Vulkan pipeline #6: Adreno shader compiler and Mali null-address failures; OpenCL slower on tested Adreno. | No qualifying candidate device evidence. | No device runs. **Keep**, never infer GPU support from desktop software Vulkan. | +| Separate CPU desktop payload / packaging fallback | #6: Vulkan payload links a loader that may be absent. | Candidate Vulkan dependency still exists. | Build/dependency rules retained; target configuration tests pass. **Keep.** | +| `GGML_NATIVE=OFF`, OpenMP off, examples/WebP/WebM off / build policy | Initial pipeline: portable, self-contained library payload; no examples/video encoder dependencies. These are configuration choices, not source repairs. | No evidence justifies new runtime dependencies or host-CPU specialization. | macOS dependency validation passes. **Keep.** | +| Apple minimum OS / packaging contract | Initial/`bc8c1c2` (#11): support floors and XCFramework SwiftPM consumption. Info.plist floor comes from each binary's LC_BUILD_VERSION. | Candidate compile retains macOS13.3/iOS16.4; no hard-coded plist substitute. | Single macOS binary import/dependency check passes; full five-image XCFramework/signing/consumer not run. **Keep.** | +| Apple privacy manifest / packaging contract | `68b29da` (#13): SDK must declare required-reason APIs in each framework slice, at the correct versioned/flat location. | Existing stat/fstat/file-stamp access still present. No new categories may be guessed from upgrade success. | Manifest/packaging fixture tests run; full candidate XCFramework import audit unavailable. **Keep** file-timestamp reasons, require all slices before release. | + +## Reader controls and issue boundaries + +Owner [#23](https://github.com/leehack/stable-diffusion-native/issues/23) +remains reproducible. Baseline and candidate `pickle_io.cpp` and `binary_io.h` +are byte-identical. Using the native fixture from draft +[#25](https://github.com/leehack/stable-diffusion-native/pull/25), exact head +`d9bacef31e2569d5df4c3e508caf278d704280eb`: + +| Control | Baseline source | Candidate source | Separate patched source copy | +| --- | --- | --- | --- | +| Valid bounded pickles / malformed and truncated pickles | pass | pass | pass | +| Malformed input in >INT_MAX and >UINT32_MAX spans | rejected | rejected | rejected | +| Same valid prefixes in those large containing spans | fail at GLOBAL | fail at GLOBAL | pass all cases | +| Bounded inputs with ASan | same source as candidate | pass | pass | + +The diagnostic copy makes `find_char` accept `size_t`, caps its int-returning +scan at `INT_MAX`, and removes all 13 narrowing remaining-span casts. It is +outside the submodule and **not part of this branch's runtime**. This proves +the scanner mechanism, not a shipped fix. PR #25 remains test-only with its +expected failure; no duplicate test PR or upstream submission was made. +The original trusted >2 GiB real-checkpoint/public-Dart evidence is in #23; +this run does not repeat the huge-checkpoint conversion or loading. + +Owner [#24](https://github.com/leehack/stable-diffusion-native/issues/24) +remains unresolved: its original full ZIP load timed out at 45 s. Later +non-reproduction reported in #25 does not identify a causal fix or reconcile +that input/environment. No large ZIPs were created/downloaded here, and the +scanner patch must not be presented as fixing ZIP loading. + +## Executed qualification and remaining gates + +Built the baseline and candidate macOS-arm64 libraries using the owner build +tool; relocated ccache into the worktree after the sandbox refused its usual +home-directory cache. Candidate `validate_artifacts.py macos-arm64` passes. +No production wrapper source change was needed. + +The checksum-verified published v0.2.0-4 archive and dylib are compared +separately from the source rebuild. The archive SHA-256 is +`840df6e27e1cac6d378dd5529f7c3b8d3b176417f950ef523c3535c09c92f528`; +the dylib SHA-256 is +`79ed1ab7c0451596bcb18070d459826a8b04409875873e91f39ffd84b2ffe1ce`. + +The guarded native comparison uses existing SD-Turbo F16 GGUF + full TAESD, +CPU/four threads, 128×128, one Euler/discrete step, seed7, CFG2, prompt +`a lighthouse`. It verifies image/count/dimensions, writes raw RGB, frees via +`sd_dart_exit_free`, checks registry count1→0 and double-free no-op. Candidate +explicit mode controls use attention on/convolution off; an explicit +unbatched route and a batched CPU route are compared. This is a correctness +smoke, not a performance benchmark or new-family qualification. All four +RGB controls have SHA-256 +`bf06aed2310655953e6a403c264db320ac23aaf407e8f8e67fc7853a05839c86`. +The candidate's null-mode/legacy-flag route was also compared separately. + +The selected suite ran 48 tests: 44 passed, three Linux-only import/stdio +checks skipped on macOS, and the newly written conversion probe failed +because it supplied a null `tensor_type_rules` string. The existing smoke +recipe passes an empty string; the corrected probe now does so and passes +its targeted rerun (one test). The independent auditor reproduced the same +invalid-null failure on the baseline, so this was a harness error, not a +candidate regression. The combined completed evidence is 45 passing tests +and three explicit skips; the full suite was not repeated after that +one-argument test correction. Existing coverage includes native ASan/TSan +stand-ins and the pinned-Dart progress/shutdown stress controls. The native +model-dependent exit runtime harness was deliberately excluded. + +An initial whole-suite attempt used the default Flutter wrapper instead of +the pinned Dart SDK and auto-detected the library built during the run; it +was stopped and is not qualification evidence. All replacement tests used +the pinned Flutter 3.47.1 Dart binary. No remaining owned test jobs are left +running at the freeze. + +**Required before adoption:** exact-head independent review; regenerate and +map consumer bindings/default policy; obtain Metal GROUP_NORM batch1/batch2 +sentinels and real batched/unbatched image controls; run release and ASan +Metal teardown and Linux cooperative native/Dart teardown; qualify real +Vulkan memory/performance; audit every Apple slice's privacy/minimum-OS/ +XCFramework/SwiftPM/signing; complete platform packaging/export checks. + +Upstream [#2149](https://github.com/leejet/stable-diffusion.cpp/issues/2149) +is still open. Candidate Metal GROUP_NORM still dispatches `ngrp,1,1` and +passes dimensions0–2 only; its normalization kernel is unchanged from the +baseline. This source evidence plus the upstream reproduction means the new +`batched_cfg=true` default cannot be claimed supported on Metal. CPU parity +does not resolve that gate. FP8 CPU #2139/#2140 (first master-966) and Vulkan +convolution #2137/#2141 (first master-967) have upstream repair provenance, +but representative FP8/real-Vulkan runtime qualification was unavailable. +No unrelated model substitutes for those acceptance checks. + +No consumer pins, published artifacts, tags, workflows, issues or PRs were +changed. All existing primary/active investigation checkouts were preserved. diff --git a/tests/native/runtime_api_test.cpp b/tests/native/runtime_api_test.cpp new file mode 100644 index 0000000..7405aae --- /dev/null +++ b/tests/native/runtime_api_test.cpp @@ -0,0 +1,49 @@ +#include "sd_dart_wrapper.h" + +#include +#include +#include +#include + +#define CHECK(condition) do { if (!(condition)) { \ + std::fprintf(stderr, "CHECK(%s) failed\n", #condition); return 1; } } while (0) + +// Every shipped target is 64-bit. Appended context members require consumers +// to regenerate bindings even when preceding members keep their offsets. +static_assert(sizeof(void*) == 8, "this probe describes the shipped 64-bit ABI"); +static_assert(sizeof(sd_ctx_params_t) == 336, "context ABI changed"); +static_assert(offsetof(sd_ctx_params_t, batched_cfg) == 289, "CFG ABI changed"); +static_assert(offsetof(sd_ctx_params_t, linear_scale) == 292, "context ABI changed"); +static_assert(offsetof(sd_ctx_params_t, tokenizer) == 304, "context ABI changed"); +static_assert(offsetof(sd_ctx_params_t, conditioning_cache_size) == 316, "context ABI changed"); +static_assert(offsetof(sd_ctx_params_t, flash_attn_mode) == 320, "attention mode ABI changed"); +static_assert(offsetof(sd_ctx_params_t, conv_direct_mode) == 328, "convolution mode ABI changed"); +static_assert(sizeof(sd_img_gen_params_t) == 552, "image ABI changed"); +static_assert(sizeof(sd_vid_gen_params_t) == 584, "video ABI changed"); +using Convert = bool (*)(const char*, const char*, const char*, const char*, const char*, const char*, + const char*, sd_type_t, const char*, bool, int, const sd_lora_t*, int); +static_assert(std::is_same::value, "conversion ABI changed"); + +int main(int argc, char** argv) { + CHECK(argc == 3); + struct GuardedParams { + sd_ctx_params_t params; + unsigned char guard[32]; + } guarded; + std::memset(&guarded, 0xA5, sizeof(guarded)); + sd_ctx_params_init(&guarded.params); + for (unsigned char value : guarded.guard) CHECK(value == 0xA5); + const sd_ctx_params_t& params = guarded.params; + CHECK(params.batched_cfg); + CHECK(params.flash_attn_mode == nullptr && params.conv_direct_mode == nullptr); + CHECK(!params.flash_attn && !params.diffusion_flash_attn); + CHECK(!params.diffusion_conv_direct && !params.vae_conv_direct); + CHECK(params.conditioning_cache_size == 4); + CHECK(!sd_ctx_supports_ref_image_conditioning(nullptr)); + const sd_preview_info_t preview = sd_get_preview_info(); + CHECK(preview.sample_pass == 0 && preview.total_steps == 0); + CHECK(convert_with_components(argv[1], nullptr, nullptr, nullptr, nullptr, nullptr, + argv[2], SD_TYPE_F16, "", false, 1, nullptr, 0)); + std::puts("runtime API defaults, bounds, capability query and conversion passed"); + return 0; +} diff --git a/tests/test_runtime_api.py b/tests/test_runtime_api.py new file mode 100644 index 0000000..41a7418 --- /dev/null +++ b/tests/test_runtime_api.py @@ -0,0 +1,49 @@ +"""Exercise the changed upstream ABI against a built runtime without a model or GPU.""" + +import os +import platform +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "tools")) +import build # noqa: E402 +from smoke_test import write_safetensors # noqa: E402 + + +class RuntimeApiTest(unittest.TestCase): + def test_context_defaults_and_component_conversion(self): + target_name = os.environ.get("SD_RUNTIME_API_TARGET", "") + if not target_name and sys.platform == "darwin": + target_name = "macos-arm64" if platform.machine() == "arm64" else "macos-x64" + if not target_name or sys.platform == "win32": + self.skipTest("set SD_RUNTIME_API_TARGET to a built macOS or Linux target") + target = build.TARGETS[target_name] + library = build.BIN_ROOT / target.name / "lib" / target.library + if not library.is_file(): + self.skipTest(f"no built runtime for {target.name}") + with tempfile.TemporaryDirectory() as directory: + directory = Path(directory) + source = directory / "probe.safetensors" + output = directory / "probe.gguf" + write_safetensors(source) + binary = directory / "runtime_api_test" + subprocess.run([ + os.environ.get("CXX") or shutil.which("c++"), "-std=c++17", "-O1", + f"-I{ROOT / 'src'}", f"-I{build.HEADER.parent}", + str(ROOT / "tests/native/runtime_api_test.cpp"), + f"-L{library.parent}", "-lstable-diffusion", f"-Wl,-rpath,{library.parent}", + "-o", str(binary), + ], check=True) + result = subprocess.run([str(binary), str(source), str(output)], + capture_output=True, text=True, timeout=30) + self.assertEqual(0, result.returncode, result.stdout + result.stderr) + self.assertEqual(b"GGUF", output.read_bytes()[:4]) + + +if __name__ == "__main__": + unittest.main() diff --git a/third_party/stable-diffusion.cpp b/third_party/stable-diffusion.cpp index 3f8527a..0518b34 160000 --- a/third_party/stable-diffusion.cpp +++ b/third_party/stable-diffusion.cpp @@ -1 +1 @@ -Subproject commit 3f8527a46c54ecf4cb4ed6003da8e8982283c73c +Subproject commit 0518b34374761cb82aeda5ad3c2f0686fdad2cec From 50230f71d8a3559d17c6fb1b346645f338f2afdc Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Thu, 22 Oct 2026 07:51:46 -0400 Subject: [PATCH 2/4] fix: run CFG unbatched in Metal builds and record upstream 982 evidence Upstream 982 enables batched CFG by default, and ggml-metal GROUP_NORM writes only the first batch item (leejet/stable-diffusion.cpp#2149), so a batched step returns a wrong image as success. sd_dart_new_sd_ctx loads a copy with batched_cfg off when the library is built with Metal. Smoke-tests conversion under a non-ASCII UTF-8 path on every target, and records the Metal controls, local suite and remaining gates. --- CMakeLists.txt | 8 +- README.md | 18 ++-- docs/upstream_982_qualification.md | 129 ++++++++++++++++------------ src/sd_dart_exit.cpp | 16 ++++ src/sd_dart_wrapper.h | 7 ++ tests/native/exit_teardown_test.cpp | 33 +++++++ tests/test_exit_teardown.py | 8 ++ tools/smoke_test.py | 30 ++++++- 8 files changed, 183 insertions(+), 66 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5823873..ab3b3a9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,5 +35,11 @@ foreach(backend CUDA HIPBLAS METAL VULKAN OPENCL SYCL MUSA) set(SD_DART_GPU_BACKEND 1) endif() endforeach() +if(SD_METAL) + set(SD_DART_METAL 1) +else() + set(SD_DART_METAL 0) +endif() target_compile_definitions(stable-diffusion PRIVATE - SD_DART_GPU_BACKEND=${SD_DART_GPU_BACKEND}) + SD_DART_GPU_BACKEND=${SD_DART_GPU_BACKEND} + SD_DART_METAL=${SD_DART_METAL}) diff --git a/README.md b/README.md index f58beb7..562102e 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ alive; see [Exit on Linux](#exit-on-linux). | Function | Behavior | | --- | --- | -| `sd_dart_new_sd_ctx` | `new_sd_ctx` that tracks the context before it returns. | +| `sd_dart_new_sd_ctx` | `new_sd_ctx` that tracks the context before it returns. A Metal build loads with batched CFG off ([Upstream qualification](#upstream-qualification)). | | `sd_dart_generate_image` | `generate_image` as a call in flight: teardown cancels it and waits for it. | | `sd_dart_cancel_generation` | `sd_cancel_generation` for a tracked context, from any thread. Does nothing for a context that is not tracked, including one already freed. Never blocks. | | `sd_dart_exit_free` | Untracks an object and frees it with the function it was tracked with (`free_sd_ctx` for a context). Does nothing when it is not tracked, so it also works as a Dart `NativeFinalizer`. | @@ -606,11 +606,17 @@ the macOS probe and builds the iOS device and simulator slices. ## Upstream qualification -The local upstream 982 candidate is preparation only; see the -[ABI audit, carry inventory and outstanding gates](docs/upstream_982_qualification.md). -Its larger context struct requires regenerated consumer bindings, and Metal -batched CFG remains unqualified. `tests/test_runtime_api.py` exercises a built -runtime's defaults and component conversion without models or a GPU. +Upstream 982 has its +[ABI audit, carry inventory, evidence and outstanding gates](docs/upstream_982_qualification.md). +Its larger context struct requires regenerated consumer bindings. +`tests/test_runtime_api.py` exercises a built runtime's defaults and component +conversion without models or a GPU. + +In a Metal build, the Apple targets, `sd_dart_new_sd_ctx` runs CFG unbatched +whatever `sd_ctx_params_t.batched_cfg` says: ggml-metal's group normalization +writes only the first batch item +([leejet/stable-diffusion.cpp#2149](https://github.com/leejet/stable-diffusion.cpp/issues/2149)), +so a batched CFG step returns a wrong image without an error. ## Release diff --git a/docs/upstream_982_qualification.md b/docs/upstream_982_qualification.md index fdb999d..27f45c8 100644 --- a/docs/upstream_982_qualification.md +++ b/docs/upstream_982_qualification.md @@ -1,9 +1,11 @@ -# Upstream 982 qualification — local preparation only +# Upstream 982 qualification -This branch prepares upstream `0518b34374761cb82aeda5ad3c2f0686fdad2cec` +This branch moves to upstream `0518b34374761cb82aeda5ad3c2f0686fdad2cec` (`master-982-0518b34`), with ggml `dc998771e700d49bbdf9a39cede22bb5b3311767`. -It is **not ready for publication or consumer adoption**. No source workaround -has enough evidence for retirement. The public baseline remains owner +It adds one carry, unbatched CFG in Metal builds (below), and retires none: +no source workaround has enough evidence for retirement. It is not released; +the platforms listed under "Remaining gates" are unqualified. The public +baseline remains owner `v0.2.0-4` / `c9922d960c79f7ba7381161ca6784f3b7c1e2fce`, upstream `3f8527a46c54ecf4cb4ed6003da8e8982283c73c` / ggml `89c4413f`. Live owner main and upstream master were refreshed on 2026-10-21. @@ -37,6 +39,9 @@ initializer writes beyond it. This is a **blocking consumer ABI gate**, even though older members keep their offsets. Bindings must be regenerated from released headers, and the mode/batching policy must be mapped explicitly. Null modes preserve the legacy flag route; adopting auto mode is separate. +In a Metal build `sd_dart_new_sd_ctx` loads with `batched_cfg=false` whatever +the caller passes, so a consumer that keeps the `true` default still gets +correct images there; raw `new_sd_ctx` does not. `convert_with_components` now appends `const sd_lora_t* loras, int lora_count`. The new runtime test compiles that exact function-pointer type and calls the @@ -78,6 +83,7 @@ v0.2.0-2, Linux host-exit retention in v0.2.0-3 and Metal fail-loud in v0.2.0-4. | GPU build-presence/status mapping / permanent | #17: an absent backend and a compiled backend finding no device are distinct states. | Candidate ggml registry still cannot represent the build-time distinction alone. | `test_device_memory` with/without backend definitions. **Keep.** | | Free-memory underflow/clamp / defensive mitigation | #17: Vulkan subtracts unsigned usage from budget; overuse wrapped near `SIZE_MAX`; clamp output to `[0,total]`. | ggml `65c60b239f02ab3fb35017af1e817d17a13e4952` now uses min(budget,heap), saturating subtraction. Included by SD `af0fc72` (#2147); earliest locally available containing release `master-973-f0aac52`. SD `60423ab` (master-961) reclaims before wrapped estimates; different layer. | Wrapper under/over-budget stand-ins pass. No real Vulkan comparison or artifact-level retirement proof. **Keep** cheap output invariant; upstream source overlap alone is insufficient. | | Metal fail-loud output discard/context poison / correctness mitigation | `ea60915` (#21), owner #19: command-buffer failure could return corrupt images as success. Published v0.2.0-4. | Candidate Metal still logs the same failure/status/error-state diagnostics; no verified upstream image-result cleanup/context poison equivalent. | Existing `test_exit_teardown` controlled failures/cleanup/success and `test_log` level/overflow controls retained. CPU image success passes. Physical failure unavailable. **Keep** conservative overlap invalidation. Separate from watchdog prevention (#1047). | +| Metal unbatched CFG / correctness mitigation (new) | This branch: upstream `1b0ba10` (#2085) runs both CFG branches in one batched UNet forward, on by default, and ggml-metal GROUP_NORM writes only batch item 0 ([#2149](https://github.com/leejet/stable-diffusion.cpp/issues/2149)). The wrong image is returned as success. | Candidate ggml-metal GROUP_NORM still dispatches `ngrp,1,1`; #2149 open. No upstream fix. | `test_exit_teardown` `batched-cfg` stand-in in default and `-DSD_DART_METAL=1` builds (fails with the guard removed); real Metal image controls below. **Keep** until a fixed ggml passes the batch-2 sentinel. | | Windows short upstream build + inherited `EP_PREFIX` / build workaround | #14: Vulkan shader generator hit FTK1011 at 260-character tlog paths; short upstream path alone failed. EP prefix reduced the recorded path to 213 chars. | Candidate still nests an ExternalProject shader build. No measured default-path Windows replacement. | Source retained; Windows build unavailable. **Keep.** | | Export allowlist / permanent | `935b70e` and #14: separate ggml copies must coexist without symbol collisions. Headers define public closure. | Candidate adds two public exports; does not remove collision risk. | Candidate macOS artifact exact export/dependency validation passes, no imported `__cxa_atexit`. Windows PE and ELF parser fixtures retained. **Keep.** | | Windows MSVC generator / build selection | Initial pipeline: Ninja could select runner MinGW rather than MSVC. | No evidence candidate changes runner compiler selection. | Tool configuration retained, no Windows run. **Keep.** | @@ -117,12 +123,40 @@ non-reproduction reported in #25 does not identify a causal fix or reconcile that input/environment. No large ZIPs were created/downloaded here, and the scanner patch must not be presented as fixing ZIP loading. -## Executed qualification and remaining gates +## Upstream fixes this upgrade brings -Built the baseline and candidate macOS-arm64 libraries using the owner build -tool; relocated ccache into the worktree after the sandbox refused its usual -home-directory cache. Candidate `validate_artifacts.py macos-arm64` passes. -No production wrapper source change was needed. +| Upstream | Change | Evidence here | +| --- | --- | --- | +| `8c442fb` (#2144) | Library opens model files through UTF-8 paths on Windows. | `tools/smoke_test.py` converts synthetic tensors between files in a non-ASCII directory on every smoke target, Windows CPU and Vulkan included; it passes on macOS arm64. Windows runs only in CI. | +| `2f016e8` (#2145) | The same for the CLI and server examples. | Not built here (examples off). | +| `60423ab` (#2124), `b7a74e6` (#2119) | Reclaim before trusting wrapped Vulkan iGPU memory estimates; demote resident params to disk when reclamation fails. | No Vulkan iGPU here; unqualified. The owner free-memory clamp stays. | + +## Metal batched CFG + +Upstream `1b0ba10` (#2085) added `batched_cfg`, default `true`. On Metal it +returns a wrong image as success, so every Apple build would regress for a +consumer that keeps the default. `sd_dart_new_sd_ctx` therefore loads a copy of +the parameters with `batched_cfg=false` when the library is built with Metal +(`SD_DART_METAL`, from `SD_METAL`). The caller's struct is not modified. Other +builds pass the flag through: CPU batched output is identical to unbatched. + +Executed on an Apple M4 Max, macOS arm64: + +| Control | Result | +| --- | --- | +| Model-free GROUP_NORM `[8,8,64,B]`, 32 groups, output prefilled with 123456, candidate ggml | CPU batch 1/2 all items written (max error 3e-7). MTL0 batch 1 and batch-2 item 0 correct; **batch-2 item 1 unchanged 4096/4096**. | +| SD-Turbo F16 + TAESD, 128×128, 1 Euler step, seed 7, CFG 2, raw `new_sd_ctx`, MTL0 | Unbatched: range 0..255, mean 129.74. **Batched: range 71..147, mean 108.89**, 46,710/49,152 bytes differ by more than 16 from unbatched. | +| Same, CPU | Batched and unbatched byte-identical to the published v0.2.0-4 CPU image (SHA-256 prefix `bf06aed231065595`). | +| Published v0.2.0-4 vs candidate unbatched, MTL0 | Mean absolute difference 0.15, max 5. | +| Guarded build, `sd_dart_new_sd_ctx` with `batched_cfg=true`, MTL0 | Byte-identical to the candidate's unbatched Metal image. Raw `new_sd_ctx` still reproduces the defect. | + +The `batched-cfg` stand-in in `tests/native/exit_teardown_test.cpp` runs in the +default build (flag passed through) and with `-DSD_DART_METAL=1` (copy with the +flag off, caller's struct unchanged); with the guard removed the Metal variant +fails. Vulkan, CUDA and other GPU backends keep batched CFG unqualified: no +device here, and #2149 names only Metal. + +## Executed qualification The checksum-verified published v0.2.0-4 archive and dylib are compared separately from the source rebuild. The archive SHA-256 is @@ -130,51 +164,32 @@ separately from the source rebuild. The archive SHA-256 is the dylib SHA-256 is `79ed1ab7c0451596bcb18070d459826a8b04409875873e91f39ffd84b2ffe1ce`. -The guarded native comparison uses existing SD-Turbo F16 GGUF + full TAESD, -CPU/four threads, 128×128, one Euler/discrete step, seed7, CFG2, prompt -`a lighthouse`. It verifies image/count/dimensions, writes raw RGB, frees via -`sd_dart_exit_free`, checks registry count1→0 and double-free no-op. Candidate -explicit mode controls use attention on/convolution off; an explicit -unbatched route and a batched CPU route are compared. This is a correctness -smoke, not a performance benchmark or new-family qualification. All four -RGB controls have SHA-256 -`bf06aed2310655953e6a403c264db320ac23aaf407e8f8e67fc7853a05839c86`. -The candidate's null-mode/legacy-flag route was also compared separately. - -The selected suite ran 48 tests: 44 passed, three Linux-only import/stdio -checks skipped on macOS, and the newly written conversion probe failed -because it supplied a null `tensor_type_rules` string. The existing smoke -recipe passes an empty string; the corrected probe now does so and passes -its targeted rerun (one test). The independent auditor reproduced the same -invalid-null failure on the baseline, so this was a harness error, not a -candidate regression. The combined completed evidence is 45 passing tests -and three explicit skips; the full suite was not repeated after that -one-argument test correction. Existing coverage includes native ASan/TSan -stand-ins and the pinned-Dart progress/shutdown stress controls. The native -model-dependent exit runtime harness was deliberately excluded. - -An initial whole-suite attempt used the default Flutter wrapper instead of -the pinned Dart SDK and auto-detected the library built during the run; it -was stopped and is not qualification evidence. All replacement tests used -the pinned Flutter 3.47.1 Dart binary. No remaining owned test jobs are left -running at the freeze. - -**Required before adoption:** exact-head independent review; regenerate and -map consumer bindings/default policy; obtain Metal GROUP_NORM batch1/batch2 -sentinels and real batched/unbatched image controls; run release and ASan -Metal teardown and Linux cooperative native/Dart teardown; qualify real -Vulkan memory/performance; audit every Apple slice's privacy/minimum-OS/ -XCFramework/SwiftPM/signing; complete platform packaging/export checks. - -Upstream [#2149](https://github.com/leejet/stable-diffusion.cpp/issues/2149) -is still open. Candidate Metal GROUP_NORM still dispatches `ngrp,1,1` and -passes dimensions0–2 only; its normalization kernel is unchanged from the -baseline. This source evidence plus the upstream reproduction means the new -`batched_cfg=true` default cannot be claimed supported on Metal. CPU parity -does not resolve that gate. FP8 CPU #2139/#2140 (first master-966) and Vulkan -convolution #2137/#2141 (first master-967) have upstream repair provenance, -but representative FP8/real-Vulkan runtime qualification was unavailable. -No unrelated model substitutes for those acceptance checks. - -No consumer pins, published artifacts, tags, workflows, issues or PRs were -changed. All existing primary/active investigation checkouts were preserved. +On the guarded candidate macOS arm64 build, with the pinned Flutter 3.47.1 +Dart SDK: + +| Check | Result | +| --- | --- | +| `validate_artifacts.py macos-arm64` | pass | +| `smoke_test.py macos-arm64 --expect-device MTL` | pass, including the non-ASCII path conversion | +| `unittest discover -s tests` with `SD_REQUIRE_DART`, `SD_REQUIRE_RUNTIME`, `SD_REQUIRE_METAL_GENERATION`, 3 runs | 57 tests, pass, 7 skipped (Linux-only ELF/stdio/host-shutdown, real model, libllamadart) | +| `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_SANITIZER=address`, Metal generation required | pass, 5 skipped (Linux, real model, libllamadart) | +| `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_MODEL` = SD-Turbo Q4_0 GGUF (256×256), Metal generation required, Dart VM harness | pass, 3 skipped (Linux, libllamadart); the untracked-context control aborted in ggml-metal at exit, as before | +| Same with `SD_EXIT_TEARDOWN_SANITIZER=address` | pass, 4 skipped (also the Dart VM, which cannot load a sanitized runtime) | + +Earlier guarded CPU controls (SD-Turbo F16 + TAESD, four threads) freed via +`sd_dart_exit_free` with registry count 1→0 and a double-free no-op. They are +correctness smokes, not performance benchmarks or new-family qualification. + +## Remaining gates + +- Exact-head independent review. +- Consumer: regenerate bindings from the released headers and map + `batched_cfg`, `flash_attn_mode` and `conv_direct_mode` explicitly. +- Linux cooperative native/Dart teardown with a built Linux target, release + and ASan. +- Windows: the Unicode path smoke runs only in CI; no Windows device run. +- Real Vulkan memory, convolution (#2137/#2141) and batched CFG; FP8 CPU + (#2139/#2140). +- Every Apple slice's privacy imports, minimum OS, XCFramework, SwiftPM and + signing; iOS devices. +- Android CPU on a device. diff --git a/src/sd_dart_exit.cpp b/src/sd_dart_exit.cpp index 55e71be..586b41f 100644 --- a/src/sd_dart_exit.cpp +++ b/src/sd_dart_exit.cpp @@ -17,6 +17,11 @@ #include #endif +// Set by the top-level CMakeLists.txt: whether Metal is compiled in. +#ifndef SD_DART_METAL +#define SD_DART_METAL 0 +#endif + namespace { struct Entry { @@ -412,6 +417,17 @@ SdDartStaticsCall::~SdDartStaticsCall() { sd_ctx_t* sd_dart_new_sd_ctx(const sd_ctx_params_t* sd_ctx_params) { Call call(kObjectCall | kWorkCall); ErrorWindow errors; +#if SD_DART_METAL + // ggml-metal's GROUP_NORM writes only the first batch item + // (leejet/stable-diffusion.cpp#2149), so batched CFG returns a wrong + // image as a success that no backend failure reveals. + sd_ctx_params_t unbatched; + if (sd_ctx_params != nullptr && sd_ctx_params->batched_cfg) { + unbatched = *sd_ctx_params; + unbatched.batched_cfg = false; + sd_ctx_params = &unbatched; + } +#endif sd_ctx_t* context = new_sd_ctx(sd_ctx_params); insert(context, free_context, cancel_context, SD_DART_EXIT_STAGE_CONTEXT); return context; diff --git a/src/sd_dart_wrapper.h b/src/sd_dart_wrapper.h index 0cf1d61..35b7d8a 100644 --- a/src/sd_dart_wrapper.h +++ b/src/sd_dart_wrapper.h @@ -210,6 +210,13 @@ SD_API void sd_dart_exit_teardown(void); // returns. Free the context with sd_dart_exit_free(). A load cannot be // cancelled: teardown waits for it, with the longer bound. Blocks after // teardown. +// +// In a library built with Metal, the Apple targets, the context runs CFG +// unbatched whatever sd_ctx_params_t.batched_cfg says, also on the CPU: +// ggml-metal's group normalization leaves every batch item after the first +// unwritten (leejet/stable-diffusion.cpp#2149), and the image a batched CFG +// step then returns is wrong without any error. The caller's parameters are +// not modified. SD_API sd_ctx_t* sd_dart_new_sd_ctx(const sd_ctx_params_t* sd_ctx_params); // generate_image() as a call in flight. Enables native log recording and diff --git a/tests/native/exit_teardown_test.cpp b/tests/native/exit_teardown_test.cpp index 2f0e5b6..28a0807 100644 --- a/tests/native/exit_teardown_test.cpp +++ b/tests/native/exit_teardown_test.cpp @@ -206,6 +206,8 @@ std::atomic loads_begun{0}; std::atomic calls_after_teardown{0}; // The arguments the stand-ins were last called with. std::atomic last_load_params{nullptr}; +std::atomic last_load_batched_cfg{false}; +std::atomic last_load_threads{0}; std::atomic last_request{nullptr}; std::atomic last_images_out{nullptr}; @@ -1789,6 +1791,8 @@ sd_ctx_t* new_sd_ctx(const sd_ctx_params_t* sd_ctx_params) { loads_begun.fetch_add(1); calls_after_teardown.fetch_add(teardown_returned.load() ? 1 : 0); last_load_params.store(sd_ctx_params); + last_load_batched_cfg.store(sd_ctx_params->batched_cfg); + last_load_threads.store(sd_ctx_params->n_threads); if (bool (*hold)() = hold_load.load()) { hold(); } @@ -1853,6 +1857,34 @@ void free_sd_images(sd_image_t* images, int count) { ++image_batches_freed; } +// A Metal build loads with batched CFG off from a copy of the caller's +// parameters; any other build passes them through. +int test_batched_cfg() { + sd_ctx_params_t params{}; + params.n_threads = 3; + params.batched_cfg = true; + sd_ctx_t* batched = sd_dart_new_sd_ctx(¶ms); + CHECK(batched != nullptr); + CHECK(sd_dart_exit_tracked_count() == 1); + CHECK(params.batched_cfg); + CHECK(last_load_threads.load() == 3); +#if SD_DART_METAL + CHECK(last_load_params.load() != ¶ms); + CHECK(!last_load_batched_cfg.load()); +#else + CHECK(last_load_params.load() == ¶ms); + CHECK(last_load_batched_cfg.load()); +#endif + params.batched_cfg = false; + sd_ctx_t* unbatched = sd_dart_new_sd_ctx(¶ms); + CHECK(last_load_params.load() == ¶ms); + CHECK(!last_load_batched_cfg.load()); + sd_dart_exit_free(unbatched); + sd_dart_exit_free(batched); + CHECK(sd_dart_exit_tracked_count() == 0); + return 0; +} + int main(int argc, char** argv) { const std::string scenario = argc > 1 ? argv[1] : ""; const struct { @@ -1893,6 +1925,7 @@ int main(int argc, char** argv) { {"last-error", test_last_error}, {"generation-backend-failure", test_generation_backend_failure}, {"log-at-exit", test_log_at_exit}, + {"batched-cfg", test_batched_cfg}, #if defined(__APPLE__) {"exit-in-flight", test_exit_in_flight}, #endif diff --git a/tests/test_exit_teardown.py b/tests/test_exit_teardown.py index 218c4fa..50b4a53 100644 --- a/tests/test_exit_teardown.py +++ b/tests/test_exit_teardown.py @@ -128,6 +128,14 @@ def supports(self, flag: str) -> bool: def test_tracks_waits_and_frees_in_order(self) -> None: self.build_and_run() + def test_metal_build_runs_cfg_unbatched(self) -> None: + with tempfile.TemporaryDirectory() as directory: + binary = Path(directory) / "exit_teardown_test" + subprocess.run(compile_command(binary, SOURCES, "-DSD_DART_METAL=1"), check=True) + result = subprocess.run([str(binary), "batched-cfg"], capture_output=True, text=True, + timeout=60) + self.assertEqual(0, result.returncode, result.stdout + result.stderr) + def test_scenarios_are_clean_under_sanitizers(self) -> None: for sanitizer in SANITIZERS: flag = f"-fsanitize={sanitizer}" diff --git a/tools/smoke_test.py b/tools/smoke_test.py index 058a9b6..6507c6f 100644 --- a/tools/smoke_test.py +++ b/tools/smoke_test.py @@ -7,8 +7,9 @@ Also checks the `sd_dart_wrapper.h` progress recorder against real progress, which converting a few synthetic tensors reports without needing a model, and -that the GPU device memory exports agree with the device list, and that a -load which fails leaves its reason with the log recorder. +that the GPU device memory exports agree with the device list, that a +load which fails leaves its reason with the log recorder, and that +conversion reads and writes files under a non-ASCII UTF-8 path. """ from __future__ import annotations @@ -207,6 +208,30 @@ def check_log_recording(library: Path) -> list[str]: return problems +def check_unicode_path(lib: ctypes.CDLL) -> list[str]: + """Converts synthetic tensors between files in a non-ASCII directory. + + Upstream takes UTF-8 paths; on Windows they must not go through the ANSI + code page. + """ + lib.str_to_sd_type.argtypes = [ctypes.c_char_p] + lib.convert.restype = ctypes.c_bool + lib.convert.argtypes = [ctypes.c_char_p, ctypes.c_char_p, ctypes.c_char_p, + ctypes.c_int, ctypes.c_char_p, ctypes.c_bool] + with tempfile.TemporaryDirectory() as work_dir: + directory = Path(work_dir) / "모델 ü" + directory.mkdir() + source, output = directory / "probe.safetensors", directory / "출력.gguf" + write_safetensors(source) + converted = lib.convert(str(source).encode(), None, str(output).encode(), + lib.str_to_sd_type(b"f32"), b"", False) + magic = output.read_bytes()[:4] if output.is_file() else b"" + if not converted or magic != b"GGUF": + return [f"converting under a non-ASCII path returned {converted} and wrote {magic!r}"] + print(" non-ASCII UTF-8 path conversion ok") + return [] + + def host_memory() -> int: return os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") @@ -328,6 +353,7 @@ def main() -> None: [device.split("\t")[0] for device in devices], args.expect_device) problems += check_progress_recording(library) problems += check_log_recording(library) + problems += check_unicode_path(lib) for problem in problems: print(f"error: {problem}", file=sys.stderr) if problems: From 3ae6f283d709eb05bc1590c303e92a37dd3ea160 Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Thu, 22 Oct 2026 09:04:00 -0400 Subject: [PATCH 3/4] fix: document auto attention modes and check the Metal guard wiring A null flash_attn_mode or conv_direct_mode with its legacy flag false selects upstream's auto probes, which change output and are not seed-reproducible on Metal. Record the explicit consumer mapping that keeps v0.2.0-4 behavior. build.py now fails after configure when stable-diffusion lacks the SD_DART_METAL or SD_DART_GPU_BACKEND value its target needs, and the CI build jobs run the runtime API test against the built runtime instead of skipping it. --- .github/workflows/validate.yml | 6 ++++++ CMakeLists.txt | 5 +++++ README.md | 6 +++++- docs/upstream_982_qualification.md | 33 ++++++++++++++++++++++++++++-- tests/test_runtime_api.py | 10 +++++++-- tests/test_tools.py | 20 +++++++++++++++++- tools/build.py | 18 ++++++++++++++++ 7 files changed, 92 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 12b0b36..df1516f 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -122,6 +122,12 @@ jobs: env: # ggml skips CPU-type Vulkan devices such as lavapipe unless listed. GGML_VK_VISIBLE_DEVICES: "0" + - name: Check the context defaults and conversion ABI + if: runner.os != 'Windows' && matrix.smoke + run: python3 -m unittest discover -s tests -p test_runtime_api.py -v + env: + SD_RUNTIME_API_TARGET: ${{ matrix.target }} + SD_REQUIRE_RUNTIME: "1" - uses: dart-lang/setup-dart@v1 if: runner.os == 'Linux' && matrix.smoke - name: Verify cooperative host shutdown diff --git a/CMakeLists.txt b/CMakeLists.txt index ab3b3a9..9cb7067 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,3 +43,8 @@ endif() target_compile_definitions(stable-diffusion PRIVATE SD_DART_GPU_BACKEND=${SD_DART_GPU_BACKEND} SD_DART_METAL=${SD_DART_METAL}) + +# tools/build.py checks these: a missing definition falls back to 0 in the +# sources and would drop the Metal guard without a build error. +file(GENERATE OUTPUT "${CMAKE_BINARY_DIR}/sd_dart_definitions-$.txt" + CONTENT "$,\n>\n") diff --git a/README.md b/README.md index 562102e..e6401c2 100644 --- a/README.md +++ b/README.md @@ -608,7 +608,11 @@ the macOS probe and builds the iOS device and simulator slices. Upstream 982 has its [ABI audit, carry inventory, evidence and outstanding gates](docs/upstream_982_qualification.md). -Its larger context struct requires regenerated consumer bindings. +Its larger context struct requires regenerated consumer bindings. A null +`flash_attn_mode` or `conv_direct_mode` with its legacy flags `false`, the +initializer default, now selects upstream's auto probes, which change output +and are not seed-reproducible on Metal; to keep earlier behavior, pass the +explicit modes the document maps from the legacy flags. `tests/test_runtime_api.py` exercises a built runtime's defaults and component conversion without models or a GPU. diff --git a/docs/upstream_982_qualification.md b/docs/upstream_982_qualification.md index 27f45c8..174c989 100644 --- a/docs/upstream_982_qualification.md +++ b/docs/upstream_982_qualification.md @@ -38,7 +38,35 @@ The added pointers make an old 320-byte Dart allocation unsafe: the candidate initializer writes beyond it. This is a **blocking consumer ABI gate**, even though older members keep their offsets. Bindings must be regenerated from released headers, and the mode/batching policy must be mapped explicitly. -Null modes preserve the legacy flag route; adopting auto mode is separate. + +A null or empty mode honors a legacy flag only when that flag is `true`. With +the flag `false`, the `sd_ctx_params_init` default and what llamadart writes +today, upstream runs its **auto** probes (`flash_attention_for` and +`conv_direct_for` in `diffusion_engine.cpp`). Auto changes output against +v0.2.0-4, and on Metal it is not seed-reproducible: flash attention is chosen +by a timing probe at load. + +| SD-Turbo F16 + TAESD, 128², 1 step, seed 7, CFG 2 | Image SHA-256 prefix | +| --- | --- | +| v0.2.0-4 CPU, flags false / `diffusion_flash_attn` true | `4ca7bc91` / `bf06aed2` | +| Candidate CPU, null modes, flags false (auto picks flash on) | `bf06aed2` | +| Candidate CPU, `"off"` / `"off,diffusion=on"` | `4ca7bc91` / `bf06aed2` | +| Candidate MTL0, null modes, flags false | auditor saw flash on in 2 of 10 loads, with different images | +| Candidate MTL0, `"off"` / `"off,diffusion=on"`, three loads each | `4e51cf6f` ×3 / `7f798ea8` ×3, no auto probe logged | + +To keep v0.2.0-4 behavior the consumer must always pass both modes, +never null, mapped from its existing flags: + +| v0.2.0-4 flags | `flash_attn_mode` | +| --- | --- | +| `flash_attn` | `"off,te=on,clip_vision=on,vae=on,diffusion=on"` (v0.2.0-4 left ControlNet off) | +| only `diffusion_flash_attn` | `"off,diffusion=on"` | +| neither | `"off"` | + +`conv_direct_mode` is `"off"`, plus `",diffusion=on,control_net=on"` for +`diffusion_conv_direct` and `",vae=on"` for `vae_conv_direct`. The legacy +booleans can stay as they are; an explicit mode overrides them. `"auto"` +remains available as a separate, opt-in consumer feature. In a Metal build `sd_dart_new_sd_ctx` loads with `batched_cfg=false` whatever the caller passes, so a consumer that keeps the `true` default still gets correct images there; raw `new_sd_ctx` does not. @@ -170,8 +198,9 @@ Dart SDK: | Check | Result | | --- | --- | | `validate_artifacts.py macos-arm64` | pass | +| `build.py build --target macos-arm64` definitions check | `SD_DART_METAL=1`, `SD_DART_GPU_BACKEND=1`; with `SD_DART_METAL` dropped from `CMakeLists.txt` the build stops after configure | | `smoke_test.py macos-arm64 --expect-device MTL` | pass, including the non-ASCII path conversion | -| `unittest discover -s tests` with `SD_REQUIRE_DART`, `SD_REQUIRE_RUNTIME`, `SD_REQUIRE_METAL_GENERATION`, 3 runs | 57 tests, pass, 7 skipped (Linux-only ELF/stdio/host-shutdown, real model, libllamadart) | +| `unittest discover -s tests` with `SD_REQUIRE_DART`, `SD_REQUIRE_RUNTIME`, `SD_REQUIRE_METAL_GENERATION`, 3 runs | 59 tests, pass, 7 skipped (Linux-only ELF/stdio/host-shutdown, real model, libllamadart) | | `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_SANITIZER=address`, Metal generation required | pass, 5 skipped (Linux, real model, libllamadart) | | `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_MODEL` = SD-Turbo Q4_0 GGUF (256×256), Metal generation required, Dart VM harness | pass, 3 skipped (Linux, libllamadart); the untracked-context control aborted in ggml-metal at exit, as before | | Same with `SD_EXIT_TEARDOWN_SANITIZER=address` | pass, 4 skipped (also the Dart VM, which cannot load a sanitized runtime) | diff --git a/tests/test_runtime_api.py b/tests/test_runtime_api.py index 41a7418..762eb48 100644 --- a/tests/test_runtime_api.py +++ b/tests/test_runtime_api.py @@ -16,16 +16,22 @@ class RuntimeApiTest(unittest.TestCase): + def skip_unless_required(self, reason: str) -> None: + """Skips, or fails with SD_REQUIRE_RUNTIME=1, as the CI build jobs set.""" + if os.environ.get("SD_REQUIRE_RUNTIME"): + self.fail(reason) + self.skipTest(reason) + def test_context_defaults_and_component_conversion(self): target_name = os.environ.get("SD_RUNTIME_API_TARGET", "") if not target_name and sys.platform == "darwin": target_name = "macos-arm64" if platform.machine() == "arm64" else "macos-x64" if not target_name or sys.platform == "win32": - self.skipTest("set SD_RUNTIME_API_TARGET to a built macOS or Linux target") + self.skip_unless_required("set SD_RUNTIME_API_TARGET to a built macOS or Linux target") target = build.TARGETS[target_name] library = build.BIN_ROOT / target.name / "lib" / target.library if not library.is_file(): - self.skipTest(f"no built runtime for {target.name}") + self.skip_unless_required(f"no built runtime for {target.name}") with tempfile.TemporaryDirectory() as directory: directory = Path(directory) source = directory / "probe.safetensors" diff --git a/tests/test_tools.py b/tests/test_tools.py index d24cae9..d945f66 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -8,7 +8,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "tools")) import apple_xcframework # noqa: E402 -from build import HEADERS, TARGETS, WRAPPER_HEADER # noqa: E402 +from build import HEADERS, TARGETS, WRAPPER_HEADER, wrapper_definition_problems # noqa: E402 from package_release import TAG_PATTERN # noqa: E402 from sd_api import api_symbols # noqa: E402 from validate_artifacts import ( # noqa: E402 @@ -195,6 +195,24 @@ def test_native_hosts_declare_their_cpu(self): if target.os in ("linux", "windows"): self.assertTrue(target.host_arch, target.name) + def test_metal_targets_must_configure_the_metal_guard(self): + apple = TARGETS["ios-arm64"] + self.assertEqual([], wrapper_definition_problems( + apple, {"SD_DART_METAL=1", "SD_DART_GPU_BACKEND=1", "GGML_SHARED"})) + for definitions in ({"SD_DART_GPU_BACKEND=1"}, + {"SD_DART_METAL=0", "SD_DART_GPU_BACKEND=1"}): + self.assertEqual(["ios-arm64 configured stable-diffusion without SD_DART_METAL=1"], + wrapper_definition_problems(apple, definitions)) + + def test_other_targets_must_leave_it_off(self): + self.assertEqual([], wrapper_definition_problems( + TARGETS["linux-x64-vulkan"], {"SD_DART_METAL=0", "SD_DART_GPU_BACKEND=1"})) + self.assertEqual([], wrapper_definition_problems( + TARGETS["android-arm64"], {"SD_DART_METAL=0", "SD_DART_GPU_BACKEND=0"})) + self.assertEqual(["windows-x64 configured stable-diffusion without SD_DART_METAL=0"], + wrapper_definition_problems( + TARGETS["windows-x64"], {"SD_DART_METAL=1", "SD_DART_GPU_BACKEND=0"})) + class AppleXcframeworkTest(unittest.TestCase): def test_every_apple_target_lands_in_one_slice(self): diff --git a/tools/build.py b/tools/build.py index 3009941..75a0408 100644 --- a/tools/build.py +++ b/tools/build.py @@ -184,6 +184,23 @@ def configure_args(target: Target, work_dir: Path) -> list[str]: return args +def wrapper_definition_problems(target: Target, definitions: set[str]) -> list[str]: + """What the wrapper sources were configured with that this target contradicts.""" + expected = { + f"SD_DART_METAL={1 if 'metal' in target.accelerators else 0}", + f"SD_DART_GPU_BACKEND={1 if set(target.accelerators) - {'cpu'} else 0}", + } + return [f"{target.name} configured stable-diffusion without {definition}" + for definition in sorted(expected - definitions)] + + +def check_wrapper_definitions(target: Target, work_dir: Path) -> None: + path = work_dir / "sd_dart_definitions-Release.txt" + definitions = set(path.read_text().split()) if path.is_file() else set() + for problem in wrapper_definition_problems(target, definitions): + fail(problem) + + def find_library(build_dir: Path, library: str) -> Path: matches = [p for p in build_dir.rglob(library) if p.is_file() and not p.is_symlink()] if not matches: @@ -216,6 +233,7 @@ def build(target: Target, jobs: int) -> Path: work_dir.mkdir(parents=True, exist_ok=True) cmake_args = configure_args(target, work_dir) run(["cmake", "-S", str(REPO_ROOT), "-B", str(work_dir), *cmake_args]) + check_wrapper_definitions(target, work_dir) run(["cmake", "--build", str(work_dir), "--config", "Release", "-j", str(jobs)]) out_dir = BIN_ROOT / target.name From 9f26f8bafda30a77b4b6fd7b552aaf2cb2ee6504 Mon Sep 17 00:00:00 2001 From: Jhin Lee Date: Thu, 22 Oct 2026 09:13:53 -0400 Subject: [PATCH 4/4] docs: list the carried pickle patch beside the Metal CFG guard --- README.md | 3 +++ docs/upstream_982_qualification.md | 36 +++++++++++++++++------------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0cf1f0e..341074e 100644 --- a/README.md +++ b/README.md @@ -609,6 +609,9 @@ the macOS probe and builds the iOS device and simulator slices. Upstream 982 has its [ABI audit, carry inventory, evidence and outstanding gates](docs/upstream_982_qualification.md). +It carries two fixes the pin lacks: unbatched CFG in Metal builds (below) and +the [`pickle_line_span.patch`](patches/README.md) for legacy checkpoints above +2 GiB. Its larger context struct requires regenerated consumer bindings. A null `flash_attn_mode` or `conv_direct_mode` with its legacy flags `false`, the initializer default, now selects upstream's auto probes, which change output diff --git a/docs/upstream_982_qualification.md b/docs/upstream_982_qualification.md index 174c989..8cfa301 100644 --- a/docs/upstream_982_qualification.md +++ b/docs/upstream_982_qualification.md @@ -2,8 +2,11 @@ This branch moves to upstream `0518b34374761cb82aeda5ad3c2f0686fdad2cec` (`master-982-0518b34`), with ggml `dc998771e700d49bbdf9a39cede22bb5b3311767`. -It adds one carry, unbatched CFG in Metal builds (below), and retires none: -no source workaround has enough evidence for retirement. It is not released; +It carries two fixes the pin lacks: unbatched CFG in Metal builds, a wrapper +change (below), and `patches/pickle_line_span.patch` for +[#23](https://github.com/leehack/stable-diffusion-native/issues/23), merged in +[#26](https://github.com/leehack/stable-diffusion-native/pull/26). It retires +none: no source workaround has enough evidence for retirement. It is not released; the platforms listed under "Remaining gates" are unqualified. The public baseline remains owner `v0.2.0-4` / `c9922d960c79f7ba7381161ca6784f3b7c1e2fce`, upstream @@ -98,7 +101,7 @@ v0.2.0-2, Linux host-exit retention in v0.2.0-3 and Metal fail-loud in v0.2.0-4. | Carry / class | Purpose, original trigger and owner provenance | Affected revisions / upstream equivalent and earliest evidence | Tests and decision | | --- | --- | --- | --- | -| Vendored source patches | None: the owner stores a submodule, not an applied patch stack. Upstream's ggml fork is upstream content. | Both baseline and candidate submodules are clean. | **None to remove.** Do not mistake upstream `af0fc72` removing its conv1d workaround for retirement of an owner patch. | +| Carried upstream patch `pickle_line_span.patch` / correctness fix | [#26](https://github.com/leehack/stable-diffusion-native/pull/26) for [#23](https://github.com/leehack/stable-diffusion-native/issues/23): the legacy pickle scanner narrows the remaining span to `int`, so checkpoints above 2 GiB are rejected. `CMakeLists.txt` patches a build-tree copy of `src/model_io/pickle_io.cpp`; the submodule stays clean. | Candidate `pickle_io.cpp` is byte-identical to the baseline; no upstream fix. The patch applies to `0518b34` and the macOS build reports `Carrying patches/pickle_line_span.patch`; build info lists it. | `test_pickle_span` passes against the candidate. **Keep.** It is the only patch; upstream's ggml fork is upstream content, and upstream `af0fc72` removing its conv1d workaround retires no owner patch. | | Ordered native progress ring / permanent | `ea53d85` (#14): Dart callbacks after shutdown aborted; callback locks deadlocked VM shutdown; latest-only polling lost batch transitions. | All owner builds since #14. Candidate still has a process-global native callback, without a managed-runtime polling/lifetime replacement. No equivalent release. | `test_progress`, `test_progress_stress`, real CPU generation. **Keep** all history, atomics and registration lifetime. | | Native log/error ring / permanent | `6eb1bac` (#17): upstream log threads cannot safely call Dart; failed loads return null without a reason. | All owner builds since #17. Upstream callback still borrows message storage; no equivalent. | `test_log`, missing-model/exit stand-ins. **Keep** bounded error window, UTF-8 boundaries and dropped-message accounting. | | Log spin abandonment / mitigation | #17: Windows `ExitProcess` can kill a flag holder; a descheduled reader formerly hid load errors. Separate error flag and 100 ms bound prevent indefinite waits. | Candidate has no equivalent recorder or holder-death protocol. | `test_log` plain, ASan and TSan controls; Windows execution remains unavailable. **Keep.** | @@ -125,8 +128,10 @@ v0.2.0-2, Linux host-exit retention in v0.2.0-3 and Metal fail-loud in v0.2.0-4. ## Reader controls and issue boundaries Owner [#23](https://github.com/leehack/stable-diffusion-native/issues/23) -remains reproducible. Baseline and candidate `pickle_io.cpp` and `binary_io.h` -are byte-identical. Using the native fixture from draft +reproduces on the unpatched candidate: baseline and candidate `pickle_io.cpp` +and `binary_io.h` are byte-identical. This branch carries the fix from +[#26](https://github.com/leehack/stable-diffusion-native/pull/26) (see the +inventory). Controls run before #26 merged, with the native fixture from [#25](https://github.com/leehack/stable-diffusion-native/pull/25), exact head `d9bacef31e2569d5df4c3e508caf278d704280eb`: @@ -137,13 +142,10 @@ are byte-identical. Using the native fixture from draft | Same valid prefixes in those large containing spans | fail at GLOBAL | fail at GLOBAL | pass all cases | | Bounded inputs with ASan | same source as candidate | pass | pass | -The diagnostic copy makes `find_char` accept `size_t`, caps its int-returning -scan at `INT_MAX`, and removes all 13 narrowing remaining-span casts. It is -outside the submodule and **not part of this branch's runtime**. This proves -the scanner mechanism, not a shipped fix. PR #25 remains test-only with its -expected failure; no duplicate test PR or upstream submission was made. -The original trusted >2 GiB real-checkpoint/public-Dart evidence is in #23; -this run does not repeat the huge-checkpoint conversion or loading. +That diagnostic copy proved the scanner mechanism; the shipped fix is #26's +patch, whose `test_pickle_span` passes on this branch. This branch does not +repeat the trusted >2 GiB real-checkpoint conversion or public-Dart load +recorded in #23. Owner [#24](https://github.com/leehack/stable-diffusion-native/issues/24) remains unresolved: its original full ZIP load timed out at 45 s. Later @@ -192,15 +194,19 @@ separately from the source rebuild. The archive SHA-256 is the dylib SHA-256 is `79ed1ab7c0451596bcb18070d459826a8b04409875873e91f39ffd84b2ffe1ce`. -On the guarded candidate macOS arm64 build, with the pinned Flutter 3.47.1 -Dart SDK: +On the guarded candidate macOS arm64 build with #26's patch (dylib SHA-256 +`e4e1a2be2271ae1dc3fb1c31cae156b926c49cc98bc581c4544803fd46917b90`), with the +pinned Flutter 3.47.1 Dart SDK. The real-model rows ran on the build before +the merge, which differs only in the patched legacy pickle reader; the Metal +guard control was repeated on the merged build with the same images. | Check | Result | | --- | --- | | `validate_artifacts.py macos-arm64` | pass | +| `test_pickle_span.py` after merging #26 | 5 tests pass; patch applies to `0518b34` | | `build.py build --target macos-arm64` definitions check | `SD_DART_METAL=1`, `SD_DART_GPU_BACKEND=1`; with `SD_DART_METAL` dropped from `CMakeLists.txt` the build stops after configure | | `smoke_test.py macos-arm64 --expect-device MTL` | pass, including the non-ASCII path conversion | -| `unittest discover -s tests` with `SD_REQUIRE_DART`, `SD_REQUIRE_RUNTIME`, `SD_REQUIRE_METAL_GENERATION`, 3 runs | 59 tests, pass, 7 skipped (Linux-only ELF/stdio/host-shutdown, real model, libllamadart) | +| `unittest discover -s tests` with `SD_REQUIRE_DART`, `SD_REQUIRE_RUNTIME`, `SD_REQUIRE_METAL_GENERATION`, 3 runs | 64 tests, pass, 7 skipped (Linux-only ELF/stdio/host-shutdown, real model, libllamadart) | | `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_SANITIZER=address`, Metal generation required | pass, 5 skipped (Linux, real model, libllamadart) | | `test_exit_teardown_runtime.py` with `SD_EXIT_TEARDOWN_MODEL` = SD-Turbo Q4_0 GGUF (256×256), Metal generation required, Dart VM harness | pass, 3 skipped (Linux, libllamadart); the untracked-context control aborted in ggml-metal at exit, as before | | Same with `SD_EXIT_TEARDOWN_SANITIZER=address` | pass, 4 skipped (also the Dart VM, which cannot load a sanitized runtime) |