fix(rocm): validate MLX_ROCM_FFT_CACHE_SIZE (0 is UB, junk throws) #2051

Part of #1801

Problem / Background

Found by the security review of PR #2049 (#1876), but already present on main. MLX_ROCM_FFT_CACHE_SIZE is parsed with an unchecked std::stoul, so a mistyped value breaks the first device FFT (Kokoro TTS, Phi-4-multimodal audio) instead of being rejected with a message. This is fork-side overlay code, so the fix is an upstreaming candidate for #1813.

Current Behavior

src/lib/mlx-cpp/patches-rocm/mlx/backend/rocm/lru_cache.h:18-23: the LRUBytesKeyCache constructor does capacity_ = std::stoul(env) with no validation. Its only env-driven user is fft_plan_cache() in fft.hip:137-142 (a function-local static, default 8 on main, 128 after #2049). Per value, confirmed with a host g++ 14 probe of the header:

• 0: accepted. The first put() (lru_cache.h:47-51) runs while (cache_list_.size() >= capacity_) with 0 >= 0 true, so it calls back() and pop_back() on an empty std::list: undefined behavior. The probe threw std::bad_alloc from copying the garbage node; with -D_GLIBCXX_ASSERTIONS it aborts on Assertion '!this->empty()'.
• Non-numeric (abc) or set but empty: std::stoul throws std::invalid_argument whose what() is only stoul, with no variable name. It escapes the static's initializer out of FFT::eval_gpu; the static stays uninitialized, so every later FFT throws again.
• Out of range (99999999999999999999999): std::out_of_range, also stoul.
• Negative (-1): accepted silently. stoul negates, giving SIZE_MAX, so the cache never evicts.
• Trailing junk (8abc): accepted silently as 8.

Upstream CUDA at the pinned MLX 81ba1c6a (mlx/backend/cuda/lru_cache.h:28-40, mlx/utils.cpp:294-300) reads MLX_CUDA_FFT_CACHE_SIZE with atoi: 0 or non-numeric yields 0 and the constructor throws std::runtime_error("LRUCache requires capacity > 0."), while -1 becomes SIZE_MAX. It does not fail silently on 0, but it still throws and still accepts negatives, so copying it exactly is rejected.

Proposed Solution

Add inline size_t capacity_from_env(const char* env_var, size_t default_capacity) in lru_cache.h (namespace mlx::core::rocm) and use it in the LRUBytesKeyCache constructor. Use the same parsing rules as gpu_watchdog_seconds() in device.cpp:453-476: std::strtol base 10, and the whole string must be a decimal integer. Unset returns the default with no output. Accept 1 to std::numeric_limits<int>::max() (the bound CUDA's int get_var implies). Otherwise (empty, trailing junk, ERANGE, <= 0), print one line to stderr, [ROCm] ignoring invalid MLX_ROCM_FFT_CACHE_SIZE="<value>" (expected a positive integer); using the default <N>, and return the default. The warning prints once, because the static then initializes successfully. As defense in depth, both constructors (LRUBytesKeyCache and LRUCache(size_t), whose graph_cache_{400} in device.h is constant) throw std::runtime_error("LRUCache requires capacity > 0.") on 0, as CUDA does. Clamping instead of rejecting was rejected: a warning plus the default is what MLX_ROCM_GPU_WATCHDOG_SECS does, and one convention is easier to document.

Scope

• In scope: lru_cache.h, a new integration test, patches-rocm/LOCAL_FIXES.md, docs/environment-variables.md.
• Out of scope: MLX_CUDA_FFT_CACHE_SIZE (upstream code), changing the default (fix(rocm): set blocking-sync before first HIP queue, FFT cache to 128 #2049), fork submission (chore(rocm): mlxcelverse ROCm fork sync script, MLX pin-bump procedure, and upstreaming local fixes #1813).

Implementation Notes

• Reuse: copy the strtol checks from gpu_watchdog_seconds(); do not add a second style of validation.
• Constraints: header-only; no MLX env::get_var (it uses atoi).
• Edge cases: leading whitespace and + are accepted by strtol, which is acceptable; 0x10 is rejected as trailing junk.

Acceptance Criteria

• 0, -1, abc, empty, 8abc and an overflowing value each print exactly one warning line naming the variable and the default, and the first rfft on the GPU succeeds and matches the CPU stream.
• 16 prints no warning and FFT matches the CPU stream; unset behaves as before.
• New tests/rocm_fft_cache_env.rs (#![cfg(feature = "rocm")], skip unless gpu_backend_kind() is ROCm) runs one child per value by re-invoking its own binary on an ignored child test with the env var set, as tests/rocm_gpu_faults.rs does, and asserts child exit status and stderr.
• patches-rocm/LOCAL_FIXES.md gets a numbered entry (next free number) describing the old behavior per value, the new rule, and "Applies to the fork; to be proposed there."
• docs/environment-variables.md (the ROCm paragraph near line 192) states the valid range (a positive integer up to 2147483647) and that other values warn and use the default.

Verification

cargo build --features rocm
cargo test --features rocm --test rocm_fft_cache_env
cargo test --features rocm --test rocm_gpu_faults
cargo clippy --features rocm --all-targets -- -D warnings
cargo fmt --check

A pass is every child exiting 0 with the expected stderr, on a ROCm host (measured so far on gfx1151).