Merge pull request #1557 from pguyot/w10/add-support-for-large-line-numbers

Add support for large line numbers in stack traces (up to 2147483647)

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

Author: bettio

src/libAtomVM/module.c

@@ -43,13 +43,6 @@
 #define LITT_UNCOMPRESSED_SIZE_OFFSET 8
 #define LITT_HEADER_SIZE 12
 
-// TODO Constants similar to these are defined in opcodesswitch.h and should
-// be refactored so they can be used here, as well.
-#define TAG_COMPACT_INT 0x01
-#define TAG_COMPACT_ATOM 0x02
-#define TAG_EXTENDED_INT 0x09
-#define TAG_EXTENDED_ATOM 0x0A
-
 #define CHECK_FREE_SPACE(space, error)           \
     if ((size_t) ((pos + space) - data) > len) { \
         fprintf(stderr, error);                  \
@@ -449,22 +442,39 @@ static bool module_check_line_refs(Module *mod, const uint8_t **data, size_t len
         }
         uint8_t tag = *pos;
         switch (tag & 0x0F) {
-            case TAG_COMPACT_INT: {
+            case COMPACT_INTEGER: {
                 ++i;
                 ++pos;
                 break;
             }
-            case TAG_EXTENDED_INT: {
+            case COMPACT_LARGE_INTEGER: {
                 ++pos;
+                switch (tag & COMPACT_LARGE_IMM_MASK) {
+                    case COMPACT_11BITS_VALUE: {
+                        ++pos;
+                        break;
+                    }
+                    case COMPACT_NBITS_VALUE: {
+                        int sz = (tag >> 5) + 2;
+                        if (UNLIKELY(sz > 4)) {
+                            fprintf(stderr, "Invalid line_ref: expected extended int with sz <= 4 (line number <= 2^31)");
+                            return false;
+                        }
+                        pos += sz;
+                        break;
+                    }
+                    default:
+                        fprintf(stderr, "Invalid line_ref: expected extended int -- tag = %u", (unsigned int) tag);
+                        return false;
+                }
                 if ((size_t) (pos - *data) > len) {
                     fprintf(stderr, "Invalid line_ref: expected extended int.\n");
                     return false;
                 }
                 ++i;
-                ++pos;
                 break;
             }
-            case TAG_COMPACT_ATOM: {
+            case 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);
@@ -473,11 +483,16 @@ static bool module_check_line_refs(Module *mod, const uint8_t **data, size_t len
                 ++pos;
                 break;
             }
-            case TAG_EXTENDED_ATOM: {
+            case COMPACT_LARGE_ATOM: {
+                // We don't support more than 11bits (2048) locations.
+                if (UNLIKELY((tag & COMPACT_LARGE_IMM_MASK) != COMPACT_11BITS_VALUE)) {
+                    fprintf(stderr, "Invalid line_ref: location_ix is larger than 2048.\n");
+                    return false;
+                }
                 uint16_t high_order_3_bits = (tag & 0xE0);
                 ++pos;
                 if ((size_t) (pos - *data) > len) {
-                    fprintf(stderr, "Invalid line_ref: expected extended int.\n");
+                    fprintf(stderr, "Invalid line_ref: expected extended atom.\n");
                     return false;
                 }
                 uint8_t next_byte = *pos;
@@ -490,7 +505,6 @@ static bool module_check_line_refs(Module *mod, const uint8_t **data, size_t len
                 break;
             }
             default:
-                // TODO handle integer compact encodings > 2048
                 fprintf(stderr, "Unsupported line_ref tag: %u\n", tag);
                 return false;
         }
@@ -520,7 +534,7 @@ static bool module_check_locations(Module *mod, const uint8_t *data, size_t len)
     return true;
 }
 
-static bool module_get_line_ref(Module *mod, uint16_t line_ref, uint16_t *out_line, uint16_t *out_location)
+static bool module_get_line_ref(Module *mod, uint16_t line_ref, uint32_t *out_line, uint16_t *out_location)
 {
     // First is undefined
     if (line_ref == 0) {
@@ -535,9 +549,9 @@ static bool module_get_line_ref(Module *mod, uint16_t line_ref, uint16_t *out_li
     while (i <= mod->line_refs_count) {
         uint8_t tag = *pos;
         switch (tag & 0x0F) {
-            case TAG_COMPACT_INT: {
+            case COMPACT_INTEGER: {
                 if (i == line_ref) {
-                    uint16_t line_idx = ((tag & 0xF0) >> 4);
+                    uint32_t line_idx = ((tag & 0xF0) >> 4);
                     *out_line = line_idx;
                     *out_location = location_ix;
                     return true;
@@ -546,32 +560,49 @@ static bool module_get_line_ref(Module *mod, uint16_t line_ref, uint16_t *out_li
                 ++pos;
                 break;
             }
-            case TAG_EXTENDED_INT: {
+            case COMPACT_LARGE_INTEGER: {
+                uint32_t line_idx;
+                switch (tag & COMPACT_LARGE_IMM_MASK) {
+                    case COMPACT_11BITS_VALUE: {
+                        uint16_t high_order_3_bits = (tag & 0xE0);
+                        line_idx = ((high_order_3_bits << 3) | pos[1]);
+                        pos += 2;
+                        break;
+                    }
+                    case COMPACT_NBITS_VALUE: {
+                        pos++;
+                        int sz = (tag >> 5) + 2;
+                        line_idx = 0;
+                        for (int i = 0; i < sz; i++) {
+                            line_idx = line_idx * 256 + pos[i];
+                        }
+                        pos += sz;
+                        break;
+                    }
+                    default:
+                        UNREACHABLE();
+                }
                 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: {
+            case COMPACT_ATOM: {
                 location_ix = ((tag & 0xF0) >> 4);
                 ++pos;
                 break;
             }
-            case TAG_EXTENDED_ATOM: {
+            case COMPACT_LARGE_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;
+                UNREACHABLE();
         }
     }
 
@@ -680,7 +711,7 @@ void module_insert_line_ref_offset(Module *mod, int line_ref, int offset)
     list_append(&mod->line_ref_offsets, &ref_offset->head);
 }
 
-static bool module_find_line_ref(Module *mod, uint16_t line_ref, uint16_t *line, size_t *filename_len, const uint8_t **filename)
+static bool module_find_line_ref(Module *mod, uint16_t line_ref, uint32_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))) {
@@ -689,7 +720,7 @@ static bool module_find_line_ref(Module *mod, uint16_t line_ref, uint16_t *line,
     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)
+bool module_find_line(Module *mod, unsigned int offset, uint32_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);

src/libAtomVM/module.h

@@ -410,7 +410,7 @@ void module_insert_line_ref_offset(Module *mod, int line_ref, int offset);
  * @param filename on output the filename or NULL if it's module.erl. Can be NULL.
  * @return \c true if the line was found
  */
-bool module_find_line(Module *mod, unsigned int offset, uint16_t *line, size_t *filename_len, const uint8_t **filename);
+bool module_find_line(Module *mod, unsigned int offset, uint32_t *line, size_t *filename_len, const uint8_t **filename);
 
 /**
  * @return true if the module has line information, false, otherwise.

src/libAtomVM/stacktrace.c

@@ -139,7 +139,7 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
                 prev_mod = cp_mod;
                 prev_mod_offset = mod_offset;
                 if (module_has_line_chunk(cp_mod)) {
-                    uint16_t line;
+                    uint32_t line;
                     const uint8_t *filename;
                     size_t filename_len;
                     if (LIKELY(module_find_line(cp_mod, (unsigned int) mod_offset, &line, &filename_len, &filename))) {
@@ -163,7 +163,7 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
                 prev_mod = cl_mod;
                 prev_mod_offset = mod_offset;
                 if (module_has_line_chunk(cl_mod)) {
-                    uint16_t line;
+                    uint32_t line;
                     const uint8_t *filename;
                     size_t filename_len;
                     if (LIKELY(module_find_line(cl_mod, (unsigned int) mod_offset, &line, &filename_len, &filename))) {
@@ -180,7 +180,7 @@ term stacktrace_create_raw(Context *ctx, Module *mod, int current_offset, term e
 
     num_frames++;
     if (module_has_line_chunk(mod)) {
-        uint16_t line;
+        uint32_t line;
         const uint8_t *filename;
         size_t filename_len;
         if (LIKELY(module_find_line(mod, (unsigned int) current_offset, &line, &filename_len, &filename))) {
@@ -342,7 +342,7 @@ term stacktrace_build(Context *ctx, term *stack_info, uint32_t live)
 
         term aux_data = term_nil();
         if (module_has_line_chunk(cp_mod)) {
-            uint16_t line;
+            uint32_t line;
             const uint8_t *filename;
             size_t filename_len;
             if (LIKELY(module_find_line(cp_mod, (unsigned int) mod_offset, &line, &filename_len, &filename))) {

tests/erlang_tests/test_stacktrace.erl

@@ -20,6 +20,11 @@
 -module(test_stacktrace).
 
 -export([start/0, maybe_crash/1]).
+-export([
+    throw_with_file_and_line/0,
+    throw_with_other_file_and_line/0,
+    throw_with_other_file_and_line_large_value/0
+]).
 
 -include("test_stacktrace.hrl").
 
@@ -36,6 +41,7 @@ start() ->
     ok = test_catch(),
     ok = maybe_test_filelineno(),
     ok = maybe_test_filelineno_other_file(),
+    ok = maybe_test_filelineno_large(),
     0.
 
 test_local_throw() ->
@@ -233,15 +239,15 @@ test_catch() ->
     do_some_stuff(Result),
     Result.
 
-maybe_test_filelineno() ->
+maybe_test_filelineno(Fun) ->
     ok =
         try
-            throw_with_file_and_line(),
+            ?MODULE:Fun(),
             fail
         catch
             throw:{File, Line}:Stacktrace ->
                 [Frame | _] = Stacktrace,
-                {?MODULE, throw_with_file_and_line, 0, AuxData} = Frame,
+                {?MODULE, Fun, 0, AuxData} = Frame,
                 case {get_value(file, AuxData), get_value(line, AuxData)} of
                     {undefined, undefined} ->
                         ok;
@@ -262,34 +268,16 @@ maybe_test_filelineno() ->
                 end
         end.
 
+maybe_test_filelineno() ->
+    maybe_test_filelineno(throw_with_file_and_line).
+
 maybe_test_filelineno_other_file() ->
-    ok =
-        try
-            throw_with_other_file_and_line(),
-            fail
-        catch
-            throw:{File, Line}:Stacktrace ->
-                [Frame | _] = Stacktrace,
-                {?MODULE, throw_with_other_file_and_line, 0, AuxData} = Frame,
-                case {get_value(file, AuxData), get_value(line, AuxData)} of
-                    {undefined, undefined} ->
-                        ok;
-                    {F, L} ->
-                        Ef =
-                            case is_binary(F) of
-                                true ->
-                                    erlang:binary_to_list(F);
-                                _ ->
-                                    F
-                            end,
-                        case File == Ef andalso Line == L of
-                            true ->
-                                ok;
-                            _ ->
-                                {unexpected_file_line, F, L}
-                        end
-                end
-        end.
+    maybe_test_filelineno(throw_with_other_file_and_line).
+
+% This test actually succeeds even if large line numbers are not supported
+% because all line numbers are then disabled.
+maybe_test_filelineno_large() ->
+    maybe_test_filelineno(throw_with_other_file_and_line_large_value).
 
 get_value(_Key, []) ->
     undefined;