sampling: fix out-of-bounds logits read when the vocab has no newline token (#2188)

llama_token_nl() can return LLAMA_TOKEN_NULL (-1). Falcon3's BPE tokenizer maps
"\n" to zero tokens, so the loader takes its fallback (linefeed_id =
special_pad_id), and the two variants tested reach null by different routes. On
Falcon3-7B-Instruct that copy runs before LLM_KV_TOKENIZER_PAD_ID is read from
the GGUF, so it copies the BPE default, which is itself LLAMA_TOKEN_NULL, even
though the model has a pad token. Falcon3-7B-Base carries no pad id at all and
lands on null whatever the ordering, so a load-order fix alone would not close
this.

llama_sampling_prepare_impl then evaluated logits[-1], an out-of-bounds read one
float before the current position's logit row. Whether that address is mapped
depends on allocation layout, so the crash is configuration-dependent rather
than universal.

This is a crash risk only and cannot change output: the value read is written
back only to a candidate whose id equals nl_token, and no real candidate id is
-1, so it never reaches the sampler.

The fix caches the token once, skips the read when it is null, and skips the
penalize-newline restore, since there is nothing to restore. For a vocab with a
real newline token the block is unchanged.

Repro on Falcon3-7B-Instruct-Q4_K_M, four P100s with the layers split across all
four, -ngl 99 -fa 1 -c 8192, one chat request per trial with a fresh server each
trial: main segfaults 4/4, this change returns HTTP 200 4/4. Across eight models
and both --penalize-nl polarities, 26 greedy comparisons of generated text show
no difference between main and this change on any vocab that has a real newline
token.

Mainline carried this block verbatim until ggml-org/llama.cpp#9294 moved the
penalty stage into the sampler chain, which dropped the raw read and handles the
null id at sampler init instead. ggml-org/llama.cpp#10803 later removed
penalize_nl entirely, so there is no upstream counterpart to port this to.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
mb8565
2026-07-26 10:59:58 +03:00
committed by GitHub
co-authored by Claude Opus 5
parent de55d9e2f6
commit 8be938842b
+10 -3
View File
@@ -655,15 +655,22 @@ static llama_token_data_array llama_sampling_prepare_impl(
const auto& penalty_tokens = params.use_penalty_prompt_tokens ? params.penalty_prompt_tokens : prev;
const int penalty_tokens_used_size = std::min((int)penalty_tokens.size(), penalty_last_n);
if (penalty_tokens_used_size) {
const float nl_logit = logits[llama_token_nl(llama_get_model(ctx_main))];
// llama_token_nl() can legitimately return LLAMA_TOKEN_NULL (-1). Some vocabs (e.g. Falcon3's
// BPE tokenizer) tokenize "\n" to zero tokens, and the loader's fallback
// (llama-vocab.cpp, linefeed_id = special_pad_id) runs before LLM_KV_TOKENIZER_PAD_ID is read
// from the GGUF, so it copies the BPE default, which is itself LLAMA_TOKEN_NULL. Reading
// logits[-1] is then an out-of-bounds access one float before the current position's logit
// row, which segfaults when that address happens to be unmapped.
const llama_token nl_token = llama_token_nl(llama_get_model(ctx_main));
const float nl_logit = nl_token != LLAMA_TOKEN_NULL ? logits[nl_token] : 0.0f;
llama_sample_repetition_penalties(ctx_main, &cur_p,
penalty_tokens.data() + penalty_tokens.size() - penalty_tokens_used_size,
penalty_tokens_used_size, penalty_repeat, penalty_freq, penalty_present);
if (!penalize_nl) {
if (!penalize_nl && nl_token != LLAMA_TOKEN_NULL) {
for (size_t idx = 0; idx < cur_p.size; idx++) {
if (cur_p.data[idx].id == llama_token_nl(llama_get_model(ctx_main))) {
if (cur_p.data[idx].id == nl_token) {
cur_p.data[idx].logit = nl_logit;
break;
}