Skip to content

Make check_stack_depth() effective for embedders; guard recursive output paths #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pyramation wants to merge 2 commits into
base: 18-constructive
Choose a base branch
from
Open

Make check_stack_depth() effective for embedders; guard recursive output paths #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64e7eda
Make check_stack_depth() effective for embedders and guard the recurs…
pyramation Oct 24, 2026
b66cc16
Free the PL/pgSQL alias list after each parse
pyramation Oct 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/pg_query_fingerprint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "pg_query_fingerprint.h"

#include "postgres.h"
#include "miscadmin.h"
#include "xxhash/xxhash.h"
#include "lib/ilist.h"

Expand Down Expand Up @@ -263,6 +264,7 @@ _fingerprintFreeContext(FingerprintContext *ctx) {
void
_fingerprintNode(FingerprintContext *ctx, const void *obj, const void *parent, char *field_name, unsigned int depth)
{
check_stack_depth();
// Some queries are overly complex in their parsetree - lets consistently cut them off at 100 nodes deep
if (depth >= 100) {
return;
Expand Down
7 changes: 7 additions & 0 deletions src/pg_query_json_plpgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include "pg_query.h"
#include "postgres.h"
#include "miscadmin.h"
#include "pg_query_json_plpgsql.h"

#include "pg_query_json_helper.c"
Expand Down Expand Up @@ -138,6 +140,7 @@ static void dump_type(StringInfo out, PLpgSQL_type *node);
static void
dump_stmt(StringInfo out, PLpgSQL_stmt *node)
{
check_stack_depth();
appendStringInfoChar(out, '{');
switch (node->cmd_type)
{
Expand Down Expand Up @@ -627,6 +630,7 @@ dump_getdiag_item(StringInfo out, PLpgSQL_diag_item *node)
static void
dump_expr(StringInfo out, PLpgSQL_expr *node)
{
check_stack_depth();
WRITE_NODE_TYPE("PLpgSQL_expr");

WRITE_STRING_FIELD(query, query, query);
Expand Down Expand Up @@ -812,7 +816,10 @@ pg_query_plpgsql_reset_aliases(void)
int i;
for (i = 0; i < plpgsql_naliases; i++)
free(plpgsql_aliases[i].name);
free(plpgsql_aliases);
plpgsql_aliases = NULL;
plpgsql_naliases = 0;
plpgsql_aliases_size = 0;
}

void
Expand Down
2 changes: 2 additions & 0 deletions src/pg_query_outfuncs_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "pg_query_outfuncs.h"

#include "postgres.h"
#include "miscadmin.h"

#include <ctype.h>

Expand Down Expand Up @@ -289,6 +290,7 @@ _outAConst(StringInfo out, const A_Const *node)
static void
_outNode(StringInfo out, const void *obj)
{
check_stack_depth();
if (obj == NULL)
{
appendStringInfoString(out, "null");
Expand Down
2 changes: 2 additions & 0 deletions src/pg_query_outfuncs_protobuf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "pg_query_outfuncs.h"

#include "postgres.h"
#include "miscadmin.h"
#include <ctype.h>
#include "access/relation.h"
#include "nodes/parsenodes.h"
Expand Down Expand Up @@ -246,6 +247,7 @@ _outAConst(PgQuery__AConst* out, const A_Const *node)
static void
_outNode(PgQuery__Node* out, const void *obj)
{
check_stack_depth();
if (obj == NULL)
return; // Keep out as NULL

Expand Down
36 changes: 33 additions & 3 deletions src/pg_query_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,39 @@ PgQueryParseResult pg_query_parse_opts(const char* input, int parser_options)
result.stderr_buffer = parsetree_and_error.stderr_buffer;
result.error = parsetree_and_error.error;

tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
if (result.error == NULL)
{
PG_TRY();
{
tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
}
PG_CATCH();
{
ErrorData* error_data;
PgQueryError* error;

MemoryContextSwitchTo(ctx);
error_data = CopyErrorData();
error = malloc(sizeof(PgQueryError));
error->message = strdup(error_data->message);
error->filename = strdup(error_data->filename);
error->funcname = strdup(error_data->funcname);
error->context = NULL;
error->lineno = error_data->lineno;
error->cursorpos = error_data->cursorpos;
result.error = error;
FlushErrorState();
}
PG_END_TRY();
}
else
{
tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
}
Comment on lines +165 to +170

@tenki-reviewer tenki-reviewer Bot Oct 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

Serialize-on-parse-error path left outside PG_TRY

In pg_query_parse_opts the new PG_TRY only covers the result.error == NULL branch; the else branch still calls pg_query_nodes_to_json on the partial tree with no handler (src/pg_query_parse.c:167-169). check_stack_depth() in _outNode (src/pg_query_outfuncs_json.c:293) can throw for a deeply nested partial statement on exactly this error path, and the uncaught throw skips pg_query_exit_memory_context and terminates the process. Restructuring the code so the error branch runs outside the try block creates the gap; the throw can be caught as easily as the success branch.

📋 Prompt for AI Agents

In src/pg_query_parse.c, pg_query_parse_opts lines 138-170: remove the if (result.error == NULL) ... else ... split and run the pg_query_nodes_to_json/strdup/pfree sequence unconditionally inside the existing PG_TRY block, keeping the PG_CATCH that builds a PgQueryError. When a parse error already exists, the partial tree is still serialized, but a check_stack_depth()/ereport throw during serialization is caught and reported instead of longjmping out of the library unguarded.


pg_query_exit_memory_context(ctx);

Expand Down
2 changes: 2 additions & 0 deletions src/pg_query_parse_plpgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,8 @@ PgQueryPlpgsqlParseResult pg_query_parse_plpgsql(const char* input)
result.plpgsql_funcs[strlen(result.plpgsql_funcs) - 1] = ']';

free(parse_result.stderr_buffer);
/* Drop the aliases recorded during this compile so nothing outlives the call */
pg_query_plpgsql_reset_aliases();
pg_query_exit_memory_context(ctx);

return result;
Expand Down
11 changes: 11 additions & 0 deletions src/postgres/src_backend_utils_misc_stack_depth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,14 @@ stack_is_too_deep(void)
#if defined(HAVE_GETRLIMIT)
#else
#endif

/* libpg-query-node: set the stack-depth reference point and limit from the embedder. */
void
pg_query_set_stack_limit(size_t bytes)
{
char here;

stack_base_ptr = &here;
max_stack_depth_bytes = (ssize_t) bytes;
max_stack_depth = (int) (bytes / 1024);
}
Comment on lines +151 to +159

@tenki-reviewer tenki-reviewer Bot Oct 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

New check_stack_depth guards are inert; setter undeclared

The PR adds check_stack_depth() to five recursive walkers (_fingerprintNode, _outNode in JSON and protobuf, dump_stmt/dump_expr in plpgsql JSON, deparseExpr), but stack_is_too_deep() returns false whenever stack_base_ptr == NULL (src/postgres/src_backend_utils_misc_stack_depth.c:123-127) and nothing in the library ever sets it. The only setter, pg_query_set_stack_limit (src/postgres/src_backend_utils_misc_stack_depth.c:151-159), is declared in no header and called nowhere, so embedders cannot invoke it portably; and because it writes __thread state, a limit configured on one thread leaves every other worker thread unprotected. The advertised stack-overflow protection is dead code until an undeclared, undocumented, per-thread function is called.

📋 Prompt for AI Agents

In the libpg_query repo: (1) add extern void pg_query_set_stack_limit(size_t bytes); to the public header pg_query.h alongside the other public API declarations; (2) make the stack-depth check active by default — e.g. in pg_query_enter_memory_context (src/pg_query.c), set stack_base_ptr via __builtin_frame_address(0) when it is still NULL so stack_is_too_deep() measures depth on every thread that enters a pg_query entry point, keeping pg_query_set_stack_limit as an override. Without this, the check_stack_depth() calls added in src/pg_query_fingerprint.c, src/pg_query_json_plpgsql.c, src/pg_query_outfuncs_json.c, src/pg_query_outfuncs_protobuf.c, and src/postgres_deparse.c never fire, and the new setter is unusable without a hand-written prototype.

2 changes: 2 additions & 0 deletions src/postgres_deparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "postgres_deparse.h"

#include "postgres.h"
#include "miscadmin.h"
#include "catalog/index.h"
#include "catalog/pg_am.h"
#include "catalog/pg_attribute.h"
Expand Down Expand Up @@ -784,6 +785,7 @@ static void deparseCExpr(DeparseState *state, Node *node);
// "a_expr" in gram.y
static void deparseExpr(DeparseState *state, Node *node, DeparseNodeContext context)
{
check_stack_depth();
if (node == NULL)
return;
switch (nodeTag(node))
Expand Down
Loading