poke-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] pkl-ast: Avoid double-frees by keeping track of frees


From: Arsen Arsenović
Subject: [PATCH] pkl-ast: Avoid double-frees by keeping track of frees
Date: Thu, 27 Oct 2022 16:42:56 +0200

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25379
---
Hi,

I added a freed pointer set to pkl_ast_node_free, in order to prevent it from
attempting to go down or free invalid pointers.  This solves the two known
recursion bugs, which are now testcases.

One thing to consider is that freeing now allocates memory for the set, and
that maybe one of the other set impls in gnulib is a better fit, so that's
something to think about.

A nice thing that this lets us potentially do, though, is go over all allocated
ASTs so that we can descend through them to find dangling pointers (since we
now have a place where we can iterate a list of freshly-dangling pointers that
shouldn't be used anywhere ideally).  I might give implementing that a shot one
of these days, though I can't guarantee anything since I expect some pretty
hectic times ahead of me.

Have a good day!

 ChangeLog                             |  15 ++
 bootstrap.conf                        |   3 +
 libpoke/pkl-ast.c                     | 288 +++++++++++++++-----------
 testsuite/Makefile.am                 |   2 +
 testsuite/poke.pkl/recursion-bad-1.pk |   3 +
 testsuite/poke.pkl/recursion-bad-2.pk |  11 +
 6 files changed, 202 insertions(+), 120 deletions(-)
 create mode 100644 testsuite/poke.pkl/recursion-bad-1.pk
 create mode 100644 testsuite/poke.pkl/recursion-bad-2.pk

diff --git a/ChangeLog b/ChangeLog
index 37c67b50..718e0132 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2022-10-27  Arsen Arsenović  <arsen@aarsen.me>
+
+       * bootstrap.conf (libpoke_modules): Add set, xset and
+       linkedhash-set.
+       * libpoke/pkl-ast.c (pkl_ast_node_free): Allocate a visitation
+       set, and move most code...
+       (pkl_ast_node_free_1): ... to here, so that we can track and avoid
+       freed nodes.  Also, no longer consult IS_RECURSIVE for lifetime
+       information.
+       (visit_and_free): New function.  Frees and tracks what it freed,
+       to avoid double frees.
+       * testsuite/poke.pkl/recursion-bad-1.pk: New test.
+       * testsuite/poke.pkl/recursion-bad-2.pk: New test.
+       * testsuite/Makefile.am (EXTRA_DIST): Add the new tests.
+
 2022-10-25  Arsen Arsenović  <arsen@aarsen.me>
 
        * doc/Makefile.am (AM_MAKEINFOFLAGS): Enable the
diff --git a/bootstrap.conf b/bootstrap.conf
index 8a16795c..347aa551 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -82,11 +82,14 @@ libpoke_modules="
   gettime
   intprops
   isatty
+  linkedhash-set
   mkstemp
   nanosleep
   printf-posix
   random
   secure_getenv
+  set
+  xset
   snprintf
   stdarg
   stdbool
diff --git a/libpoke/pkl-ast.c b/libpoke/pkl-ast.c
index 2b7cff73..4ae3bf4b 100644
--- a/libpoke/pkl-ast.c
+++ b/libpoke/pkl-ast.c
@@ -25,6 +25,8 @@
 #include <inttypes.h>
 #include "string-buffer.h"
 #include "xalloc.h"
+#include "gl_linkedhash_set.h"
+#include "gl_xset.h"
 
 #include "pvm.h"
 #include "pvm-alloc.h" /* For pvm_{alloc,free}_uncollectable.  */
@@ -2253,18 +2255,46 @@ pkl_ast_node_free_chain (pkl_ast_node ast)
     }
 }
 
-/* Free all allocated resources used by AST.  Note that nodes marked
-   as "registered", as well as their children, are not disposed.  */
+/* Mark PTR as freed in VISITATIONS and free.  It is safe to call this
+   functions on freed pointers inside VISITATIONS as well as NULLs. */
 
-void
-pkl_ast_node_free (pkl_ast_node ast)
+static void
+visit_and_free (gl_set_t visitations, void *ptr)
 {
+  if (!ptr)
+    return;
+
+  bool added = gl_set_add (visitations, ptr);
+
+  /* This one was already freed.  */
+  if (!added)
+      return;
+
+  free (ptr);
+}
+
+/* Recursively descends down a tree, marking and freeing as it goes.
+   VISITATIONS is a set of pointers that has already been freed that this
+   function can track frees in.  */
+
+static void
+pkl_ast_node_free_1 (gl_set_t visitations, pkl_ast_node ast)
+{
+  /* Take care not to use pkl_ast_node_free here, as doing that will result in
+     a double-free, as it throws away the current visitation set.  In the same
+     vein, take care to mark and check VISITATIONS, using visit_and_free when
+     possible.  */
   pkl_ast_node t, n;
   size_t i;
+  bool added;
 
   if (ast == NULL)
     return;
 
+  /* Check if we already freed this one.  */
+  if (gl_set_search (visitations, ast))
+    return;
+
   assert (PKL_AST_REFCOUNT (ast) > 0);
 
   if (PKL_AST_REFCOUNT (ast) > 1)
@@ -2273,62 +2303,67 @@ pkl_ast_node_free (pkl_ast_node ast)
       return;
     }
 
+  /* OK, we need to free.  Mark this node as freed (it will be actually freed
+     at the end).  */
+  added = gl_set_add (visitations, ast);
+  assert (added);
+
   switch (PKL_AST_CODE (ast))
     {
     case PKL_AST_PROGRAM:
       for (t = PKL_AST_PROGRAM_ELEMS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
       break;
 
     case PKL_AST_SRC:
-      free (PKL_AST_SRC_FILENAME (ast));
+      visit_and_free (visitations, PKL_AST_SRC_FILENAME (ast));
       break;
 
     case PKL_AST_EXP:
 
       for (i = 0; i < PKL_AST_EXP_NUMOPS (ast); i++)
-        pkl_ast_node_free (PKL_AST_EXP_OPERAND (ast, i));
+        pkl_ast_node_free_1 (visitations, PKL_AST_EXP_OPERAND (ast, i));
 
       break;
 
     case PKL_AST_COND_EXP:
 
-      pkl_ast_node_free (PKL_AST_COND_EXP_COND (ast));
-      pkl_ast_node_free (PKL_AST_COND_EXP_THENEXP (ast));
-      pkl_ast_node_free (PKL_AST_COND_EXP_ELSEEXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_COND_EXP_COND (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_COND_EXP_THENEXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_COND_EXP_ELSEEXP (ast));
       break;
 
     case PKL_AST_ENUM:
 
-      pkl_ast_node_free (PKL_AST_ENUM_TAG (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ENUM_TAG (ast));
 
       for (t = PKL_AST_ENUM_VALUES (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
 
     case PKL_AST_ENUMERATOR:
 
-      pkl_ast_node_free (PKL_AST_ENUMERATOR_IDENTIFIER (ast));
-      pkl_ast_node_free (PKL_AST_ENUMERATOR_VALUE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ENUMERATOR_IDENTIFIER (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ENUMERATOR_VALUE (ast));
       break;
 
     case PKL_AST_TYPE:
 
-      free (PKL_AST_TYPE_NAME (ast));
+      visit_and_free (visitations, PKL_AST_TYPE_NAME (ast));
       switch (PKL_AST_TYPE_CODE (ast))
         {
         case PKL_TYPE_ARRAY:
           pvm_free_uncollectable (PKL_AST_TYPE_A_CLOSURES (ast));
 
-          pkl_ast_node_free (PKL_AST_TYPE_A_BOUND (ast));
-          pkl_ast_node_free (PKL_AST_TYPE_A_ETYPE (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_A_BOUND (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_A_ETYPE (ast));
           break;
         case PKL_TYPE_STRUCT:
           pvm_free_uncollectable (PKL_AST_TYPE_S_CLOSURES (ast));
@@ -2336,21 +2371,21 @@ pkl_ast_node_free (pkl_ast_node ast)
           for (t = PKL_AST_TYPE_S_ELEMS (ast); t; t = n)
             {
               n = PKL_AST_CHAIN (t);
-              pkl_ast_node_free (t);
+              pkl_ast_node_free_1 (visitations, t);
             }
           break;
         case PKL_TYPE_FUNCTION:
-          pkl_ast_node_free (PKL_AST_TYPE_F_RTYPE (ast));
-          pkl_ast_node_free (PKL_AST_TYPE_F_FIRST_OPT_ARG (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_F_RTYPE (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_F_FIRST_OPT_ARG 
(ast));
           for (t = PKL_AST_TYPE_F_ARGS (ast); t; t = n)
             {
               n = PKL_AST_CHAIN (t);
-              pkl_ast_node_free (t);
+              pkl_ast_node_free_1 (visitations, t);
             }
           break;
         case PKL_TYPE_OFFSET:
-          pkl_ast_node_free (PKL_AST_TYPE_O_UNIT (ast));
-          pkl_ast_node_free (PKL_AST_TYPE_O_BASE_TYPE (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_O_UNIT (ast));
+          pkl_ast_node_free_1 (visitations, PKL_AST_TYPE_O_BASE_TYPE (ast));
           break;
         case PKL_TYPE_INTEGRAL:
         case PKL_TYPE_STRING:
@@ -2362,76 +2397,76 @@ pkl_ast_node_free (pkl_ast_node ast)
 
     case PKL_AST_STRUCT_TYPE_FIELD:
 
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_NAME (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_SIZE (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_CONSTRAINT (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_LABEL (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_OPTCOND_PRE (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_TYPE_FIELD_OPTCOND_POST (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_SIZE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_CONSTRAINT 
(ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_INITIALIZER 
(ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_LABEL (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_OPTCOND_PRE 
(ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_TYPE_FIELD_OPTCOND_POST 
(ast));
       break;
 
     case PKL_AST_FUNC_TYPE_ARG:
 
-      pkl_ast_node_free (PKL_AST_FUNC_TYPE_ARG_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_TYPE_ARG_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_TYPE_ARG_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_TYPE_ARG_NAME (ast));
       break;
 
     case PKL_AST_INDEXER:
 
-      pkl_ast_node_free (PKL_AST_INDEXER_ENTITY (ast));
-      pkl_ast_node_free (PKL_AST_INDEXER_INDEX (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_INDEXER_ENTITY (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_INDEXER_INDEX (ast));
       break;
 
     case PKL_AST_TRIMMER:
 
-      pkl_ast_node_free (PKL_AST_TRIMMER_ENTITY (ast));
-      pkl_ast_node_free (PKL_AST_TRIMMER_FROM (ast));
-      pkl_ast_node_free (PKL_AST_TRIMMER_TO (ast));
-      pkl_ast_node_free (PKL_AST_TRIMMER_ADDEND (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRIMMER_ENTITY (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRIMMER_FROM (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRIMMER_TO (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRIMMER_ADDEND (ast));
       break;
 
     case PKL_AST_FUNC:
 
-      free (PKL_AST_FUNC_NAME (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_RET_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_BODY (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_FIRST_OPT_ARG (ast));
+      visit_and_free (visitations, PKL_AST_FUNC_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_RET_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_BODY (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_FIRST_OPT_ARG (ast));
       for (t = PKL_AST_FUNC_ARGS (ast); t; t = n)
             {
               n = PKL_AST_CHAIN (t);
-              pkl_ast_node_free (t);
+              pkl_ast_node_free_1 (visitations, t);
             }
       break;
 
     case PKL_AST_FUNC_ARG:
 
-      pkl_ast_node_free (PKL_AST_FUNC_ARG_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_ARG_IDENTIFIER (ast));
-      pkl_ast_node_free (PKL_AST_FUNC_ARG_INITIAL (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_ARG_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_ARG_IDENTIFIER (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNC_ARG_INITIAL (ast));
       break;
 
     case PKL_AST_STRING:
 
-      free (PKL_AST_STRING_POINTER (ast));
+      visit_and_free (visitations, PKL_AST_STRING_POINTER (ast));
       break;
 
     case PKL_AST_IDENTIFIER:
 
-      free (PKL_AST_IDENTIFIER_POINTER (ast));
+      visit_and_free (visitations, PKL_AST_IDENTIFIER_POINTER (ast));
       break;
 
     case PKL_AST_STRUCT_REF:
 
-      pkl_ast_node_free (PKL_AST_STRUCT_REF_STRUCT (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_REF_IDENTIFIER (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_REF_STRUCT (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_REF_IDENTIFIER (ast));
       break;
 
     case PKL_AST_STRUCT_FIELD:
 
-      pkl_ast_node_free (PKL_AST_STRUCT_FIELD_NAME (ast));
-      pkl_ast_node_free (PKL_AST_STRUCT_FIELD_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_FIELD_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_STRUCT_FIELD_EXP (ast));
       break;
 
     case PKL_AST_STRUCT:
@@ -2439,14 +2474,14 @@ pkl_ast_node_free (pkl_ast_node ast)
       for (t = PKL_AST_STRUCT_FIELDS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
       break;
 
     case PKL_AST_ARRAY_INITIALIZER:
 
-      pkl_ast_node_free (PKL_AST_ARRAY_INITIALIZER_INDEX (ast));
-      pkl_ast_node_free (PKL_AST_ARRAY_INITIALIZER_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ARRAY_INITIALIZER_INDEX (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ARRAY_INITIALIZER_EXP (ast));
       break;
 
     case PKL_AST_ARRAY:
@@ -2454,92 +2489,91 @@ pkl_ast_node_free (pkl_ast_node ast)
       for (t = PKL_AST_ARRAY_INITIALIZERS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
 
     case PKL_AST_DECL:
 
-      free (PKL_AST_DECL_SOURCE (ast));
-      pkl_ast_node_free (PKL_AST_DECL_NAME (ast));
-      pkl_ast_node_free (PKL_AST_DECL_INITIAL (ast));
+      visit_and_free (visitations, PKL_AST_DECL_SOURCE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_DECL_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_DECL_INITIAL (ast));
       break;
 
     case PKL_AST_OFFSET:
 
-      pkl_ast_node_free (PKL_AST_OFFSET_MAGNITUDE (ast));
-      pkl_ast_node_free (PKL_AST_OFFSET_UNIT (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_OFFSET_MAGNITUDE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_OFFSET_UNIT (ast));
       break;
 
     case PKL_AST_CAST:
 
-      pkl_ast_node_free (PKL_AST_CAST_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_CAST_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_CAST_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_CAST_EXP (ast));
       break;
 
     case PKL_AST_ISA:
 
-      pkl_ast_node_free (PKL_AST_ISA_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_ISA_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ISA_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ISA_EXP (ast));
       break;
 
     case PKL_AST_MAP:
 
-      pkl_ast_node_free (PKL_AST_MAP_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_MAP_IOS (ast));
-      pkl_ast_node_free (PKL_AST_MAP_OFFSET (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_MAP_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_MAP_IOS (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_MAP_OFFSET (ast));
       break;
 
     case PKL_AST_CONS:
 
-      pkl_ast_node_free (PKL_AST_CONS_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_CONS_VALUE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_CONS_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_CONS_VALUE (ast));
       break;
 
     case PKL_AST_FUNCALL:
 
-      pkl_ast_node_free (PKL_AST_FUNCALL_FUNCTION (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNCALL_FUNCTION (ast));
       for (t = PKL_AST_FUNCALL_ARGS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
 
     case PKL_AST_FUNCALL_ARG:
 
-      pkl_ast_node_free (PKL_AST_FUNCALL_ARG_EXP (ast));
-      pkl_ast_node_free (PKL_AST_FUNCALL_ARG_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNCALL_ARG_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FUNCALL_ARG_NAME (ast));
       break;
 
     case PKL_AST_VAR:
 
-      pkl_ast_node_free (PKL_AST_VAR_NAME (ast));
-      if (!PKL_AST_VAR_IS_RECURSIVE (ast))
-        pkl_ast_node_free (PKL_AST_VAR_DECL (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_VAR_NAME (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_VAR_DECL (ast));
       break;
 
     case PKL_AST_INCRDECR:
 
-      pkl_ast_node_free (PKL_AST_INCRDECR_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_INCRDECR_EXP (ast));
       break;
 
     case PKL_AST_LAMBDA:
 
-      pkl_ast_node_free (PKL_AST_LAMBDA_FUNCTION (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LAMBDA_FUNCTION (ast));
       break;
 
     case PKL_AST_ASM_EXP:
 
-      pkl_ast_node_free (PKL_AST_ASM_EXP_TYPE (ast));
-      pkl_ast_node_free (PKL_AST_ASM_EXP_TEMPLATE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ASM_EXP_TYPE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ASM_EXP_TEMPLATE (ast));
 
       for (t = PKL_AST_ASM_EXP_INPUTS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
@@ -2549,97 +2583,97 @@ pkl_ast_node_free (pkl_ast_node ast)
       for (t = PKL_AST_COMP_STMT_STMTS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
 
     case PKL_AST_ASS_STMT:
 
-      pkl_ast_node_free (PKL_AST_ASS_STMT_LVALUE (ast));
-      pkl_ast_node_free (PKL_AST_ASS_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ASS_STMT_LVALUE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ASS_STMT_EXP (ast));
       break;
 
     case PKL_AST_IF_STMT:
 
-      pkl_ast_node_free (PKL_AST_IF_STMT_EXP (ast));
-      pkl_ast_node_free (PKL_AST_IF_STMT_THEN_STMT (ast));
-      pkl_ast_node_free (PKL_AST_IF_STMT_ELSE_STMT (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_IF_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_IF_STMT_THEN_STMT (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_IF_STMT_ELSE_STMT (ast));
       break;
 
     case PKL_AST_LOOP_STMT:
 
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_ITERATOR (ast));
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_CONDITION (ast));
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_BODY (ast));
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_HEAD (ast));
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_TAIL (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_ITERATOR (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_CONDITION (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_BODY (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_HEAD (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_TAIL (ast));
 
       break;
 
     case PKL_AST_LOOP_STMT_ITERATOR:
 
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_ITERATOR_DECL (ast));
-      pkl_ast_node_free (PKL_AST_LOOP_STMT_ITERATOR_CONTAINER (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_ITERATOR_DECL (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_LOOP_STMT_ITERATOR_CONTAINER 
(ast));
 
       break;
 
     case PKL_AST_RETURN_STMT:
 
-      pkl_ast_node_free (PKL_AST_RETURN_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_RETURN_STMT_EXP (ast));
       break;
 
     case PKL_AST_EXP_STMT:
 
-      pkl_ast_node_free (PKL_AST_EXP_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_EXP_STMT_EXP (ast));
       break;
 
     case PKL_AST_TRY_STMT:
 
-      pkl_ast_node_free (PKL_AST_TRY_STMT_BODY (ast));
-      pkl_ast_node_free (PKL_AST_TRY_STMT_HANDLER (ast));
-      pkl_ast_node_free (PKL_AST_TRY_STMT_ARG (ast));
-      pkl_ast_node_free (PKL_AST_TRY_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_BODY (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_HANDLER (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_ARG (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_EXP (ast));
       break;
 
     case PKL_AST_TRY_STMT_BODY:
 
-      pkl_ast_node_free (PKL_AST_TRY_STMT_BODY_CODE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_BODY_CODE (ast));
       break;
 
     case PKL_AST_TRY_STMT_HANDLER:
 
-      pkl_ast_node_free (PKL_AST_TRY_STMT_HANDLER_CODE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_TRY_STMT_HANDLER_CODE (ast));
       break;
 
     case PKL_AST_FORMAT_ARG:
-      free (PKL_AST_FORMAT_ARG_SUFFIX (ast));
-      free (PKL_AST_FORMAT_ARG_BEGIN_SC (ast));
-      free (PKL_AST_FORMAT_ARG_END_SC (ast));
-      pkl_ast_node_free (PKL_AST_FORMAT_ARG_EXP (ast));
+      visit_and_free (visitations, PKL_AST_FORMAT_ARG_SUFFIX (ast));
+      visit_and_free (visitations, PKL_AST_FORMAT_ARG_BEGIN_SC (ast));
+      visit_and_free (visitations, PKL_AST_FORMAT_ARG_END_SC (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_FORMAT_ARG_EXP (ast));
       break;
 
     case PKL_AST_FORMAT:
       {
-        free (PKL_AST_FORMAT_PREFIX (ast));
+        visit_and_free (visitations, PKL_AST_FORMAT_PREFIX (ast));
         for (t = PKL_AST_FORMAT_ARGS (ast); t; t = n)
           {
             n = PKL_AST_CHAIN (t);
-            pkl_ast_node_free (t);
+            pkl_ast_node_free_1 (visitations, t);
           }
         for (t = PKL_AST_FORMAT_TYPES (ast); t; t = n)
           {
             n = PKL_AST_CHAIN (t);
-            pkl_ast_node_free (t);
+            pkl_ast_node_free_1 (visitations, t);
           }
-        pkl_ast_node_free (PKL_AST_FORMAT_FMT (ast));
+        pkl_ast_node_free_1 (visitations, PKL_AST_FORMAT_FMT (ast));
         break;
       }
 
     case PKL_AST_PRINT_STMT:
       {
-        pkl_ast_node_free (PKL_AST_PRINT_STMT_STR_EXP (ast));
-        pkl_ast_node_free (PKL_AST_PRINT_STMT_FORMAT (ast));
+        pkl_ast_node_free_1 (visitations, PKL_AST_PRINT_STMT_STR_EXP (ast));
+        pkl_ast_node_free_1 (visitations, PKL_AST_PRINT_STMT_FORMAT (ast));
         break;
       }
 
@@ -2649,22 +2683,22 @@ pkl_ast_node_free (pkl_ast_node ast)
 
 
     case PKL_AST_RAISE_STMT:
-      pkl_ast_node_free (PKL_AST_RAISE_STMT_EXP (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_RAISE_STMT_EXP (ast));
       break;
 
     case PKL_AST_ASM_STMT:
-      pkl_ast_node_free (PKL_AST_ASM_STMT_TEMPLATE (ast));
+      pkl_ast_node_free_1 (visitations, PKL_AST_ASM_STMT_TEMPLATE (ast));
 
       for (t = PKL_AST_ASM_STMT_INPUTS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       for (t = PKL_AST_ASM_STMT_OUTPUTS (ast); t; t = n)
         {
           n = PKL_AST_CHAIN (t);
-          pkl_ast_node_free (t);
+          pkl_ast_node_free_1 (visitations, t);
         }
 
       break;
@@ -2679,10 +2713,24 @@ pkl_ast_node_free (pkl_ast_node ast)
       assert (0);
     }
 
-  pkl_ast_node_free (PKL_AST_TYPE (ast));
+  pkl_ast_node_free_1 (visitations, PKL_AST_TYPE (ast));
+
+  /* Free this node, to match the mark above.  */
   free (ast);
 }
 
+/* Free all allocated resources used by AST.  Note that nodes marked
+   as "registered", as well as their children, are not disposed.  */
+
+void
+pkl_ast_node_free (pkl_ast_node ast)
+{
+  gl_set_t visitations = gl_set_create_empty (GL_LINKEDHASH_SET,
+                                              NULL, NULL, NULL);
+  pkl_ast_node_free_1 (visitations, ast);
+  gl_set_free (visitations);
+}
+
 /* Allocate and initialize a new AST and return it.  */
 
 pkl_ast
@@ -3198,7 +3246,7 @@ pkl_ast_print_1 (FILE *fp, pkl_ast_node ast, int indent)
       PRINT_AST_SUBAST (name, STRUCT_TYPE_FIELD_NAME);
       PRINT_AST_SUBAST (type, STRUCT_TYPE_FIELD_TYPE);
       PRINT_AST_IMM (computed_p, STRUCT_TYPE_FIELD_COMPUTED_P, "%d");
-      PRINT_AST_SUBAST (type, STRUCT_TYPE_FIELD_SIZE);
+      PRINT_AST_SUBAST (size, STRUCT_TYPE_FIELD_SIZE);
       PRINT_AST_SUBAST (exp, STRUCT_TYPE_FIELD_CONSTRAINT);
       PRINT_AST_SUBAST (exp, STRUCT_TYPE_FIELD_INITIALIZER);
       PRINT_AST_SUBAST (exp, STRUCT_TYPE_FIELD_LABEL);
diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
index 20a4ffa8..de61ea44 100644
--- a/testsuite/Makefile.am
+++ b/testsuite/Makefile.am
@@ -2041,6 +2041,8 @@ EXTRA_DIST = \
   poke.pkl/raise-6.pk \
   poke.pkl/raise-diag-1.pk \
   poke.pkl/rand-1.pk \
+  poke.pkl/recursion-bad-1.pk \
+  poke.pkl/recursion-bad-2.pk \
   poke.pkl/redef-diag-1.pk \
   poke.pkl/redef-diag-2.pk \
   poke.pkl/redef-diag-3.pk \
diff --git a/testsuite/poke.pkl/recursion-bad-1.pk 
b/testsuite/poke.pkl/recursion-bad-1.pk
new file mode 100644
index 00000000..0793a595
--- /dev/null
+++ b/testsuite/poke.pkl/recursion-bad-1.pk
@@ -0,0 +1,3 @@
+/* { dg-do compile } */
+/* https://sourceware.org/bugzilla/show_bug.cgi?id=25379 */
+{ fun foo = void: { foo (); } /* { dg-error "syntax error" } */
diff --git a/testsuite/poke.pkl/recursion-bad-2.pk 
b/testsuite/poke.pkl/recursion-bad-2.pk
new file mode 100644
index 00000000..26deb983
--- /dev/null
+++ b/testsuite/poke.pkl/recursion-bad-2.pk
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* Test that an error while typifying does not result in bad recursive frees.
+*/
+"xxx" in [1]; /* { dg-error "" } */
+
+fun fact = (uint<4> a) uint<64>:
+  {
+    if (a == 0)
+      return 1;
+    return a * fact (a);
+  }
-- 
2.38.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]