COMMON: Fixed memory handling issues with UNITs

Author: chrisws

ChangeLog

@@ -1,3 +1,6 @@
+2016-04-20 (0.12.6)
+	Fixed memory handling issues with UNITs
+
 2016-03-29 (0.12.6)
 	Removed TICKSPERSEC
 	Removed BALLOC, MALLOC and VADR keywords. Removed duplicate ENVIRON

Makefile.am

@@ -13,6 +13,9 @@ deb:
 test:
 	(cd @TEST_DIR@ && make test)
 
+leak-test:
+	(cd src/platform/unix && make leak-test)
+
 release:
 	(cd $(SUBDIRS) && make release)
 

src/common/scan.c

@@ -108,7 +108,7 @@ bc_symbol_rec_t *add_imptable_rec(const char *proc_name, int lib_id, int symbol_
     comp_imptable.elem = (bc_symbol_rec_t **)realloc(comp_imptable.elem,
                                                      (comp_imptable.count + 1) * sizeof(bc_symbol_rec_t **));
   } else {
-    comp_imptable.elem = (bc_symbol_rec_t **)malloc(comp_imptable.count * sizeof(bc_symbol_rec_t **));
+    comp_imptable.elem = (bc_symbol_rec_t **)malloc(sizeof(bc_symbol_rec_t **));
   }
   comp_imptable.elem[comp_imptable.count] = sym;
   comp_imptable.count++;
@@ -128,7 +128,7 @@ void add_libtable_rec(const char *lib, int uid, int type) {
     comp_libtable.elem = (bc_lib_rec_t **)realloc(comp_libtable.elem,
                                                  (comp_libtable.count + 1) * sizeof(bc_lib_rec_t **));
   } else {
-    comp_libtable.elem = (bc_lib_rec_t **)malloc(comp_libtable.count * sizeof(bc_lib_rec_t **));
+    comp_libtable.elem = (bc_lib_rec_t **)malloc(sizeof(bc_lib_rec_t **));
   }
   comp_libtable.elem[comp_libtable.count] = imlib;
   comp_libtable.count++;
@@ -4346,31 +4346,32 @@ byte_code comp_create_bin() {
   hdr.var_count = comp_varcount;
   hdr.lab_count = comp_labcount;
   hdr.data_ip = comp_first_data_ip;
-  hdr.size = sizeof(bc_head_t) + comp_prog.count + (comp_labcount * ADDRSZ)
-             + sizeof(unit_sym_t) * comp_expcount + sizeof(bc_lib_rec_t) * comp_libcount
-             + sizeof(bc_symbol_rec_t) * comp_impcount;
-
+  hdr.size = sizeof(bc_head_t) + comp_prog.count + (comp_labcount * ADDRSZ) +
+             sizeof(unit_sym_t) * comp_expcount +
+             sizeof(bc_lib_rec_t) * comp_libcount +
+             sizeof(bc_symbol_rec_t) * comp_impcount;
   if (comp_unit_flag) {
     hdr.size += sizeof(unit_file_t);
   }
 
   hdr.lib_count = comp_libcount;
   hdr.sym_count = comp_impcount;
-
   if (comp_unit_flag) {
+    memset(&uft, 0, sizeof(unit_file_t));
+
     // it is a unit... add more info
-    bc.size = hdr.size + 4;
+    bc.size = hdr.size;
     bc.code = malloc(bc.size);
 
     // unit header
     memcpy(&uft.sign, "SBUn", 4);
     uft.version = 1;
+
     strcpy(uft.base, comp_unit_name);
     uft.sym_count = comp_expcount;
 
-    cp = bc.code;
-    memcpy(cp, &uft, sizeof(unit_file_t));
-    cp += sizeof(unit_file_t);
+    memcpy(bc.code, &uft, sizeof(unit_file_t));
+    cp = bc.code + sizeof(unit_file_t);
 
     // unit symbol table (export)
     for (i = 0; i < uft.sym_count; i++) {
@@ -4423,7 +4424,6 @@ byte_code comp_create_bin() {
   if (!opt_quiet && !opt_interactive) {
     log_printf("\n");
     log_printf(RES_NUMBER_OF_VARS, comp_varcount, comp_varcount - 18);
-    // system variables
     log_printf(RES_NUMBER_OF_LABS, comp_labcount);
     log_printf(RES_NUMBER_OF_UDPS, comp_udpcount);
     log_printf(RES_CODE_SIZE, comp_prog.count);
@@ -4514,11 +4514,12 @@ int comp_compile(const char *sb_file_name) {
     }
   }
 
+  int is_unit = comp_unit_flag;
   comp_close();
   close_task(tid);
   activate_task(prev_tid);
 
-  if (opt_nosave) {
+  if (opt_nosave && !is_unit) {
     ctask->bytecode = bc.code;
   } else if (bc.code) {
     free(bc.code);

src/common/smbas.h

@@ -28,8 +28,7 @@ extern "C" {
  */
 typedef struct {
   char sign[4]; /**< always "SBEx" */
-  byte unused;
-  word ver; /**< version of this structure */
+  dword ver; /**< version of this structure */
   dword sbver; /**< version of SB */
   dword flags; /**< flags
    b0 = Big-endian CPU
@@ -41,12 +40,10 @@ typedef struct {
   dword var_count; /**< number of variables */
   dword lab_count; /**< number of labels */
   dword data_ip; /**< default DATA position */
-  dword uds_tab_ip; /**< location of struct mapping table */
 
   // ver 2
-  word lib_count; /**< libraries count (needed units) */
+  dword lib_count; /**< libraries count (needed units) */
   dword sym_count; /**< symbol count (linked-symbols) */
-  char reserved[26];
 } bc_head_t;
 
 /**
@@ -173,7 +170,6 @@ EXTERN char gsb_last_errmsg[SB_ERRMSG_SIZE + 1]; /**< last error message     */
 #define prog_libtable       ctask->sbe.exec.libtable
 #define prog_symtable       ctask->sbe.exec.symtable
 #define prog_exptable       ctask->sbe.exec.exptable
-#define prog_uds_tab_ip     ctask->sbe.exec.uds_tab_ip
 #define prog_timer          ctask->sbe.exec.timer
 #define comp_extfunctable   ctask->sbe.comp.extfunctable
 #define comp_extfunccount   ctask->sbe.comp.extfunccount
@@ -207,7 +203,6 @@ EXTERN char gsb_last_errmsg[SB_ERRMSG_SIZE + 1]; /**< last error message     */
 #define comp_udpcount       ctask->sbe.comp.udpcount
 #define comp_udpsize        ctask->sbe.comp.udpsize
 #define comp_next_field_id  ctask->sbe.comp.next_field_id
-#define comp_uds_tab_ip     ctask->sbe.comp.uds_tab_ip
 #define comp_use_global_vartable    ctask->sbe.comp.use_global_vartable
 #define comp_stack          ctask->sbe.comp.stack
 #define comp_sp             ctask->sbe.comp.stack.count

src/common/units.c

@@ -41,6 +41,7 @@ void unit_mgr_close() {
   unit_count = 0;
   if (units) {
     free(units);
+    units = NULL;
   }
 }
 
@@ -105,7 +106,7 @@ int find_unit(const char *name, char *file) {
  * @return the unit handle or -1 on error
  */
 int open_unit(const char *file) {
-  int h, i;
+  int h;
   unit_t u;
   int uid = -1;
 
@@ -161,34 +162,23 @@ int open_unit(const char *file) {
   }
 
   // load symbol-table
-  u.symbols = (unit_sym_t *) malloc(u.hdr.sym_count * sizeof(unit_sym_t));
-  read(h, u.symbols, u.hdr.sym_count * sizeof(unit_sym_t));
+  if (u.hdr.sym_count) {
+    u.symbols = (unit_sym_t *)malloc(u.hdr.sym_count * sizeof(unit_sym_t));
+    read(h, u.symbols, u.hdr.sym_count * sizeof(unit_sym_t));
+  }
 
   // setup the rest
   strcpy(u.name, unitname);
   u.status = unit_loaded;
 
   // add unit
-  if (unit_count == 0) {
-    // this is the first unit
-    uid = unit_count;
-    unit_count++;
-    units = (unit_t *) malloc(unit_count * sizeof(unit_t));
-  } else {
-    // search for an empty entry
-    for (i = 0; i < unit_count; i++) {
-      if (units[i].status == unit_undefined) {
-        uid = i;
-        break;
-      }
-    }
+  uid = unit_count;
+  unit_count++;
 
-    // resize the table
-    if (uid == -1) {
-      uid = unit_count;
-      unit_count++;
-      units = (unit_t *) realloc(units, unit_count * sizeof(unit_t));
-    }
+  if (units == NULL) {
+    units = (unit_t *)malloc(unit_count * sizeof(unit_t));
+  } else {
+    units = (unit_t *)realloc(units, unit_count * sizeof(unit_t));
   }
 
   // copy unit's data
@@ -207,9 +197,7 @@ int open_unit(const char *file) {
  */
 int close_unit(int uid) {
   if (uid >= 0) {
-    unit_t *u;
-
-    u = &units[uid];
+    unit_t *u = &units[uid];
     if (u->status == unit_loaded) {
       u->status = unit_undefined;
       free(u->symbols);
@@ -233,11 +221,8 @@ int import_unit(int uid) {
   char buf[SB_KEYWORD_SIZE + 1];int i;
 
   if (uid >= 0) {
-    unit_t *u;
-
-    u = &units[uid];
+    unit_t *u = &units[uid];
     if (u->status == unit_loaded) {
-
       for (i = 0; i < u->hdr.sym_count; i++) {
         // build the name
         // with any path component removed from the name
@@ -246,13 +231,13 @@ int import_unit(int uid) {
             dir_sep ? dir_sep + 1 : u->hdr.base, u->symbols[i].symbol);
 
         switch (u->symbols[i].type) {
-          case stt_function:
+        case stt_function:
           comp_add_external_func(buf, uid | UID_UNIT_BIT);
           break;
-          case stt_procedure:
+        case stt_procedure:
           comp_add_external_proc(buf, uid | UID_UNIT_BIT);
           break;
-          case stt_variable:
+        case stt_variable:
           comp_add_external_var(buf, uid | UID_UNIT_BIT);
           break;
         };
@@ -272,7 +257,7 @@ int import_unit(int uid) {
 /**
  * execute a call to a unit
  */
-int unit_exec(int lib_id, int index, var_t * ret) {
+int unit_exec(int lib_id, int index, var_t *ret) {
   unit_sym_t *us;               // unit's symbol data
   bc_symbol_rec_t *ps;          // program's symbol data
   int my_tid;
@@ -282,15 +267,12 @@ int unit_exec(int lib_id, int index, var_t * ret) {
   ps = &prog_symtable[index];
   us = &(taskinfo(ps->task_id)->sbe.exec.exptable[ps->exp_idx]);
 
-  //
   switch (ps->type) {
   case stt_variable:
     break;
   case stt_procedure:
     exec_sync_variables(1);
-
     cmd_call_unit_udp(kwPROC, ps->task_id, us->address, 0);
-
     activate_task(ps->task_id);
     if (prog_error) {
       gsb_last_error = prog_error;
@@ -309,9 +291,7 @@ int unit_exec(int lib_id, int index, var_t * ret) {
 
   case stt_function:
     exec_sync_variables(1);
-
     cmd_call_unit_udp(kwFUNC, ps->task_id, us->address, us->vid);
-
     activate_task(ps->task_id);
     if (prog_error) {
       gsb_last_error = prog_error;

src/common/units.h

@@ -64,7 +64,7 @@ typedef enum {
  * unit: file header
  */
 typedef struct {
-  dword sign[4]; /**< Always "SBUn" */
+  char sign[4]; /**< Always "SBUn" */
   int version; /**< version of this structure, always 1 */
   char base[SB_KEYWORD_SIZE + 1]; /**< unit-base name */
   int  sym_count; /**< number of symbols */

src/platform/unix/Makefile.am

@@ -37,3 +37,8 @@ test: ${bin_PROGRAMS}
       cat test.out;                                           \
     fi ;                                                      \
   done;
+
+leak-test: ${bin_PROGRAMS}
+	@for utest in $(UNIT_TESTS); do                             \
+    valgrind --leak-check=full ./${bin_PROGRAMS} ${TEST_DIR}/$${utest}.bas 1>/dev/null; \
+  done;