Skip to content

UPSTREAM PR #21082: common: add bounds check in common_init_result::sampler to prevent segfault on failed model load#1313

Open
loci-dev wants to merge 3 commits intomainfrom
loci/pr-21082-index-check-init-result-sampler
Open

UPSTREAM PR #21082: common: add bounds check in common_init_result::sampler to prevent segfault on failed model load#1313
loci-dev wants to merge 3 commits intomainfrom
loci/pr-21082-index-check-init-result-sampler

Conversation

@loci-dev
Copy link
Copy Markdown

Note

Source pull request: ggml-org/llama.cpp#21082

Overview

This fix supercedes #21049 to fix the segfault when non-existent models are loaded. It reverts the previous fix because it is no longer necessary with this more correct fix in place. In addition a regression test was added to expose the issue and showcase the fix.

Before:

./build/bin/test-init-fail
build: 8557 (43d7135f7) with GNU 13.3.0 for Linux x86_64
common_init_result: fitting params to device memory, for bugs during this step try to reproduce them with -fit off, or provide --verbose logs if the bug only occurs with -fit on
gguf_init_from_file: failed to open GGUF file '/non_existent_model.gguf' (No such file or directory)
llama_model_load: error loading model: llama_model_loader: failed to load model from /non_existent_model.gguf
llama_model_load_from_file_impl: failed to load model
llama_params_fit: encountered an error while trying to fit params to free device memory: failed to load model
llama_params_fit: fitting params to free memory took 0.00 seconds
gguf_init_from_file: failed to open GGUF file '/non_existent_model.gguf' (No such file or directory)
llama_model_load: error loading model: llama_model_loader: failed to load model from /non_existent_model.gguf
llama_model_load_from_file_impl: failed to load model
common_init_from_params: failed to load model '/non_existent_model.gguf'
Segmentation fault (core dumped)

After:

./build/bin/test-init-fail
build: 8557 (43d7135f7) with GNU 13.3.0 for Linux x86_64
common_init_result: fitting params to device memory, for bugs during this step try to reproduce them with -fit off, or provide --verbose logs if the bug only occurs with -fit on
gguf_init_from_file: failed to open GGUF file '/non_existent_model.gguf' (No such file or directory)
llama_model_load: error loading model: llama_model_loader: failed to load model from /non_existent_model.gguf
llama_model_load_from_file_impl: failed to load model
llama_params_fit: encountered an error while trying to fit params to free device memory: failed to load model
llama_params_fit: fitting params to free memory took 0.00 seconds
gguf_init_from_file: failed to open GGUF file '/non_existent_model.gguf' (No such file or directory)
llama_model_load: error loading model: llama_model_loader: failed to load model from /non_existent_model.gguf
llama_model_load_from_file_impl: failed to load model
main: OK
common_init_from_params: failed to load model '/non_existent_model.gguf'

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES AI was used to write the unit test

@loci-review
Copy link
Copy Markdown

loci-review bot commented Mar 29, 2026

Flame Graph: build.bin.llama-bench::_ZN18common_init_result7samplerEi

Target version:

Flame Graph: build.bin.llama-bench::_ZN18common_init_result7samplerEi

The target version shows additional execution paths for bounds validation (size() call and conditional branching) before the original pointer accessor chain, explaining the 78% response time increase.

Additional Findings

No performance-critical hot paths affected. Core inference operations (matrix operations, attention mechanisms, KV cache, quantization kernels) show 0% change. All overhead is one-time during initialization. The bounds checking prevents crashes at negligible cost (~171ns per initialization call).

🔎 Full breakdown: Loci Inspector
💬 Questions? Tag @loci-dev

@loci-dev loci-dev force-pushed the main branch 11 times, most recently from fd3ce9d to 1770118 Compare April 6, 2026 02:18
@loci-dev loci-dev force-pushed the main branch 8 times, most recently from 385b1fc to 06d9e10 Compare April 13, 2026 02:18
@loci-dev loci-dev force-pushed the main branch 6 times, most recently from d101579 to 63ab8d1 Compare April 18, 2026 02:17
@loci-dev loci-dev force-pushed the main branch 2 times, most recently from 7638ab4 to f1b46d5 Compare April 20, 2026 02:19
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