Zero-initialize parent_cpstate in analyze_cypher#2423
Merged
jrgemignani merged 1 commit intoapache:masterfrom May 4, 2026
Merged
Zero-initialize parent_cpstate in analyze_cypher#2423jrgemignani merged 1 commit intoapache:masterfrom
jrgemignani merged 1 commit intoapache:masterfrom
Conversation
cypher_parsestate parent_cpstate is declared on the stack in analyze_cypher() and only pstate is explicitly set before it is passed to make_cypher_parsestate(). The latter reads parent_cpstate->subquery_where_flag (and other fields) in cypher_parse_node.c, which leaves them with indeterminate values. UBSan flagged the garbage bool (value 8) and aborted the backend. Use MemSet to zero the struct before populating pstate so all remaining members start with a defined value.
hari90
force-pushed
the
claude/fix-cypher-parser-memory-J8LsQ
branch
from
c6a9b79 to
e30e73c
Compare
jrgemignani
approved these changes
May 4, 2026
There was a problem hiding this comment.
Pull request overview
Fixes an uninitialized-stack-memory bug in the Cypher analysis path by ensuring the temporary cypher_parsestate wrapper passed into make_cypher_parsestate() starts from a fully defined state, preventing UBSan failures and undefined behavior.
Changes:
- Zero-initialize
parent_cpstatewithMemSet()inanalyze_cypher()before setting its explicitly required fields. - Remove now-redundant explicit
NULLassignments for members covered by the zero-initialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Thank you |
hari90
added a commit
to yugabyte/yugabyte-db
that referenced
this pull request
May 4, 2026
## Summary Fixes #31404. Zero-initialize the stack-allocated `parent_cpstate` in `analyze_cypher()` before passing it to `make_cypher_parsestate()`. The struct previously had only `pstate`, `graph_name`, and `params` set, leaving `subquery_where_flag` (and other fields) as uninitialized stack memory. `make_cypher_parsestate()` reads `parent_cpstate->subquery_where_flag` directly (`cypher_parse_node.c:62`), so under UndefinedBehaviorSanitizer the load aborts the backend whenever the stack byte is something other than 0 or 1. The first `cypher()` call in the asan/ubsan build was reliably crashing with: ``` runtime error: load of value 8, which is not a valid value for type 'bool' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior cypher_parse_node.c:62:56 ``` This is a latent upstream Apache AGE bug; the file is a verbatim subtree import. Worth reporting upstream as well. The redundant `graph_name = NULL`/`params = NULL` assignments are removed since `memset` already zeros them. ## Test plan - `./yb_build.sh asan --java-test 'org.yb.pgsql.TestPgRegressThirdPartyExtensionsMage#schedule'` was failing on `yb.orig.basic` before this change and passes with it. Upstream fix apache/age#2423 --- [CSI](<https://csiweb.dev.yugabyte.com/pull/31405/>)
hari90
added a commit
to yugabyte/yugabyte-db
that referenced
this pull request
May 5, 2026
…#31405) (#31423) ## Summary Fixes #31404. Zero-initialize the stack-allocated `parent_cpstate` in `analyze_cypher()` before passing it to `make_cypher_parsestate()`. The struct previously had only `pstate`, `graph_name`, and `params` set, leaving `subquery_where_flag` (and other fields) as uninitialized stack memory. `make_cypher_parsestate()` reads `parent_cpstate->subquery_where_flag` directly (`cypher_parse_node.c:62`), so under UndefinedBehaviorSanitizer the load aborts the backend whenever the stack byte is something other than 0 or 1. The first `cypher()` call in the asan/ubsan build was reliably crashing with: ``` runtime error: load of value 8, which is not a valid value for type 'bool' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior cypher_parse_node.c:62:56 ``` This is a latent upstream Apache AGE bug; the file is a verbatim subtree import. Worth reporting upstream as well. The redundant `graph_name = NULL`/`params = NULL` assignments are removed since `memset` already zeros them. Original commit: 154e1a6 / #31405 ## Test plan - `./yb_build.sh asan --java-test 'org.yb.pgsql.TestPgRegressThirdPartyExtensionsMage#schedule'` was failing on `yb.orig.basic` before this change and passes with it. Upstream fix apache/age#2423 --- [CSI](<https://csiweb.dev.yugabyte.com/pull/31423/>)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cypher_parsestate parent_cpstate is declared on the stack in analyze_cypher() and only pstate, graph_name, and params are explicitly set before it is passed to make_cypher_parsestate(). The latter reads parent_cpstate->subquery_where_flag (and other fields) in cypher_parse_node.c, which leaves them with indeterminate values. Yugabyte UBSan flagged the garbage bool (value 8) and aborted the backend.
Use MemSet to zero the struct before populating the explicit fields so all remaining members start with a defined value.