Fix issue 2093: pfree() called with a NULL pointer (apache#2095)

Fixed issue 2093 where pfree() was called with a NULL pointer. The issue is due to some confusion with pfree(). There are 2 definitions for it, one that checks for a passed NULL and the other which does not. Created a function, pfree_if_not_null(), to check for NULL and call pfree() if a NULL wasn't passed. Modified the pfree references in the following files - src/backend/commands/label_commands.c src/backend/executor/cypher_merge.c src/backend/executor/cypher_set.c src/backend/parser/ag_scanner.l src/backend/parser/cypher_analyze.c src/backend/parser/cypher_expr.c src/backend/parser/cypher_gram.y src/backend/parser/cypher_parse_agg.c src/backend/utils/adt/age_global_graph.c src/backend/utils/adt/age_graphid_ds.c src/backend/utils/adt/age_session_info.c src/backend/utils/adt/age_vle.c src/backend/utils/adt/agtype.c src/backend/utils/adt/agtype_gin.c src/backend/utils/adt/agtype_raw.c src/backend/utils/adt/agtype_util.c src/backend/utils/load/ag_load_edges.c src/backend/utils/load/ag_load_labels.c src/include/utils/age_graphid_ds.h src/include/utils/age_session_info.h src/include/utils/agtype.h Added regression tests for the original issue. Resolved Conflicts: src/backend/commands/label_commands.c src/backend/parser/cypher_expr.c src/backend/parser/cypher_parse_agg.c src/backend/utils/adt/age_global_graph.c src/backend/utils/adt/agtype.c Resolved Conflicts: src/backend/utils/load/ag_load_edges.c src/backend/utils/load/ag_load_labels.c
jrgemignani · Sep 11, 2024 · 63550f0 · 63550f0
1 parent 3e78355
commit 63550f0
Show file tree

Hide file tree

Showing 22 changed files with 134 additions and 95 deletions.
diff --git a/regress/expected/expr.out b/regress/expected/expr.out
@@ -8505,6 +8505,21 @@ SELECT agtype_to_int8(bool('neither'));
 ERROR:  invalid input syntax for type boolean: "neither"
 LINE 1: SELECT agtype_to_int8(bool('neither'));
                                    ^
+--
+-- Issue 2093: Server crashes when executing SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+--
+SELECT agtype_access_operator(agtype_in('[null, null]'));
+ agtype_access_operator 
+------------------------
+
+(1 row)
+
+SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+ agtype_hash_cmp 
+-----------------
+      -505290721
+(1 row)
+
 --
 -- Cleanup
 --

diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql
@@ -3453,6 +3453,12 @@ SELECT agtype_to_int2(bool('neither'));
 SELECT agtype_to_int4(bool('neither'));
 SELECT agtype_to_int8(bool('neither'));
 
+--
+-- Issue 2093: Server crashes when executing SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+--
+SELECT agtype_access_operator(agtype_in('[null, null]'));
+SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+
 --
 -- Cleanup
 --

diff --git a/src/backend/commands/label_commands.c b/src/backend/commands/label_commands.c
@@ -94,8 +94,6 @@ static void range_var_callback_for_remove_relation(const RangeVar *rel,
                                                    Oid odl_rel_oid,
                                                    void *arg);
 
-
-
 PG_FUNCTION_INFO_V1(create_vlabel);
 
 /*

diff --git a/src/backend/executor/cypher_merge.c b/src/backend/executor/cypher_merge.c
@@ -347,7 +347,7 @@ static void free_path_entry_array(path_entry **path_array, int length)
 
     for (index = 0; index < length; index++)
     {
-        pfree(path_array[index]);
+        pfree_if_not_null(path_array[index]);
     }
 }
 
@@ -901,10 +901,10 @@ static void end_cypher_merge(CustomScanState *node)
         free_path_entry_array(entry, path_length);
 
         /* free up the array container */
-        pfree(entry);
+        pfree_if_not_null(entry);
 
         /* free up the created_path container */
-        pfree(css->created_paths_list);
+        pfree_if_not_null(css->created_paths_list);
 
         css->created_paths_list = next;
     }

diff --git a/src/backend/executor/cypher_set.c b/src/backend/executor/cypher_set.c
@@ -555,7 +555,7 @@ static void process_update_list(CustomScanState *node)
         lidx++;
     }
     /* free our lookup array */
-    pfree(luindex);
+    pfree_if_not_null(luindex);
 }
 
 static TupleTableSlot *exec_cypher_set(CustomScanState *node)

diff --git a/src/backend/parser/ag_scanner.l b/src/backend/parser/ag_scanner.l
@@ -34,6 +34,7 @@
 #include "mb/pg_wchar.h"
 
 #include "parser/ag_scanner.h"
+#include "utils/agtype.h"
 }
 
 %option 8bit
@@ -795,7 +796,7 @@ void *ag_yyrealloc(void *ptr, yy_size_t size, yyscan_t yyscanner)
     {
         if (size == 0)
         {
-            pfree(ptr);
+            pfree_if_not_null(ptr);
             return NULL;
         }
         else
@@ -812,7 +813,7 @@ void *ag_yyrealloc(void *ptr, yy_size_t size, yyscan_t yyscanner)
 void ag_yyfree(void *ptr, yyscan_t yyscanner)
 {
     if (ptr)
-        pfree(ptr);
+        pfree_if_not_null(ptr);
 }
 
 static void strbuf_init(strbuf *sb, int capacity)
@@ -825,7 +826,7 @@ static void strbuf_init(strbuf *sb, int capacity)
 static void strbuf_cleanup(strbuf *sb)
 {
     if (sb->buffer)
-        pfree(sb->buffer);
+        pfree_if_not_null(sb->buffer);
 }
 
 static void strbuf_append_buf(strbuf *sb, const char *b, const int len)
@@ -1121,8 +1122,8 @@ static void _numstr_to_decimal(const char *numstr, const int base, strbuf *sb)
         strbuf_append_buf(sb, &buf[buf_i], ndigits_per_remainder - buf_i);
     }
 
-    pfree(remainders);
-    pfree(words);
+    pfree_if_not_null(remainders);
+    pfree_if_not_null(words);
 }
 
 static uint32 hexdigit_value(const char c)

diff --git a/src/backend/parser/cypher_analyze.c b/src/backend/parser/cypher_analyze.c
@@ -108,7 +108,7 @@ static void post_parse_analyze(ParseState *pstate, Query *query)
         }
 
         /* reset extra_node */
-        pfree(extra_node);
+        pfree_if_not_null(extra_node);
         extra_node = NULL;
     }
 }
@@ -277,7 +277,7 @@ static void build_explain_query(Query *query, Node *explain_node)
     ((ExplainStmt *)explain_node)->options = NULL;
 
     /* we need to free query_node as it is no longer needed */
-    pfree(query_node);
+    pfree_if_not_null(query_node);
 }
 
 static bool is_rte_cypher(RangeTblEntry *rte)

diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y
@@ -26,6 +26,7 @@
 #include "parser/cypher_gram.h"
 #include "parser/cypher_parse_node.h"
 #include "parser/scansup.h"
+#include "utils/agtype.h"
 
 // override the default action for locations
 #define YYLLOC_DEFAULT(current, rhs, n) \
@@ -2714,7 +2715,7 @@ static char *create_unique_name(char *prefix_name)
     /* if we created the prefix, we need to free it */
     if (prefix_name == NULL || strlen(prefix_name) <= 0)
     {
-        pfree(prefix);
+        pfree_if_not_null(prefix);
     }
 
     return name;

diff --git a/src/backend/parser/cypher_parse_agg.c b/src/backend/parser/cypher_parse_agg.c
@@ -31,6 +31,7 @@
 #include "parser/cypher_parse_agg.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/agtype.h"
 
 typedef struct
 {
@@ -846,7 +847,7 @@ static List * expand_grouping_sets(List *groupingSets, int limit)
         while (result_len-- > 0)
             result = lappend(result, *ptr++);
 
-        pfree(buf);
+        pfree_if_not_null(buf);
     }
 
     return result;

diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c
@@ -181,7 +181,7 @@ static void create_GRAPH_global_hashtables(GRAPH_global_context *ggctx)
     ggctx->vertex_hashtable = hash_create(vhn, VERTEX_HTAB_INITIAL_SIZE,
                                           &vertex_ctl,
                                           HASH_ELEM | HASH_FUNCTION);
-    pfree(vhn);
+    pfree_if_not_null(vhn);
 
     /* initialize the edge hashtable */
     MemSet(&edge_ctl, 0, sizeof(edge_ctl));
@@ -190,7 +190,7 @@ static void create_GRAPH_global_hashtables(GRAPH_global_context *ggctx)
     edge_ctl.hash = tag_hash;
     ggctx->edge_hashtable = hash_create(ehn, EDGE_HTAB_INITIAL_SIZE, &edge_ctl,
                                         HASH_ELEM | HASH_FUNCTION);
-    pfree(ehn);
+    pfree_if_not_null(ehn);
 }
 
 /* helper function to get a List of all label names for the specified graph */
@@ -624,7 +624,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
     }
 
     /* free the graph name */
-    pfree(ggctx->graph_name);
+    pfree_if_not_null(ggctx->graph_name);
     ggctx->graph_name = NULL;
 
     ggctx->graph_oid = InvalidOid;
@@ -656,7 +656,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
         }
 
         /* free the vertex's datumCopy properties */
-        pfree(DatumGetPointer(value->vertex_properties));
+        pfree_if_not_null(DatumGetPointer(value->vertex_properties));
         value->vertex_properties = 0;
 
         /* free the edge list associated with this vertex */
@@ -698,7 +698,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
         }
 
         /* free the edge's datumCopy properties */
-        pfree(DatumGetPointer(value->edge_properties));
+        pfree_if_not_null(DatumGetPointer(value->edge_properties));
         value->edge_properties = 0;
 
         /* move to the next edge */
@@ -721,7 +721,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
     ggctx->edge_hashtable = NULL;
 
     /* free the context */
-    pfree(ggctx);
+    pfree_if_not_null(ggctx);
     ggctx = NULL;
 
     return true;
@@ -1224,7 +1224,7 @@ Datum age_vertex_stats(PG_FUNCTION_ARGS)
     ggctx = manage_GRAPH_global_contexts(graph_name, graph_oid);
 
     /* free the graph name */
-    pfree(graph_name);
+    pfree_if_not_null(graph_name);
 
     /* get the id */
     agtv_temp = GET_AGTYPE_VALUE_OBJECT_VALUE(agtv_vertex, "id");

diff --git a/src/backend/utils/adt/age_graphid_ds.c b/src/backend/utils/adt/age_graphid_ds.c
@@ -144,14 +144,14 @@ void free_ListGraphId(ListGraphId *container)
     {
         next_node = curr_node->next;
         /* we can do this because this is just a list of ints */
-        pfree(curr_node);
+        pfree_if_not_null(curr_node);
         container->size--;
         curr_node = next_node;
     }
 
     Assert(container->size == 0);
     /* free the container */
-    pfree(container);
+    pfree_if_not_null(container);
 }
 
 /* helper function to create a new, empty, graphid stack */
@@ -188,7 +188,7 @@ void free_graphid_stack(ListGraphId *stack)
         GraphIdNode *next = stack->head->next;
 
         /* free the head element */
-        pfree(stack->head);
+        pfree_if_not_null(stack->head);
         /* move the head to the next */
         stack->head = next;
     }
@@ -253,7 +253,7 @@ graphid pop_graphid_stack(ListGraphId *stack)
     stack->head = stack->head->next;
     stack->size--;
     /* free the element */
-    pfree(node);
+    pfree_if_not_null(node);
 
     /* return the id */
     return id;

diff --git a/src/backend/utils/adt/age_session_info.c b/src/backend/utils/adt/age_session_info.c
@@ -125,12 +125,12 @@ void reset_session_info(void)
     {
         if (session_info_graph_name != NULL)
         {
-            pfree(session_info_graph_name);
+            pfree_if_not_null(session_info_graph_name);
         }
 
         if (session_info_cypher_statement != NULL)
         {
-            pfree(session_info_cypher_statement);
+            pfree_if_not_null(session_info_cypher_statement);
         }
     }
 

diff --git a/src/backend/utils/adt/age_vle.c b/src/backend/utils/adt/age_vle.c
@@ -314,7 +314,7 @@ static void create_VLE_local_state_hashtable(VLE_local_context *vlelctx)
                                                 EDGE_STATE_HTAB_INITIAL_SIZE,
                                                 &edge_state_ctl,
                                                 HASH_ELEM | HASH_FUNCTION);
-    pfree(eshn);
+    pfree_if_not_null(eshn);
 }
 
 /*
@@ -401,14 +401,14 @@ static void free_VLE_local_context(VLE_local_context *vlelctx)
     /* free the stored graph name */
     if (vlelctx->graph_name != NULL)
     {
-        pfree(vlelctx->graph_name);
+        pfree_if_not_null(vlelctx->graph_name);
         vlelctx->graph_name = NULL;
     }
 
     /* free the stored edge label name */
     if (vlelctx->edge_label_name != NULL)
     {
-        pfree(vlelctx->edge_label_name);
+        pfree_if_not_null(vlelctx->edge_label_name);
         vlelctx->edge_label_name = NULL;
     }
 
@@ -430,15 +430,15 @@ static void free_VLE_local_context(VLE_local_context *vlelctx)
     }
 
     /* free the containers */
-    pfree(vlelctx->dfs_vertex_stack);
-    pfree(vlelctx->dfs_edge_stack);
-    pfree(vlelctx->dfs_path_stack);
+    pfree_if_not_null(vlelctx->dfs_vertex_stack);
+    pfree_if_not_null(vlelctx->dfs_edge_stack);
+    pfree_if_not_null(vlelctx->dfs_path_stack);
     vlelctx->dfs_vertex_stack = NULL;
     vlelctx->dfs_edge_stack = NULL;
     vlelctx->dfs_path_stack = NULL;
 
     /* and finally the context itself */
-    pfree(vlelctx);
+    pfree_if_not_null(vlelctx);
     vlelctx = NULL;
 }
-Original file line number
+Diff line change
@@ Expand Up @@
                                                        Oid odl_rel_oid,
                                                        void *arg);
     PG_FUNCTION_INFO_V1(create_vlabel);
     /*
@@ Expand Down @@