Fix handling of filenames in stack traces

- Fix a crash that happened if a stack element was in another file
- Optimize memory usage by not building lists of line references and of filenames
- Fix parsing of files with no filename at all.

This fixes most occurrences of 'unsupported line_ref tag: 0' (fixes #1388)

Signed-off-by: Paul Guyot <pguyot@kallisys.net>

Author: pguyot

CHANGELOG.md

@@ -57,6 +57,7 @@ and a race condition in otp_socket code
 memory error
 - Fixed possible concurrency problems in ESP32 UART driver
 - Fixed concurrency and memory leak related to links and monitors
+- Fixed issues with parsing of line references for stack traces
 
 ### Changed
 

src/libAtomVM/module.c

@@ -62,7 +62,7 @@
 static struct LiteralEntry *module_build_literals_table(const void *literalsBuf);
 static void module_add_label(Module *mod, int index, const uint8_t *ptr);
 static enum ModuleLoadResult module_build_imported_functions_table(Module *this_module, uint8_t *table_data, GlobalContext *glb);
-static void parse_line_table(uint16_t **line_refs, struct ModuleFilename **filenames, uint8_t *data, size_t len);
+static void module_parse_line_table(Module *mod, const uint8_t *data, size_t len);
 
 #define IMPL_CODE_LOADER 1
 #include "opcodesswitch.h"
@@ -281,7 +281,7 @@ Module *module_new_from_iff_binary(GlobalContext *global, const void *iff_binary
         return NULL;
     }
 
-    parse_line_table(&mod->line_refs, &mod->filenames, beam_file + offsets[LINT] + 8, sizes[LINT]);
+    module_parse_line_table(mod, beam_file + offsets[LINT] + 8, sizes[LINT]);
     list_init(&mod->line_ref_offsets);
 
     if (offsets[LITT]) {
@@ -437,117 +437,187 @@ const struct ExportedFunction *module_resolve_function0(Module *mod, int import_
     }
 }
 
-static uint16_t *parse_line_refs(uint8_t **data, size_t num_refs, size_t len)
+static bool module_check_line_refs(Module *mod, const uint8_t **data, size_t len)
 {
-    uint16_t *ref_table = malloc((num_refs + 1) * sizeof(uint16_t));
-    if (IS_NULL_PTR(ref_table)) {
-        return NULL;
-    }
-
     // assert pos >= *data
-    uint8_t *pos = *data;
-    for (size_t i = 0; i < num_refs + 1; ++i) {
+    const uint8_t *pos = *data;
+    size_t i = 0;
+    while (i < mod->line_refs_count) {
         if ((size_t) (pos - *data) > len) {
             fprintf(stderr, "Invalid line_ref: expected tag.\n");
-            free(ref_table);
-            return NULL;
+            return false;
         }
         uint8_t tag = *pos;
         switch (tag & 0x0F) {
             case TAG_COMPACT_INT: {
-                uint16_t line_idx = ((tag & 0xF0) >> 4);
-                ref_table[i] = line_idx;
-                ++pos;
-                break;
-            }
-            case TAG_COMPACT_ATOM: {
-                uint16_t line_idx = ((tag & 0xF0) >> 4);
-                ref_table[i] = line_idx;
+                ++i;
                 ++pos;
                 break;
             }
             case TAG_EXTENDED_INT: {
-                uint16_t high_order_3_bits = (tag & 0xE0);
                 ++pos;
                 if ((size_t) (pos - *data) > len) {
                     fprintf(stderr, "Invalid line_ref: expected extended int.\n");
-                    free(ref_table);
-                    return NULL;
+                    return false;
+                }
+                ++i;
+                ++pos;
+                break;
+            }
+            case TAG_COMPACT_ATOM: {
+                uint16_t location_ix = ((tag & 0xF0) >> 4);
+                if (location_ix > mod->locations_count) {
+                    fprintf(stderr, "Invalid line_ref: location_ix = %d is greater than locations_count = %d.\n", (int) location_ix, (int) mod->locations_count);
+                    return false;
                 }
-                uint8_t next_byte = *pos;
-                uint16_t line_idx = ((high_order_3_bits << 3) | next_byte);
                 ++pos;
-                ref_table[i] = line_idx;
                 break;
             }
             case TAG_EXTENDED_ATOM: {
-                uint16_t file_idx = ((tag & 0xF0) >> 4);
+                uint16_t high_order_3_bits = (tag & 0xE0);
                 ++pos;
                 if ((size_t) (pos - *data) > len) {
-                    fprintf(stderr, "Invalid line_ref: expected extended atom.\n");
-                    free(ref_table);
-                    return NULL;
+                    fprintf(stderr, "Invalid line_ref: expected extended int.\n");
+                    return false;
                 }
                 uint8_t next_byte = *pos;
-                uint16_t line_idx = ((next_byte & 0xF0) >> 4);
+                uint16_t location_ix = ((high_order_3_bits << 3) | next_byte);
+                if (location_ix > mod->locations_count) {
+                    fprintf(stderr, "Invalid line_ref: location_ix = %d is greater than locations_count = %d.\n", (int) location_ix, (int) mod->locations_count);
+                    return false;
+                }
                 ++pos;
-                ref_table[file_idx - 1] = line_idx;
                 break;
             }
             default:
                 // TODO handle integer compact encodings > 2048
                 fprintf(stderr, "Unsupported line_ref tag: %u\n", tag);
-                free(ref_table);
-                return NULL;
+                return false;
         }
     }
 
     *data = pos;
-    return ref_table;
+    return true;
 }
 
-struct ModuleFilename *parse_filename_table(uint8_t **data, size_t num_filenames, size_t len)
+static bool module_check_locations(Module *mod, const uint8_t *data, size_t len)
 {
-    struct ModuleFilename *filenames = malloc(num_filenames * sizeof(struct ModuleFilename));
-    if (IS_NULL_PTR(filenames)) {
-        return NULL;
-    }
-
-    // assert pos >= *data
-    uint8_t *pos = *data;
-    for (size_t i = 0; i < num_filenames; ++i) {
-        if ((size_t) ((pos + 2) - *data) > len) {
+    const uint8_t *pos = data;
+    for (size_t i = 1; i <= mod->locations_count; i++) {
+        if ((size_t) ((pos + 2) - data) > len) {
             fprintf(stderr, "Invalid filename: expected 16-bit size.\n");
-            free(filenames);
-            return NULL;
+            return false;
         }
         uint16_t size = READ_16_UNALIGNED(pos);
-        pos +=2;
-        if ((size_t) ((pos + size) - *data) > len) {
+        pos += 2;
+        if ((size_t) ((pos + size) - data) > len) {
             fprintf(stderr, "Invalid filename: expected filename data (%u bytes).\n", size);
-            free(filenames);
-            return NULL;
+            return false;
         }
-        filenames[i].len = size;
-        filenames[i].data = pos;
         pos += size;
     }
 
-    *data = pos;
-    return filenames;
+    return true;
+}
+
+static bool module_get_line_ref(Module *mod, uint16_t line_ref, uint16_t *out_line, uint16_t *out_location)
+{
+    // First is undefined
+    if (line_ref == 0) {
+        *out_line = 0;
+        *out_location = 0;
+        return true;
+    }
+
+    const uint8_t *pos = mod->line_refs_table;
+    uint16_t location_ix = 0;
+    size_t i = 1;
+    while (i <= mod->line_refs_count) {
+        uint8_t tag = *pos;
+        switch (tag & 0x0F) {
+            case TAG_COMPACT_INT: {
+                if (i == line_ref) {
+                    uint16_t line_idx = ((tag & 0xF0) >> 4);
+                    *out_line = line_idx;
+                    *out_location = location_ix;
+                    return true;
+                }
+                ++i;
+                ++pos;
+                break;
+            }
+            case TAG_EXTENDED_INT: {
+                if (i == line_ref) {
+                    uint16_t high_order_3_bits = (tag & 0xE0);
+                    uint16_t line_idx = ((high_order_3_bits << 3) | pos[1]);
+                    *out_line = line_idx;
+                    *out_location = location_ix;
+                    return true;
+                }
+                pos += 2;
+                ++i;
+                break;
+            }
+            case TAG_COMPACT_ATOM: {
+                location_ix = ((tag & 0xF0) >> 4);
+                ++pos;
+                break;
+            }
+            case TAG_EXTENDED_ATOM: {
+                uint16_t high_order_3_bits = (tag & 0xE0);
+                location_ix = ((high_order_3_bits << 3) | pos[1]);
+                pos += 2;
+                break;
+            }
+            default:
+                // TODO handle integer compact encodings > 2048
+                return false;
+        }
+    }
+
+    return false;
 }
 
-static void parse_line_table(uint16_t **line_refs, struct ModuleFilename **filenames, uint8_t *data, size_t len)
+
+static bool module_get_location(Module *mod, uint16_t location_ix, size_t *filename_len, const uint8_t **filename)
 {
+    // 0 is module.erl
+    if (location_ix == 0) {
+        *filename_len = 0;
+        if (filename) {
+            *filename = NULL;
+        }
+        return true;
+    }
+
+    const uint8_t *pos = mod->locations_table;
+    for (size_t i = 1; i <= mod->locations_count; i++) {
+        uint16_t size = READ_16_UNALIGNED(pos);
+        pos +=2;
+        if (i == location_ix) {
+            *filename_len = size;
+            if (filename) {
+                *filename = pos;
+            }
+            return true;
+        }
+        pos += size;
+    }
 
-    *line_refs = NULL;
-    *filenames = NULL;
+    return false;
+}
 
+static void module_parse_line_table(Module *mod, const uint8_t *data, size_t len)
+{
     if (len == 0) {
+        mod->line_refs_count = 0;
+        mod->line_refs_table = NULL;
+        mod->locations_count = 0;
+        mod->locations_table = NULL;
         return;
     }
 
-    uint8_t *pos = data;
+    const uint8_t *pos = data;
 
     CHECK_FREE_SPACE(4, "Error reading Line chunk: version\n");
     uint32_t version = READ_32_UNALIGNED(pos);
@@ -568,28 +638,36 @@ static void parse_line_table(uint16_t **line_refs, struct ModuleFilename **filen
     pos += 4;
 
     CHECK_FREE_SPACE(4, "Error reading Line chunk: num_refs\n");
-    uint32_t num_refs = READ_32_UNALIGNED(pos);
+    mod->line_refs_count = READ_32_UNALIGNED(pos);
     pos += 4;
 
     CHECK_FREE_SPACE(4, "Error reading Line chunk: num_filenames\n");
-    uint32_t num_filenames = READ_32_UNALIGNED(pos);
+    mod->locations_count = READ_32_UNALIGNED(pos);
     pos += 4;
 
-    *line_refs = parse_line_refs(&pos, num_refs, len - (pos - data));
-    if (IS_NULL_PTR(*line_refs)) {
+    mod->line_refs_table = pos;
+
+    if (UNLIKELY(!module_check_line_refs(mod, &pos, len - (pos - data)))) {
+        mod->line_refs_count = 0;
+        mod->line_refs_table = NULL;
+        mod->locations_count = 0;
+        mod->locations_table = NULL;
         return;
     }
 
-    *filenames = parse_filename_table(&pos, num_filenames, len - (pos - data));
-    if (IS_NULL_PTR(*filenames)) {
-        free(*line_refs);
-        return;
+    mod->locations_table = pos;
+
+    if (UNLIKELY(!module_check_locations(mod, pos, len - (pos - data)))) {
+        mod->line_refs_count = 0;
+        mod->line_refs_table = NULL;
+        mod->locations_count = 0;
+        mod->locations_table = NULL;
     }
 }
 
 void module_insert_line_ref_offset(Module *mod, int line_ref, int offset)
 {
-    if (IS_NULL_PTR(mod->line_refs) || line_ref == 0) {
+    if (IS_NULL_PTR(mod->line_refs_table) || line_ref == 0) {
         return;
     }
     struct LineRefOffset *ref_offset = malloc(sizeof(struct LineRefOffset));
@@ -602,7 +680,16 @@ void module_insert_line_ref_offset(Module *mod, int line_ref, int offset)
     list_append(&mod->line_ref_offsets, &ref_offset->head);
 }
 
-int module_find_line(Module *mod, unsigned int offset)
+static bool module_find_line_ref(Module *mod, uint16_t line_ref, uint16_t *line, size_t *filename_len, const uint8_t **filename)
+{
+    uint16_t location_ix;
+    if (UNLIKELY(!module_get_line_ref(mod, line_ref, line, &location_ix))) {
+        return false;
+    }
+    return module_get_location(mod, location_ix, filename_len, filename);
+}
+
+bool module_find_line(Module *mod, unsigned int offset, uint16_t *line, size_t *filename_len, const uint8_t **filename)
 {
     int i = 0;
     struct LineRefOffset *head = GET_LIST_ENTRY(&mod->line_ref_offsets, struct LineRefOffset, head);
@@ -611,25 +698,23 @@ int module_find_line(Module *mod, unsigned int offset)
         struct LineRefOffset *ref_offset = GET_LIST_ENTRY(item, struct LineRefOffset, head);
 
         if (offset == ref_offset->offset) {
-            return mod->line_refs[ref_offset->line_ref];
+            return module_find_line_ref(mod, ref_offset->line_ref, line, filename_len, filename);
         } else if (i == 0 && offset < ref_offset->offset) {
-            return -1;
+            return false;
         } else {
 
             struct LineRefOffset *prev_ref_offset = GET_LIST_ENTRY(ref_offset->head.prev, struct LineRefOffset, head);
             if (prev_ref_offset->offset <= offset && offset < ref_offset->offset) {
-                return mod->line_refs[prev_ref_offset->line_ref];
+                return module_find_line_ref(mod, prev_ref_offset->line_ref, line, filename_len, filename);
             }
 
             struct LineRefOffset *next_ref_offset = GET_LIST_ENTRY(ref_offset->head.next, struct LineRefOffset, head);
             if (next_ref_offset == head && ref_offset->offset <= offset) {
-                return mod->line_refs[ref_offset->line_ref];
+                return module_find_line_ref(mod, ref_offset->line_ref, line, filename_len, filename);
             }
         }
 
         ++i;
     }
-    // should never occur, but return is needed to squelch compiler warnings
-    AVM_ABORT();
-    return -1;
+    return false;
 }