adds simple llama grammar tests

[drbh]

ref ggml-org/ggml#295

This PR adds simple llama grammar test to give more insight into the how the grammar is used to constrain token generation

run tests

mkdir -p build && cd build && cmake ..
cmake --build . --config Release && ./bin/test-llama-grammar

Please let me know if I should make any changes! 🙏

This PR is similar to and a continuation on #2594

[drbh]

WIP to resolve the SUMMARY: AddressSanitizer: heap-buffer-overflow bug (seen in CI) will close PR if not resolved soon

[ggerganov]

Would be great to resolve the issue and merge this test. Hopefully people will help to debug

[drbh]

@ggerganov I believe I've identified the issue but am having some trouble resolving it.

It seems that attempting to access tok.code_points[1] can cause a heap-buffer-overflow here on master
https://github.com/ggerganov/llama.cpp/blob/b5ffb2849d23afe73647f68eec7b68187af09be6/llama.cpp#L2277-L2279

and it seems like this issue still exists on the fix unicode branch #2553 when attempting to deref tok.code_points here:
https://github.com/ejones/llama.cpp/blob/236194fc3d4241b8bfc9dc7656f497076612e99a/llama.cpp#L2357-L2359

I've been attempting to avoid this memory issue without changing any function signatures but have not found a solution. Any advice would be greatly appreciated! 🙏

[ejones]

Taking a look

[ejones]

Looks like the issue is that llama_grammar_candidate.code_points needs to be 0 terminated. With this patch the test seems to pass:

diff --git a/tests/test-llama-grammar.cpp b/tests/test-llama-grammar.cpp
index e625ca1..d0b7c48 100644
--- a/tests/test-llama-grammar.cpp
+++ b/tests/test-llama-grammar.cpp
@@ -196,8 +196,9 @@ int main()
 
     for (size_t i = 0; i < 24; ++i)
     {
-        uint32_t *cp = new uint32_t[1]; // dynamically allocate memory for code_point
-        *cp = 37 + i;
+        uint32_t *cp = new uint32_t[2]; // dynamically allocate memory for code_point
+        cp[0] = 37 + i;
+        cp[1] = 0;
         next_candidates[i] = {i, cp};
     }

On that note, I see the static functions like llama_grammar_reject_candidates_for_stack as implementation details (which might change in the future). I'm not sure what the overall testing philosophy is for this project, but I might consider focusing on the public interface. That might look something like this:

• parse and init the grammar
• verify initial output of llama_sample_grammar, in terms of which tokens it rejects (assigns a logit of -INFINITY)
• one or more times:
  • call llama_grammar_accept_token
  • verify updated llama_sample_grammar output

[drbh]

@ejones amazing thank you for the solution! (silly oversight from my side) 🙏

I also agree that it makes sense to target the public interfaces going forward. I intend on adding tests for llama_sample_grammar and deprecate the llama_grammar_reject_candidates_for_stack in a future PR. Along with the other suggestions above.

A large motivation behind these tests is to understand how the grammar works and what each function is doing so I've been working through the flow a function at a time hence the very targeted tests.