From 736b638178d605a0ee87e4b7b603c24a8440b38c Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Tue, 12 Mar 2024 22:08:06 +0000 Subject: [PATCH] Code generation crash with nested package instances. Fixes #858 --- src/lower.c | 91 +++++++++++++++++---- src/vcode.c | 5 ++ test/regress/issue858.vhd | 167 ++++++++++++++++++++++++++++++++++++++ test/regress/testlist.txt | 1 + 4 files changed, 250 insertions(+), 14 deletions(-) create mode 100644 test/regress/issue858.vhd diff --git a/src/lower.c b/src/lower.c index 26a3e248..7dfb7448 100644 --- a/src/lower.c +++ b/src/lower.c @@ -12870,8 +12870,15 @@ typedef enum { UNIT_FINALISED = 3, } unit_kind_t; +typedef void (*dep_visit_fn_t)(vcode_unit_t, void *); +typedef bool (*dep_filter_fn_t)(ident_t, void *); + typedef struct _unit_registry { - hash_t *map; + hash_t *map; + hset_t *visited; + dep_filter_fn_t filter_fn; + dep_visit_fn_t visit_fn; + void *context; } unit_registry_t; typedef struct { @@ -12964,39 +12971,92 @@ void unit_registry_purge(unit_registry_t *ur, ident_t prefix) } } -static void flush_dependency_cb(ident_t name, void *ctx) +static bool flush_dependency_filter(ident_t name, void *ctx) { unit_registry_t *ur = ctx; void *ptr = hash_get(ur->map, name); - if (pointer_tag(ptr) != UNIT_DEFERRED) - return; - - deferred_unit_t *du = untag_pointer(ptr, deferred_unit_t); + if (ptr == NULL) + return false; - if (du->offset >= 0) - return; // Does not reference objects which can be moved by GC + ident_t module; + ptrdiff_t offset; + switch (pointer_tag(ptr)) { + case UNIT_DEFERRED: + { + deferred_unit_t *du = untag_pointer(ptr, deferred_unit_t); + offset = du->offset; + } + break; + case UNIT_FINALISED: + { + vcode_unit_t vu = untag_pointer(ptr, struct _vcode_unit); + vcode_unit_object(vu, &module, &offset); + } + break; + case UNIT_GENERATED: + { + lower_unit_t *lu = untag_pointer(ptr, lower_unit_t); + vcode_unit_object(lu->vunit, &module, &offset); + } + break; + default: + fatal_trace("invalid tagged pointer %p", ptr); + } - (void)unit_registry_get(ur, name); + return offset < 0; } -void unit_registry_flush(unit_registry_t *ur, ident_t name) +static void walk_dependency_cb(ident_t name, void *ctx) { + unit_registry_t *ur = ctx; + + if (hset_contains(ur->visited, name)) + return; + + hset_insert(ur->visited, name); + + if (ur->filter_fn != NULL && !(*ur->filter_fn)(name, ctx)) + return; + vcode_unit_t vu = unit_registry_get(ur, name); assert(vu != NULL); - // Make sure all transitive dependencies of this unit which contain - // references to non-frozen objects are generated - vcode_walk_dependencies(vu, flush_dependency_cb, ur); + if (ur->visit_fn != NULL) + (*ur->visit_fn)(vu, ur->context); + + vcode_walk_dependencies(vu, walk_dependency_cb, ur); for (vcode_unit_t it = vcode_unit_child(vu); it != NULL; it = vcode_unit_next(it)) { assert(vcode_unit_kind(it) != VCODE_UNIT_THUNK); - unit_registry_flush(ur, vcode_unit_name(it)); + walk_dependency_cb(vcode_unit_name(it), ctx); } } +void unit_registry_walk_dependencies(unit_registry_t *ur, ident_t name, + dep_filter_fn_t filter_fn, + dep_visit_fn_t visit_fn, void *ctx) +{ + assert(ur->visited == NULL); + ur->visited = hset_new(128); + ur->visit_fn = visit_fn; + ur->filter_fn = filter_fn; + + walk_dependency_cb(name, ur); + + hset_free(ur->visited); + ur->visited = NULL; +} + +void unit_registry_flush(unit_registry_t *ur, ident_t name) +{ + // Make sure all transitive dependencies of this unit which contain + // references to non-frozen objects are generated + unit_registry_walk_dependencies(ur, name, flush_dependency_filter, NULL, ur); +} + void unit_registry_finalise(unit_registry_t *ur, lower_unit_t *lu) { assert(pointer_tag(hash_get(ur->map, lu->name)) == UNIT_GENERATED); @@ -13070,6 +13130,9 @@ vcode_unit_t unit_registry_get(unit_registry_t *ur, ident_t ident) object_t *obj = object_from_locus(du->arena, du->offset, lib_load_handler); + // This assertion fails in unit tests + // assert(du->offset >= 0 || !arena_frozen(object_arena(obj))); + vcode_unit_t context = du->parent ? du->parent->vunit : NULL; vcode_unit_t vu = (*du->emit_fn)(ident, obj, context); tree_t container = tree_from_object(obj); diff --git a/src/vcode.c b/src/vcode.c index 07d4e730..11cb28a0 100644 --- a/src/vcode.c +++ b/src/vcode.c @@ -6029,6 +6029,9 @@ vcode_reg_t emit_instance_name(vcode_reg_t kind) void vcode_walk_dependencies(vcode_unit_t vu, vcode_dep_fn_t fn, void *ctx) { + vcode_state_t state; + vcode_state_save(&state); + vcode_select_unit(vu); const int nblocks = vcode_count_blocks(); @@ -6054,6 +6057,8 @@ void vcode_walk_dependencies(vcode_unit_t vu, vcode_dep_fn_t fn, void *ctx) } } } + + vcode_state_restore(&state); } #ifdef DEBUG diff --git a/test/regress/issue858.vhd b/test/regress/issue858.vhd new file mode 100644 index 00000000..99abc85e --- /dev/null +++ b/test/regress/issue858.vhd @@ -0,0 +1,167 @@ +package generic_queue_pkg is + + generic(type t_generic_element; + GC_QUEUE_COUNT_MAX : natural := 1000; + GC_QUEUE_COUNT_THRESHOLD : natural := 950); + + type t_generic_queue is protected + + procedure add( + constant instance : in integer; + constant element : in t_generic_element); + + procedure add( + constant element : in t_generic_element); + + end protected; + +end package generic_queue_pkg; + +package body generic_queue_pkg is + + type t_generic_queue is protected body + + variable vr_num_elements_in_queue : integer_vector(0 to 1) := (others => 0); + variable vr_queue_count_max : integer_vector(0 to 1) := (others => GC_QUEUE_COUNT_MAX); + + procedure add( + constant instance : in integer; + constant element : in t_generic_element + ) is + begin + assert vr_num_elements_in_queue(instance) < vr_queue_count_max(instance); + end procedure; + + procedure add( + constant element : in t_generic_element + ) is + begin + add(1, element); + end procedure; + + end protected body; + +end package body generic_queue_pkg; + +------------------------------------------------------------------------------- + +package generic_sb_support_pkg is + + type t_sb_config is record + allow_lossy : boolean; + allow_out_of_order : boolean; + overdue_check_time_limit : time; + ignore_initial_garbage : boolean; + end record; + + type t_sb_config_array is array (integer range <>) of t_sb_config; + + constant C_SB_CONFIG_DEFAULT : t_sb_config := (allow_lossy => false, + allow_out_of_order => false, + overdue_check_time_limit => 0 ns, + ignore_initial_garbage => false); + +end package generic_sb_support_pkg; + +package body generic_sb_support_pkg is + +end package body generic_sb_support_pkg; + + +------------------------------------------------------------------------------- + +use work.generic_sb_support_pkg.all; + +package generic_sb_pkg is + + generic(type t_element; + constant sb_config_default : t_sb_config := C_SB_CONFIG_DEFAULT; + constant GC_QUEUE_COUNT_MAX : natural := 1000; + constant GC_QUEUE_COUNT_THRESHOLD : natural := 950); + + type t_generic_sb is protected + + procedure add_expected( + constant instance : in integer; + constant expected_element : in t_element; + constant tag : in string); + + procedure add_expected( + constant expected_element : in t_element; + constant tag : in string); + + end protected t_generic_sb; + +end package generic_sb_pkg; + +package body generic_sb_pkg is + + type t_sb_entry is record + expected_element : t_element; + source : string(1 to 5); + tag : string(1 to 5); + entry_time : time; + end record; + + package sb_queue_pkg is new work.generic_queue_pkg + generic map( + t_generic_element => t_sb_entry, + GC_QUEUE_COUNT_MAX => GC_QUEUE_COUNT_MAX, + GC_QUEUE_COUNT_THRESHOLD => GC_QUEUE_COUNT_THRESHOLD); + + use sb_queue_pkg.all; + + type t_generic_sb is protected body + + variable vr_sb_queue : sb_queue_pkg.t_generic_queue; + + procedure add_expected( + constant instance : in integer; + constant expected_element : in t_element; + constant tag : in string + ) is + variable v_sb_entry : t_sb_entry; + begin + vr_sb_queue.add(0, v_sb_entry); + end procedure add_expected; + + procedure add_expected( + constant expected_element : in t_element; + constant tag : in string + ) is + begin + add_expected(1, expected_element, tag); + end procedure add_expected; + + end protected body; + +end package body generic_sb_pkg; + + +------------------------------------------------------------------------------- + +use work.generic_sb_support_pkg.all; + +entity issue858 is +end entity; + +architecture tb of issue858 is + constant C_SLV_SB_CONFIG_DEFAULT : t_sb_config := (allow_lossy => false, + allow_out_of_order => false, + overdue_check_time_limit => 0 ns, + ignore_initial_garbage => false); + + package slv_sb_pkg is new work.generic_sb_pkg + generic map(t_element => bit_vector(7 downto 0), + sb_config_default => C_SLV_SB_CONFIG_DEFAULT); + + use slv_sb_pkg.all; + + shared variable sb_under_test : slv_sb_pkg.t_generic_sb; +begin + p_main: process + begin + sb_under_test.add_expected(X"01", "tag added"); + wait; + end process p_main; +end architecture; diff --git a/test/regress/testlist.txt b/test/regress/testlist.txt index d25a5c62..d3d40c29 100644 --- a/test/regress/testlist.txt +++ b/test/regress/testlist.txt @@ -957,3 +957,4 @@ issue856 normal,2019 driver22 fail,gold,2019 vhpi12 normal,vhpi issue862 normal,gold,2008,relaxed +issue858 normal,2008 -- 2.39.2