Skip to content

feat: upgrade llama.cpp to b10976, guard the JNI exception boundary, smoke the aarch64 fat jars - #437

Merged
bernardladenthin merged 6 commits into
mainfrom
claude/serene-goodall-cq2rgc
Sep 15, 2026
Merged

bernardladenthin merged 6 commits into
mainfrom
claude/serene-goodall-cq2rgc

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Upgrade llama.cpp b10948 → b10976 in two reviewable chunks (the full diff is 121 KiB, over the runbook's 100 KiB threshold). No project source change; no priority-list header moved; the request-field, CLI-option and response-key sets are all byte-identical across the whole range. The patch set stays at nine — nothing dropped, nothing refreshed. Headline: upstream reverses its precompiled-header experiment (#28892, #28882), which is a heap-buffer-overflow fix this project shipped — PCH arrived at b10917, so pins b10919–b10948 carried it.
  • Close the JNI exception boundary. Of the 40 Java_* entry points, exactly one had a catch (...). An exception escaping a native method across the JNI boundary is undefined behaviour and aborts the JVM. All 39 wrappable entry points now run inside a shared guard; the three that cannot (two lifecycle hooks and the trainer) use a function-try-block.
  • Smoke the two aarch64 fat jars. all-linux-aarch64 and all-windows-aarch64 were built, GPG-signed and attached to every release while publish.yml referenced them zero times — exactly what fat-jar-release-assets.md forbids, and that rule exists because a corrupt macOS dylib shipped in three releases under a green pipeline.
  • Four audit gaps closed: a model-free guard for patches/0010, tests for the close()-vs-inference use-after-free defence, OSInfo.archMapping assertions, and deletion of a dead helper class.

Test plan

  • Affected unit / integration tests pass locally
  • CI is green on this branch
  • Docs / CHANGELOG updated where applicable

Verified locally on Linux x86_64:

Check Result
Fresh cmake configure through the real FetchContent path clean; stamp at 987498f (= b10976) with nine SHA-256 lines
Wire-name extraction unchanged: 138 CLI / 57 request / 15 trainer
Release build 0 errors, 0 warnings
ctest 551/551 (537 at the bump + 7 JNI-guard + 7 context-guard)
nm -D 40 Java_* exports, 0 mangled
mvn test 1758 Java tests, 0 failures (OSInfoTest 19/19)
NativeLibraryLoadSmokeTest 4/4, 0 skipped — cross-validates the bumped pin against the linked build-info
spotbugs:check / spotless / clang-format 22.1.8 clean

Not verified locally, and this is the point of the CI run: the model-backed Java suite (no GGUF access in the sandbox), every non-Linux platform, and the two new aarch64 smoke jobs, which have never executed.

Two changes were falsified, not just asserted

A check that cannot fail is worthless, so both new guards were driven red before being wired in:

  • verify-patches-applied.sh — reverting only the patches/0010 cast exits 1; reverting the tree with the stamp intact exits 1; the intact tree exits 0.
  • The context-guard tests — deleting the fetch_add from acquire_jllama_context_impl turns 2 of them red, green again once restored.

The jllama.cpp diff is large because every body gained an indent level. It is provably mechanical: a token-level comparison against the previous file removes zero tokens — every added run is the guard wrapper or a function-try-block.

Related issues / PRs

Refs the 0013 drop at b10948 (upstream merged this project's own ggml-org/llama.cpp#28775).

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits
  • No security-sensitive changes

Generated by Claude Code

First of two reviewable chunks toward the b10976 release (the full
b10948..b10976 diff is 121 KiB, over the runbook's 100 KiB per-step
threshold; this step is 99 KiB / 21 commits).

Upstream reverses the precompiled-header experiment (#28892, #28882).
That is a correctness fix this project shipped, not housekeeping: PCH
arrived at b10917, so every pin from b10919 through b10948 carried a
heap-buffer-overflow in the rope work buffer (upstream #28858). The PCH
force-included ggml-impl.h ahead of ops.h, defining
__cpp_lib_hardware_interference_size, so the C++ kernels resolved
CACHE_LINE_SIZE to std::hardware_destructive_interference_size while
ggml-cpu.c -- a C TU, where that macro can never be defined -- kept the
64 fallback. The buffer is sized by the C side and written by the C++
side. The deleted ggml-cpu guard skipped PCH for GCC on non-x86, so the
s390x cross and aarch64-native GCC jobs were never exposed; the clang,
AppleClang, clang-cl, MSVC and GCC-on-x86 jobs were.

No project source change. No priority-list header moved; common/common.cpp
is the only priority-list file touched and only its implementation (#28749
hoists the llama_n_rs_seq check above the probe decode in
common_context_can_seq_rm -- same enum, same signature, one fewer decode).
Zero tools/server/*.{cpp,h}: every server contract file is byte-identical
by blob hash across the whole b10948..b10976 walk, so the three mechanical
server-contract greps have no input to compare.

All four CMake removals that reach this build are removals, so nothing had
to be added here; tools/server/CMakeLists.txt is not processed at all
(LLAMA_BUILD_TOOLS/SERVER are forced OFF, the server TUs compile straight
into jllama).

All nine patches apply, verified against a pristine b10976 worktree up
front and independently against pristine b10969 so this intermediate
commit is a valid bisect point. #28776 is the sequel to the previous
bump's 0013 drop: upstream added a non-VXE s390x build to their own CI,
so the scalar configuration build-linux-s390x builds is now guarded
upstream too.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP
Final chunk of the two-step walk, reaching the target release (20 KiB,
7 commits). Nothing under common/, include/, src/ or tools/server/*.{cpp,h}.

#28771 retargets five CMake path references from CMAKE_SOURCE_DIR to
PROJECT_SOURCE_DIR and extends examples/test-cmake to cover subproject
consumption. That is exactly the bug class that bites a FetchContent
subproject like this one -- CMAKE_SOURCE_DIR resolves to the top-level
project, here llama/, not llama.cpp's own root -- but none of the five
repaired files is processed by this build (LLAMA_BUILD_TOOLS/SERVER/APP
are forced OFF, tests/ and examples/ are never added, and tools/mtmd was
already correct). Inert today, one less latent trap tomorrow.

Backend work is classifier-scoped and additive in effect: #28897 widens
CUDA GGML_OP_DUP to i16/i32 for cuda13-*, and #28576 switches HIP
flash-attention MMA to fp32 accumulation on AMD MFMA hardware, which no
GitHub-hosted runner has -- the rocm-* jobs are build-only by design.
#28646 is WebUI, auto-followed by build-webui from the pinned GIT_TAG.

The patch set is unchanged at nine: nothing dropped, nothing refreshed.
All nine apply into a pristine b10976 worktree, and every standing
drop-check still reports "still required" against the pristine tag,
which the fail-loud applier cannot do for itself (it detects "does not
apply", never "upstream already fixed this").

Verified end to end: fresh configure through the real FetchContent path,
stamp at head 987498f4592a76897863cf53711dce38380c082b with nine SHA-256
lines; wire-name extraction unchanged at 138 CLI / 57 request / 15
trainer; full Release build clean with zero errors; ctest 537/537;
nm -D shows 40 Java_* exports and 0 mangled; NativeLibraryLoadSmokeTest
4/4 with 0 skipped, cross-validating the bumped LLAMA_CPP_VERSION
constant against the linked build-info.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP
An exception that escapes a native method and unwinds across the JNI
boundary is undefined behaviour and aborts the JVM on most
implementations. Before this change, of the 40 Java_* entry points only
ONE had a catch-all: jllama.cpp had 34 with a single catch (...) between
them, native_server.cpp's 5 had no handler of any kind, and
train_engine.cpp's finetuneNative guarded only its finetune() call, not
the JSON config parse (whose own handler had no catch (...)) nor the
GetStringUTFChars copy before it. load_model_impl -- the entire model
load path -- and handleDetokenize were fully unguarded.

Adds jni_guard_impl to jni_helpers.hpp (Layer A) and routes all 39
wrappable entry points through it. It is additive: an entry point that
already converts std::exception keeps doing so and never reaches the new
handlers. What it adds everywhere is the catch (...) arm, which is the
only backstop for an exception type not derived from std::exception. On
a catch it returns the zero/nullptr sentinel for the return type.

Two refusals the handler keeps, both pinned by tests: never ThrowNew
over an already-pending Java exception (the JNI spec forbids most calls
in that state, and the pending one is the more precise error), and never
ThrowNew with a null class.

Three entry points use a function-try-block instead, which also avoids
reindenting a goto-carrying body: JNI_OnLoad runs before c_llama_error
is cached and JNI_OnUnload after it is released, so neither has a class
to throw with -- OnLoad returns JNI_ERR (surfaced as
UnsatisfiedLinkError), OnUnload swallows. finetuneNative reports failure
as its return string rather than a Java exception, and train_engine.cpp
deliberately never includes jni_helpers.hpp, so its backstop preserves
that contract.

The jllama.cpp diff is large because every body gained one indent level.
It is provably mechanical: a token-level comparison against the previous
file removes ZERO tokens -- every added token run is the wrapper or a
function-try-block. native_server.cpp's 14 removed tokens are exactly
the inlined FindClass that moved into the new llama_exception_class
helper; its two unnamed JNIEnv* parameters had to be named to reach env.

Verified: clang-format 22.1.8 clean, full Release build with 0 errors
and 0 warnings, ctest 544/544 (537 + 7 new guard tests), nm -D still
shows 40 Java_* exports and 0 mangled, NativeLibraryLoadSmokeTest 4/4
with 0 skipped (JNI_OnLoad still loads after the shape change).

Also removes the PR #251 "JNI safety and server hardening" TODO section:
its other three items were already fixed (parse_string_array has null
guards, an ExceptionCheck and paired DeleteLocalRef; close() is
synchronized; OpenAiServerConfig has maxRequestBodyBytes), and this
change closes the fourth.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP
package-fatjars.sh emits four OS/arch server fat jars, and all four are
uploaded, GPG-signed and attached to every release by
github-release-signed / github-snapshot. Only the two x86-64 ones were
ever launched: grepping publish.yml for all-linux-aarch64 or
all-windows-aarch64 returned zero hits, so neither was downloaded or run
anywhere in the pipeline.

That is precisely what workspace/policies/fat-jar-release-assets.md
forbids -- "No release asset is attached that CI has not run" -- and
that rule exists because a corrupt macOS dylib shipped in three releases
under a fully green pipeline. These two jars were the remaining assets
in the same blind spot.

Adds smoke-fatjar-linux-aarch64 (ubuntu-24.04-arm) and
smoke-fatjar-windows-arm64 (windows-11-arm), mirroring the existing
x86-64 jobs -- the only differences are the runner and the jar glob.
Both GitHub ARM runners are already used by this workflow (the aarch64
CPU/Vulkan builds and build-windows-arm64), and that arm64 job already
resolves temurin at this java-version on windows-11-arm, so no new
infrastructure is involved. package-fatjars gains the two matching
single-jar upload artifacts so the smokes do not pull the multi-GB set,
and both jobs join the publish-snapshot and publish-release needs graphs
as fail-loud gates.

The Linux job also re-runs verify-bytecode-version.sh over its jar:
package-fatjars rewrites the zip per OS/arch, so the aarch64 asset is a
different artifact from the one smoke-fatjar-linux checks even though
the classes are identical.

Verified: the workflow parses (64 jobs, no dangling needs), both new
jobs gate both publish paths, the classifier set in llama/pom.xml really
yields the linux-aarch64 and windows-aarch64 targets
(vulkan-linux-aarch64 and opencl-windows-aarch64 respectively), so the
globs match real jar names, and smoke-test-fatjar.sh/.ps1 are
arch-agnostic (java -jar, /health, /v1/chat/completions).

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP
… map, dead code

Four independent items from the repo audit, plus the TODO drift they left behind.

1. patches/0010 now has an always-on, model-free guard
   (.github/verify-patches-applied.sh, run in the C++ Tests job). Most patches
   already red CI everywhere if they go missing -- 0003/0006/0007/0008 are link
   errors, 0011 and 0012 have their own C++ test files -- but 0010 casts one enum
   inside a `static` function unreachable from jllama_test, so reverting it leaves
   ctest fully green and only a model-gated Java test notices. The script also
   asserts, self-maintainingly, that every llama/patches/*.patch is named in the
   applier's stamp and that the fetched tree is actually dirty, so a stale build
   dir or a reverted patched file fails loud. Falsified both ways before wiring:
   reverting only the 0010 cast, and reverting the tree with the stamp intact,
   each exit 1; the intact tree exits 0.

2. The close()-vs-inference use-after-free defence is now tested. acquire_/
   release_jllama_context_impl and jllama_context_guard had zero references
   across all seven test files while their sibling get_jllama_context_impl had
   three; they were absent only because they are `inline` and never odr-used in
   jllama_test (g_ctx_mutex is extern here and defined in jllama.cpp, which this
   binary does not compile). A test-local definition unblocks them. 7 tests cover
   the reference count up and down, the guard's destructor on a normal and an
   early return, and the two null paths. Falsified by deleting the fetch_add from
   acquire: 2 of the new tests go red, and green again once restored.

3. OSInfo.archMapping's alias branch is asserted. getArchName()'s map lookup had
   no assertion anywhere, so a lost `amd64 -> x86_64` entry would silently send
   LlamaLoader to a resource directory that was never shipped. Only the 18
   NON-IDENTITY aliases are pinned -- an identity entry such as s390x -> s390x is
   behaviourally redundant with the \W-stripping fallback, so pinning it could not
   detect its loss -- plus the case-insensitivity of the lookup and the fallback
   itself. os.arch is saved and restored alongside the existing override property.

4. Java8CompatibilityHelper is deleted. Six of its seven public methods had zero
   production call sites; the only live one, toString(ByteArrayOutputStream,
   Charset), is inlined into its single caller in ProcessRunner. Its
   FORMAT_STRING_MANIPULATION suppression in spotbugs-exclude.xml is removed in
   the same commit -- a suppression naming a method that no longer exists is
   silently inert, which is the exact failure class CLAUDE.md warns about.

5. TODO.md's "Upstream PR submissions" section was stale in both directions: it
   said "six of the seven patches" when there are nine, still listed 0009 (merged
   upstream and dropped at b10280), omitted 0010/0011/0012, and described 0003 as
   dropping automatically when its upstream PR merges -- upstream closed that PR
   without merging, so 0003 is permanent.

Verified: clang-format 22.1.8 clean, Release build 0 errors 0 warnings, ctest
551/551 (544 + 7), mvn test 1758 Java tests 0 failures (OSInfoTest 19/19),
spotbugs:check 0 bugs, spotless clean.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP
…running

Four gaps, each of which let a real failure stay green.

LlamaTrainerIntegrationTest ran nowhere. Its model was in no models.csv row
and its property was set by no job, so the only Java -> JNI -> native trainer
round trip self-skipped on every platform. train_engine.cpp carries the same
postprocess_cpu_params pair as tts_params.hpp, so the JVM-abort class of bug
could regress there unseen. Adds stories260K.gguf (1.19 MB, F32) to the
manifest and wires -Dnet.ladenthin.llama.train.model on all six Java jobs.
The cache key is hashed from models.csv, so the new row produces a fresh
entry with no manual version bump.

The test itself needed pinning to be worth running. nCtx is fixed at 128
rather than inherited: common_opt_dataset_init computes ndata on a size_t, so
a corpus shorter than n_ctx + 1 wraps and anything under ~1.5x n_ctx trips
GGML_ASSERT(ndata > 0) -- a GGML_ABORT that kills the JVM instead of failing
the test. nGpuLayers is 0 because the backward ops this path needs are not
verified on Metal, which three jobs build with. And it now asserts the output
is neither a stub nor byte-identical to the input: llama_set_param silently
skips every non-F32 tensor, so a quantized fixture writes a plausible GGUF
having trained nothing, which the old exists/size assertions passed.

verify-test-counts.sh fails a job whose suite stopped running. A class-level
@BeforeAll assumption failure makes Surefire record tests="0" -- the class
contributes no entries at all, so a skip check is structurally blind to it.
That is exactly how every model-gated class silently aborted for months. The
check is the precise signature plus a slack total floor as backstop.
Falsified four ways.

apply-llama-patches.cmake gets a content oracle. Its dirty-tree stamp match
proved which patches were applied, never that they still are: reverting a
patched file by hand left the tree dirty and the stamp valid, so the
reconfigure was a no-op and the build silently lost the patch. The stamp now
records a fingerprint of the filtered porcelain status plus the full diff.
Verified both ways: reverting common/peg-parser.cpp aborts the configure, an
untouched reconfigure stays a 2.1 s no-op.

LlamaLoader.extractFile's reuse-vs-replace decision had no test. Going
through initialize() cannot reach it -- measured, not assumed: the cleanup
pass it runs first deletes exactly the jllama* temp paths a test must seed,
so a byte-identical seed is re-extracted with a fresh mtime. extractFile
becomes package-private, the convention this class already follows for its
other testable statics, and five tests pin extraction, the absent resource,
temp-file cleanup, reuse without rewriting (which is what keeps a
Windows-locked library loadable) and replacement of stale content. Both
branch tests falsified, each breaking only the test that owns it.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cft7guQngfyKycdJfBEfJP

Copy link
Copy Markdown
Owner Author

claude-review is red, and it is not this PR's

Failing check: Claude Code Review / claude-review (run 570).

Why it is not this PR's. The SDK run ended before it read anything:

{ "type": "result", "subtype": "success", "is_error": true,
  "duration_ms": 300, "num_turns": 1, "total_cost_usd": 0, "modelUsage": {} }

One turn, 300 ms, zero cost and an empty modelUsage — no model call ever completed, so the action never fetched the diff. Nothing about the change can influence that outcome.

It is also not new. The last successful run of this workflow was #556 on 2026-09-09. Runs 557–570 have all failed — 14 consecutive, across at least eight unrelated branches and PRs, including #436, which merged. Both pushes on this branch failed identically.

No fix exists to port. The failure is on the credentials/account side of anthropics/claude-code-action — an ANTHROPIC_API_KEY that is invalid, expired, or out of credit is the shape that produces an instant is_error with no usage. That is repository-secret state, not something a change to this branch can reach, so widening the PR to chase it would be wrong.

No re-run spent. The rule is to re-run once to confirm a failure is not this PR's. Fourteen consecutive failures across eight unrelated branches already establish that more strongly than one more run could, so the re-run is left unspent rather than used to re-prove it.

Nothing here is actionable from the diff. The rest of the pipeline is unaffected; the full Publish run for this head is waiting on the startgate environment approval.


Generated by Claude Code

@sonarqubecloud

Copy link
Copy Markdown

Copy link
Copy Markdown
Owner Author

The other three red checks — also not this PR's

Triaged the rest of the red checks on 4a699a8. All three are inherited, none is reachable from this diff. Grouped into one comment rather than three.

The checks that actually exercise the change are green: clang-format, test (×2), submit-maven, osv-scanner, scan-pr / osv-scan.


1–2. Verify GPG signing key (both the gpg and the Gradle/BouncyCastle path)

Both fail in ~2 seconds with the workflow's own guard message:

GPG_PRIVATE_KEY is empty for this run. Either the secret is not set, or it is scoped to a different environment/branch than 'maven-central' on this ref.

Both jobs declare environment: maven-central, and that environment's secrets are not delivered to a pull_request event from this ref. The job is designed to fail loudly in that case. Verified pre-existing on unrelated branches — both are red on the Publish runs for claude/llama-cpp-b10905 (943) and claude/fix-streaming-payload-tostring (934) too. Nothing to fix; it is a property of the event, not of the code.

3. analyze (CodeQL)

Real root cause, and worth knowing:

com.semmle.extractor.java.interceptors.KotlinInterceptor$KotlinVersionTooRecentError:
Kotlin version 2.4.20 is too recent. CodeQL currently supports versions below 2.4.20

CodeQL's Java extractor intercepts the Kotlin compiler; it refuses 2.4.20, so kotlin-maven-plugin:2.4.20:compile fails on llama-kotlin only under the CodeQL tracer (the reactor is otherwise fine — llama and llama-langchain4j both report SUCCESS in that same log, and the ordinary test jobs are green).

It is red on the base branch. The last 10 CodeQL runs on main have all failed, back to 2026-09-10. kotlin.version was set to 2.4.20 by 45107c0 ("build(deps): Kotlin 2.4.20 and dependency-submission action v6"), which is on main; git diff origin/main...HEAD -- llama-kotlin/ pom.xml on this branch is empty.

No fix exists to port, so per the standing rule this is a proposed patch rather than a push — the choice between the options is yours, not something to decide inside a llama.cpp version bump:

# .github/workflows/codeql.yml — in the java-kotlin matrix entry's env:
CODEQL_EXTRACTOR_JAVA_AGENT_DISABLE_KOTLIN: "true"

That is CodeQL's documented escape hatch for exactly this: the extractor skips Kotlin and still analyses all the Java, so the build succeeds and coverage of the core module is unaffected. The trade is that llama-kotlin's ~6 Kotlin files stop being scanned until CodeQL supports 2.4.x. The alternatives are pinning Kotlin below 2.4.20 (changes the shipped artifact's compiler to satisfy a scanner) or simply waiting for CodeQL support while main stays red. I have not pushed any of them.

No re-run spent on any of the three: two are deterministic secret-scoping failures and the third reproduces on main, so a re-run could not add information.


Generated by Claude Code

@bernardladenthin
bernardladenthin merged commit 848f23f into main Sep 15, 2026
11 of 78 checks passed
@bernardladenthin
bernardladenthin deleted the claude/serene-goodall-cq2rgc branch September 15, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants