ggerganov / llama.cpp

LLM inference in C/C++
MIT License
64.51k stars 9.24k forks source link

Segfault in grammar #4066

Closed jmikedupont2 closed 5 months ago

jmikedupont2 commented 9 months ago

Prerequisites

Expected Behavior

see ebnf.ebnf in pr https://github.com/ggerganov/llama.cpp/compare/master...meta-introspector:llama.cpp:bugreport/ebnf#diff-8fbce6bdd71d2aae9d1f7fc3012fd793035a6d35b970544d46aa161844ef97e2

Current Behavior


0x00005555555e0993 in llama_grammar_init (rules=0x555555991d30, n_rules=29, start_rule_index=26) a\t /home/mdupont/experiments/llama.cpp/llama.cpp:6385
6385            for (pos = rules[i]; pos->type != LLAMA_GRETYPE_END; pos++) {
(gdb)```
jmikedupont2 commented 9 months ago

wow so rule 9 is jusT null

(const llama_grammar_element *) 0x555555a11c60
(gdb) p rules[8]                                                                                   $9 = (const llama_grammar_element *) 0x555555a11da0
(gdb) p rules[9]
$10 = (const llama_grammar_element *) 0x0                                                          (gdb) p rules[10]
$11 = (const llama_grammar_element *) 0x555555a0fcb0

i am going to add a check

jmikedupont2 commented 9 months ago

Now looking at it, The grammar rules have a hole in them, I think it is from the growth function $1 = std::vector of length 29, capacity 29 = {0x555555a11c20, 0x555555d646c0, 0x555555a11f70, 0x555555a11130, 0x555555a0fcc0, 0x555555a14210, 0x555555a0fca0, 0x555555a10ef0, 0x555555a0e9c0, 0x0, 0x555555a0fde0, 0x555555a0fc80, 0x555555a120d0, 0x555555a0e340, 0x555555d87020, 0x555555d86d30, 0x555555a0fc40, 0x555555a0fdb0, 0x555555a0fce0, 0x555555a0fd20, 0x555555a0fc60, 0x555555d86f70, 0x555555a12b80, 0x555555d86fa0, 0x555555a14480, 0x555555d871a0, 0x555555d870e0, 0x555555d87290, 0x555555d872e0} (gdb) bt

0 0x00005555555e0993 in llama_grammar_init (rules=0x555555991d30, n_rules=29,

start_rule_index=26) at /home/mdupont/experiments/llama.cpp/llama.cpp:6385

1 0x00005555555b4cb1 in llama_sampling_init (params=...)

at /home/mdupont/experiments/llama.cpp/common/sampling.cpp:21

2 0x000055555556685a in main (argc=7, argv=0x7fffffffe1b8)

at /home/mdupont/experiments/llama.cpp/examples/main/main.cpp:460
jmikedupont2 commented 9 months ago

ran the debugger, rule 9 was never added.


adding rule id 7
adding rule id 6
resize id 10
adding rule id 10
adding rule id 8
resize id 12
adding rule id 12
adding rule id 11
resize id 13
adding rule id 13
resize id 15```
jmikedupont2 commented 9 months ago

so it is a whole created by resize that is used and crashes, i am going to try and find what causes the id 9 to be dropped

jmikedupont2 commented 9 months ago

I added my debug code to the branch

jmikedupont2 commented 9 months ago

Looking at the new debug dump:

added id:9 wit string:|character|
added id:9 wit string:|-|
added id:11 wit string:|character|
added id:11 wit string:|-|
resize id 10
adding rule id 10
adding rule id 8
added id:11 wit string:|terminator|
resize id 12
adding rule id 12
adding rule id 11                                                                                  added id:13 wit string:|term|
jmikedupont2 commented 9 months ago

I was able to resolve the bug, it seems to be the - in the rule. https://github.com/ggerganov/llama.cpp/commit/a05aa8a83eba20f1fbace0927eae6e478b462a5a

so this caused the crash ```terminal ::= "'" character - "'" ( character - "'" ) "'"

| '"' character - '"' ( character - '"' ) '"'```

tadad commented 9 months ago

ran into the same issue. Does https://github.com/ggerganov/llama.cpp/commit/a05aa8a83eba20f1fbace0927eae6e478b462a5a fix the segfault?

jmikedupont2 commented 9 months ago

So, this code does not fix the segfault but lets you diagnose the issue, I have not fixed the root cause of the crash, which was for me the "-" char that i had to remove, but it showed the holes. the issue is that some rules are not being parsed and they create holes in the grammar that crash. I was not able to get it to work around them either. my plan is to use antlr4 grammar parser in C++ later for llama.cpp if I can then we can use antlr to check the grammars and generate the checkers, but this will need more work.

On Thu, Nov 16, 2023, 09:45 tadad @.***> wrote:

ran into the same issue. Does a05aa8a https://github.com/ggerganov/llama.cpp/commit/a05aa8a83eba20f1fbace0927eae6e478b462a5a fix the segfault?

— Reply to this email directly, view it on GitHub https://github.com/ggerganov/llama.cpp/issues/4066#issuecomment-1814589622, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD5KQ2LHSODASWNVB2T7W63YEYRKBAVCNFSM6AAAAAA7JY2BA2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUGU4DSNRSGI . You are receiving this because you authored the thread.Message ID: @.***>

spencekim commented 9 months ago

seems related to https://github.com/ggerganov/llama.cpp/issues/3878

github-actions[bot] commented 5 months ago

This issue was closed because it has been inactive for 14 days since being marked as stale.