fix: free permanent identifiers

lib/lpc/compiler.c

@@ -2594,7 +2594,6 @@ int add_program_file (const char *name, int top) {
 void init_lpc_compiler(size_t max_locals, const char* include_dirs) {
   init_instrs ();
   init_identifiers ();
-  init_keywords ();
   init_predefines ();
   num_local_variables_allowed = max_locals;
   init_locals ();

lib/lpc/identifier.c

@@ -5,9 +5,11 @@
 #include "src/std.h"
 #include "hash.h"
 #include "identifier.h"
+#include "lex.h"
 
 ident_hash_elem_list_t *ihe_list = NULL;
 
+static size_t num_keywords = 0;
 static int num_free = 0;
 static ident_hash_elem_t **ident_hash_table; /* ident hash, current position */
 static ident_hash_elem_t **ident_hash_head;  /* ident hash, head of permanent idents */
@@ -77,19 +79,25 @@ ident_hash_elem_t *lookup_ident (const char *name) {
  * @param name The name of the identifier. No reference is made to the string after this call.
  * @return A pointer to the identifier hash element.
  */
-ident_hash_elem_t* find_or_add_perm_ident (char *name) {
+ident_hash_elem_t* find_or_add_perm_ident (char *name, short token_flag) {
   int h = IdentHash (name);
   ident_hash_elem_t *hptr, *hptr2;
 
   if ((hptr = ident_hash_table[h]))
     {
       if (!strcmp (hptr->name, name))
-        return hptr; /* found */
+        {
+          hptr->token |= token_flag;
+          return hptr; /* found */
+        }
       hptr2 = hptr->next;
       while (hptr2 != hptr)
         {
           if (!strcmp (hptr2->name, name))
-            return hptr2; /* found */
+            {
+              hptr2->token |= token_flag;
+              return hptr2; /* found */
+            }
           hptr2 = hptr2->next;
         }
       /* collision, add to slot, a circular linked list */
@@ -110,7 +118,7 @@ ident_hash_elem_t* find_or_add_perm_ident (char *name) {
 
   /* a new permanent identifier is added */
   hptr->name = name;
-  hptr->token = 0;
+  hptr->token = token_flag;
   hptr->sem_value = 0;
   hptr->dn.simul_num = -1;
   hptr->dn.local_num = -1;
@@ -349,6 +357,9 @@ void init_identifiers () {
     {
       ident_hash_table[i] = 0;
     }
+
+  /* add keywords */
+  num_keywords = init_keywords ();
 }
 
 /**
@@ -357,33 +368,36 @@ void init_identifiers () {
  * (reserved words are not freed since they point to static memory locations)
  */
 void deinit_identifiers () {
-  int i, n = 0;
+  int i, n = 0, r = 0;
   free_unused_identifiers ();
-  /* free identifiers with IHE_EFUN flag */
+  /* free permanent identifiers without IHE_RESWORD flag */
   for (i = 0; i < IDENT_HASH_SIZE; i++)
     {
       ident_hash_elem_t *head, *hptr = head = ident_hash_table[i];
-      while (hptr && (hptr->next != head))
+      while (hptr)
         {
-          if (hptr->token & IHE_EFUN)
+          ident_hash_elem_t *tmp = hptr;
+          hptr = hptr->next;
+          if (!(tmp->token & IHE_RESWORD))
             {
-              ident_hash_elem_t *tmp = hptr;
-              hptr = hptr->next;
-              FREE (tmp); /* allocated by find_or_add_perm_ident() */
+              /* non-reserved words permanent identifiers includes efuns and simul_efuns.
+               * They are allocated in find_or_add_perm_ident() and never freed when finished compiling.
+               * So we free them here.
+               */
+              FREE (tmp);
               n++;
             }
           else
-            {
-              if (!(hptr->token & IHE_RESWORD))
-                debug_warn ("leaked identifier (token: 0x%x): %s", hptr->token, hptr->name);
-              hptr = hptr->next;
-            }
+            r++; /* reserved words are actually global variables of type keyword_t, and cannot be freed */
+
+          if (hptr == head) /* end of circular linked list */
+            break;
         }
       ident_hash_table[i] = NULL;
     }
   FREE (ident_hash_table);
   ident_hash_table = NULL;
   ident_hash_head = NULL;
   ident_hash_tail = NULL;
-  opt_trace (TT_MEMORY|3, "freed %d identifiers", n);
+  debug_info ("freed %d permanent identifiers (leaked %d).", n, r - (int)num_keywords);
 }

lib/lpc/identifier.h

@@ -54,7 +54,7 @@ typedef struct keyword_s {
 #define FOA_NEEDS_MALLOC       0x2
 ident_hash_elem_t *find_or_add_ident(char *, int);
 
-ident_hash_elem_t *find_or_add_perm_ident(char *);
+ident_hash_elem_t *find_or_add_perm_ident(char *, short);
 ident_hash_elem_t *lookup_ident(const char *);
 void free_unused_identifiers(void);
 void add_keyword (const char *name, keyword_t * entry);

lib/lpc/lex.c

@@ -3504,14 +3504,15 @@ const char* main_file_name () {
   return is->file;
 }
 
-void init_keywords (void) {
+size_t init_keywords (void) {
   size_t i;
 
   for (i = 0; i < sizeof(reswords) / sizeof(reswords[0]); i++)
     {
       keyword_t *entry = &reswords[i];
       add_keyword (entry->word, entry);
     }
+  return i;
 }
 
 void init_predefines (void) {
@@ -3520,8 +3521,7 @@ void init_predefines (void) {
   ident_hash_elem_t *ihe;
   for (i = 0; i < sizeof(predefs) / sizeof(predefs[0]); i++)
     {
-      ihe = find_or_add_perm_ident (predefs[i].word);
-      ihe->token |= IHE_EFUN;
+      ihe = find_or_add_perm_ident (predefs[i].word, IHE_EFUN);
       ihe->sem_value++;
       ihe->dn.efun_num = i;
     }

lib/lpc/lex.h

@@ -96,7 +96,7 @@ extern int efun_arg_types[];
 extern char yytext[MAXLINE];
 extern keyword_t predefs[];
 
-void init_keywords (void);
+size_t init_keywords (void);
 void init_predefines (void);
 
 void push_function_context(void);

src/simul_efun.c

@@ -240,11 +240,10 @@ static void find_or_add_simul_efun (program_t* prog, function_number_t index, fu
            * 3. Else if its an efun, use that.
            * 4. Else, handle forward declaration or error.
            */
-          ihe = find_or_add_perm_ident (simuls_sorted[j].name);
+          ihe = find_or_add_perm_ident (simuls_sorted[j].name, IHE_SIMUL);
           DEBUG_CHECK1 (ihe != NULL,
                         "find_or_add_perm_ident() returned NULL for simul_efun '%s'\n",
                         simuls_sorted[j].name);
-          ihe->token |= IHE_SIMUL;
           ihe->sem_value++;
           ihe->dn.simul_num = (short)simuls_sorted[j].index;
           simuls[simuls_sorted[j].index].index = runtime_index;
@@ -262,11 +261,10 @@ static void find_or_add_simul_efun (program_t* prog, function_number_t index, fu
   simuls_sorted[first].index = (int)num_simuls;
   opt_trace (TT_SIMUL_EFUN|2, "added simul_efun #%d: %s", simuls_sorted[first].index, funp->name);
   /* update identifier hash, so LPC compiler don't have to call find_simul_efun() */
-  ihe = find_or_add_perm_ident (funp->name);
+  ihe = find_or_add_perm_ident (funp->name, IHE_SIMUL);
   DEBUG_CHECK1 (ihe != NULL,
                 "find_or_add_perm_ident() returned NULL for simul_efun '%s'\n",
                 funp->name);
-  ihe->token |= IHE_SIMUL;
   ihe->sem_value++;
   ihe->dn.simul_num = (short)num_simuls++; /* new simul_efun */
   ref_string (funp->name); /* will be freed in remove_simuls() */

tests/test_lpc_lexer/fixtures.hpp

@@ -39,7 +39,6 @@ class LPCLexerTest: public Test {
         init_strings (8192, 1000000); // LPC compiler needs this since prolog()
         init_instrs();
         init_identifiers();
-        init_keywords ();
         init_predefines ();
         set_inc_list (CONFIG_STR (__INCLUDE_DIRS__)); // automatically freed in deinit_lpc_compiler()
     }

tests/test_simul_efuns/test_simul_efuns.cpp

@@ -101,6 +101,18 @@ TEST_F(SimulEfunsTest, protectSimulEfun)
     else {
         current_object = master_ob;
         destruct_object (simul_efun_ob); // should raise error
+        /*
+         * The reason to prevent simul_efun_ob from being destructed while master_ob exists:
+         * 1. The order of function definitions in simul_efun_ob is used as simul_num in the permanent identifier table.
+         * 2. When a LPC object that calls simul_efun is loaded, the corresponding simul_num is stored in the compiled
+         *    opcode F_SIMUL_EFUN at compile time.
+         * 3. If simul_efun_ob is destructed and reloaded, the order of function definitions may change,
+         *    causing the simul_num to refer to a different function than intended.
+         * 
+         * In original LPMud and MudOS, the simul_efun_ob is never destructed once loaded.
+         * Neolith allows destructing simul_efun_ob only when master_ob does not exist, which can only be triggered by
+         * using the --pedantic option that enables subsystem teardown when the driver.
+         */
     }
     pop_context (&econ);
     FAIL() << "destruct_object(simul_efun_ob) did not raise error when master object exists.";