MDEV-37167 Nested BEGINs (4600+) cause a segmentation fault#5402
MDEV-37167 Nested BEGINs (4600+) cause a segmentation fault#5402KhaledR57 wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses MDEV-37167, where deeply nested BEGIN blocks cause a segmentation fault due to stack overflow. To resolve this, the recursive destruction of parsing contexts (sp_pcontext) is replaced with an iterative cleanup managed by the root context. Additionally, a stack overrun check is introduced in retrieve_field_definitions to safely handle deep recursion during field definition retrieval. Test cases have been added to verify the stability of deeply nested blocks. There are no review comments to evaluate, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Each nested BEGIN adds one sp_pcontext. Two walks over the finished tree recursed once per nesting level and overran the thread stack. ~sp_pcontext() freed its children recursively. The root context now remembers every context of the tree and frees them in a flat loop, so teardown depth is constant. The list head is kept on the root rather than on sp_head so that push_context() keeps its signature. retrieve_field_definitions() descends the children to build the run-time frame. It emits them in run-time offset order, so it stays recursive, but it now checks the stack and returns an error instead of crashing.
e3cccf5 to
cd16b82
Compare
There was a problem hiding this comment.
Pull request overview
Fixes MDEV-37167 by preventing thread stack exhaustion when handling extremely deep nested BEGIN ... END blocks in stored routines. It replaces recursive parse-context teardown with an iterative destruction strategy and adds a stack-overrun guard to the remaining recursive walk that constructs runtime variable definitions, turning what was previously a crash into a handled error (ER_STACK_OVERRUN_NEED_MORE).
Changes:
- Make
sp_pcontexttree teardown constant-depth by having only the root iteratively delete all contexts recorded in a flat linked list. - Add
THD*-aware stack-overrun checks tosp_pcontext::retrieve_field_definitions()and propagate errors to procedure/function/trigger runtime-context creation. - Add an MTR regression test covering deep nesting (parse error, successful create/drop, and safe execution behavior).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
sql/sp_pcontext.h |
Extends sp_pcontext with root/flat-list pointers and updates retrieve_field_definitions() API to return an error status and accept THD*. |
sql/sp_pcontext.cc |
Implements iterative destruction via root-owned flat list; adds check_stack_overrun() guard and error propagation in retrieve_field_definitions(). |
sql/sp_head.cc |
Updates all callers to handle retrieve_field_definitions() returning an error by aborting runtime context creation safely. |
mysql-test/main/mdev_37167.test |
Adds regression coverage for deep nesting, ensuring no crash and accepting either success or ER_STACK_OVERRUN_NEED_MORE on execution. |
mysql-test/main/mdev_37167.result |
Expected output for the new regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Each nested
BEGINadds onesp_pcontextto the routine's parse context tree. Building the tree costs no C stack, the parser is iterative, but two walks over the finished tree recursed once per nesting level and overran the thread stack.~sp_pcontext()freed its children recursively. The root context now remembers every context of the tree in a list and frees them in a flat loop, so teardown depth is constant.retrieve_field_definitions()descends the children fromsp_head::rcontext_create()to build the run-time frame. It emits variables and child contexts in run-time offset order, so it stays recursive, but it now checks the stack and returns an error. All three callers, for procedures, functions and triggers, already had an error path. A routine too deeply nested to fit the thread stack is now rejected withER_STACK_OVERRUN_NEED_MOREinstead of crashing.How can this PR be tested?
Basing the PR against the correct MariaDB version
mainbranch.