Garbage collection - #5

Renamed:
- sl_safe_malloc  -> mem_alloc
- sl_safe_calloc  -> mem_calloc
- sl_safe_strdup  -> mem_strdup
- sl_safe_realloc -> mem_realloc

Non-working, but compiles. I don't really like the 'PtrArray' method.

Instead of returning and receiving 'Pool' structures (like in my 'libpool'
project), it initializes and operates on a single global expression pool. This
should make the interface simpler, specially when integrating it with the
garbage collector and the 'expr_new' function.

Tests succeeded with no memory leaks.

* Move old body of 'expr_free' to new 'free_expr_members' static function, call
  it from 'pool_free'.
* Add 'BASE_POOL_SZ' macro to 'expr_pool.h'.
* Initialize and close pool from 'main'.

Pool allocator

Use a "unmark, mark, collect" approach. Not the most efficient, but it should
work for now.

Compiles fine, not tested at runtime.

This apparently fixes a big bug, although I don't understand how it's
related. If you run the 'arith.lisp' test, the program will crash trying to
read address 0x20 when accessing 'g_expr_pool->free_node'. If you put a
watchpoint in this member, you will eventually see it's being set to an invalid
address in the 161th call to 'pool_alloc':

    Hardware watchpoint 1: g_expr_pool->free_node

    Old value = (PoolNode *) 0x55555556d200
    New value = (PoolNode *) 0x555500000020
    pool_alloc () at src/expr_pool.c:159
    159     result->flags &= ~NODE_FREE;

Although people in the #gdb IRC channel told me that, in my architecture, it was
supposedly stopping at the instruction after the one that changed the
value. Either way, I don't understand where this 0x20 value came from, so this
commit might just delay the problem.

Also, this problem only happened when adding the following lines to
'repl_until_eof':

    diff --git a/src/main.c b/src/main.c
    index 75944b2..1e297ef 100644
    --- a/src/main.c
    +++ b/src/main.c
    @@ -99,6 +99,10 @@ static void repl_until_eof(Env* env, FILE* file, bool print_prompt,

             /* Free the evaluated expression */
             expr_free(evaluated);
    +
    +        gc_unmark_all();
    +        gc_mark_env(env);
    +        gc_collect();
         }
     }

About pointer ownership.

We can't do the same in 'expr_pool.h' since we use the structure directly (not
just pointers) and the compiler needs to know the size and offset of the
members.

We don't want to completely remove 'expr_free' and 'expr_list_free', since they
are still used in some parts of the program.

Perhaps in the future, when we replace this linked list approach with proper
cons pairs, we could remove 'expr_list_free'.

The values of the expressions might be pointers to allocated data (e.g. with
strings or lambdas), so we have to free that too. Since the expression "owns" a
unique pointer to its allocated data (i.e. the another expression won't point to
the same allocated string; perhaps to a copy) we should be able to free it
safely.

We should replace it to an assertion after the cons pair changes; hopefully in a
near future.

One expression in the second array might reference another expression in the
first array, but since it's been freed, it will segfault as soon as we try to
even read its flags.

This commit is not just for proving that garbage collection works. In future
commits, many calls to 'expr_clone' will be removed, and most expressions will
be re-used or modified directly (depending on the context, obviously). I decided
to keep these two commits (removing frees and removing clones) separate for
readability.

Replacing it will an assertion still fails.

Should have been included in commit 3c5c6a2

This, along with the previous commit (9a17f00), fixes a bug where 'pool_close'
tried to free expression members multiple times. The problem was caused in the
following case:

1. There was a symbol (for example) in the pool, whose string was freed from the
   heap when 'pool_close' was called.
2. After freeing that symbol, another expression of type 'EXPR_PARENT' was freed
   by 'pool_close'. However, this list contained a pointer to the symbol
   expression whose string we just freed from 'pool_close'. The old
   'free_expr_members' tried freeing the expression with 'expr_list_free', which
   called 'pool_free', which then freed the string again.

This is normally (i.e. when garbage-collecting) not a problem because the string
of the first symbol would be freed through 'pool_free', which would set
'NODE_FLAG_FREE' in the symbol's 'PoolNode'. Therefore, whenever the list is
reached, even if 'expr_list_free' tried freeing each element, 'pool_free' would
notice that it's already been freed (since it checks the flag).

The problem is that this flag is never set whenever we freed the members of the
expression from 'pool_close' (calling 'free_expr_members', not through
'pool_free'), so 'NODE_FLAG_FREE' was never set.

There are two possible solutions:

1. Simply mark the expression whose members we are freeing from 'pool_close'
   with 'NODE_FLAG_FREE'.
2. Don't call 'expr_list_free' from 'free_expr_members' entirely.

Although only one of them is necessary, I chose to add both, since it was not a
good idea to free lists recursively here anyway; specially since we want to
start reusing pointers, and we were not checking if they were in use somewhere
else (we are not checking for 'GCMARKED', but even if it was set, maybe we
weren't called from the garbage collector).

Changes:

* Rename 'free_expr_members' to 'free_heap_expr_members'.
* Don't free expressions recursively from 'free_heap_expr_members'.
* Mark expressions with freed members with 'NODE_FLAG_FREE' from 'pool_close'.

From commit 25fe243.

This is not really necessary, but it provides more accurate results in the
valgrind summary (since now 'VALGRIND_MEMPOOL_FREE' is being called). Before
this commit, valgrind showed that we were missing N frees, where N was the
number of non-free nodes when 'pool_close' was called.

The point of using valgrind is avoiding memory leaks and invalid memory accesses
in the pool (e.g. overflowing the size of an 'Expr', double-frees, etc.), not
controlling when we can access the 'ExprPool' or 'ArrayStart' structures. It's
very hard to access these structures by mistake; if we access them, for example
from the garbage collector, it's because we need to. However, the 'flags' member
of the 'PoolNode' structures, can be easily overwritten by casting a 'Expr*',
for example.

This commit doesn't fix anything, it just removes unnecessary macro calls.

* Remove 'POOL_FOREACH_ARRAYSTART' and 'POOL_FOREACH_ARRAYSTART_END' macros,
  replace with normal for-loops.
* Remove unnecessary calls to 'VALGRIND_MAKE_MEM_DEFINED' and
  'VALGRIND_MAKE_MEM_NOACCESS' from 'expr_pool.c'.

Since we store copies in the environment and not the actual pointers in the
globals, the garbage collector was freeing the globals, and they were being
overwritten with other values. When functions like 'prim_equal' tried to return
clones of 'g_tru', it returned an invalid value (like 'car', for example).