From 79fdfa9e76d846f7d5b37a04fb708f3f1235cd8a Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Wed, 28 Feb 2024 19:12:50 +0000 Subject: [PATCH] Missing static bounds checks for some index constraints --- src/bounds.c | 78 ++++++++++++++++++++++++++++++++++--------- src/common.c | 5 +++ src/common.h | 1 + src/dump.c | 5 +-- src/parse.c | 26 ++++++++++++--- src/rt/wave.c | 3 +- src/vhpi/vhpi-model.c | 4 +-- test/bounds/cons1.vhd | 16 +++++++++ test/test_bounds.c | 24 ++++++++++++- 9 files changed, 135 insertions(+), 27 deletions(-) create mode 100644 test/bounds/cons1.vhd diff --git a/src/bounds.c b/src/bounds.c index 11e8be9b..d0c8c1ab 100644 --- a/src/bounds.c +++ b/src/bounds.c @@ -1,5 +1,5 @@ // -// Copyright (C) 2011-2023 Nick Gasson +// Copyright (C) 2011-2024 Nick Gasson // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -501,7 +501,7 @@ static void bounds_check_missing_choices(tree_t t, type_t type, type_pp(index_type)); type_t base = index_type ?: type; - while (type_kind(base) == T_SUBTYPE && !type_has_ident(base)) + while (is_anonymous_subtype(base)) base = type_base(base); int64_t rlow, rhigh; @@ -706,22 +706,24 @@ static void bounds_check_aggregate(tree_t t) } } -static void bounds_check_subtype(type_t type) +static void bounds_check_index_contraints(type_t type) { - if (type_kind(type) != T_SUBTYPE || type_has_ident(type)) - return; // Not an anonymous subtype - else if (!type_is_array(type) || type_is_unconstrained(type)) - return; // Not a fully constrained array + assert(type_kind(type) == T_SUBTYPE); - // Check folded range does not violate index constraints of base type + if (type_constraints(type) == 0) + return; - type_t base = type_base(type); + tree_t c0 = type_constraint(type, 0); + if (tree_subkind(c0) != C_INDEX) + return; - const int ndims = dimension_of(base); + // Check folded range does not violate index constraints of base type + + const int ndims = dimension_of(type); for (int i = 0; i < ndims; i++) { - tree_t dim = range_of(type, i); + tree_t dim = tree_range(c0, i); - type_t cons = index_type_of(base, i); + type_t cons = index_type_of(type, i); if (type_kind(cons) == T_GENERIC) continue; // Cannot check @@ -790,7 +792,9 @@ static void bounds_check_object_decl(tree_t t) if (tree_has_value(t)) bounds_check_assignment(t, tree_value(t)); - bounds_check_subtype(tree_type(t)); + type_t type = tree_type(t); + if (is_anonymous_subtype(type)) + bounds_check_index_contraints(type); } static void bounds_check_alias_decl(tree_t t) @@ -801,7 +805,40 @@ static void bounds_check_alias_decl(tree_t t) if (tree_has_value(t)) bounds_check_assignment(t, tree_value(t)); - bounds_check_subtype(tree_type(t)); + type_t type = tree_type(t); + if (is_anonymous_subtype(type)) + bounds_check_index_contraints(type); +} + +static void bounds_check_elem_constraint(tree_t t) +{ + type_t type = tree_type(t); + if (is_anonymous_subtype(type)) + bounds_check_index_contraints(type); +} + +static void bounds_check_type_decl(tree_t t) +{ + type_t type = tree_type(t); + + if (type_is_record(type)) { + const int nfields = type_fields(type); + for (int i = 0; i < nfields; i++) { + type_t ft = tree_type(type_field(type, i)); + if (is_anonymous_subtype(ft)) + bounds_check_index_contraints(ft); + } + } + else if (type_is_array(type)) { + type_t elem = type_elem(type); + if (is_anonymous_subtype(elem)) + bounds_check_index_contraints(elem); + } +} + +static void bounds_check_subtype_decl(tree_t t) +{ + bounds_check_index_contraints(tree_type(t)); } static void bounds_check_interface_decl(tree_t t) @@ -814,7 +851,9 @@ static void bounds_check_interface_decl(tree_t t) if (mode != PORT_ARRAY_VIEW && mode != PORT_RECORD_VIEW && tree_has_value(t)) bounds_check_assignment(t, tree_value(t)); - bounds_check_subtype(tree_type(t)); + type_t type = tree_type(t); + if (is_anonymous_subtype(type)) + bounds_check_index_contraints(type); } static char *bounds_get_hint_str(tree_t where) @@ -1382,6 +1421,15 @@ static tree_t bounds_visit_fn(tree_t t, void *context) case T_BLOCK: bounds_check_block(t); break; + case T_ELEM_CONSTRAINT: + bounds_check_elem_constraint(t); + break; + case T_TYPE_DECL: + bounds_check_type_decl(t); + break; + case T_SUBTYPE_DECL: + bounds_check_subtype_decl(t); + break; default: break; } diff --git a/src/common.c b/src/common.c index db5426af..42f478de 100644 --- a/src/common.c +++ b/src/common.c @@ -1103,6 +1103,11 @@ bool is_uninstantiated_subprogram(tree_t decl) } } +bool is_anonymous_subtype(type_t type) +{ + return type_kind(type) == T_SUBTYPE && !type_has_ident(type); +} + bool unit_needs_cgen(tree_t unit) { switch (tree_kind(unit)) { diff --git a/src/common.h b/src/common.h index d595f32d..fa74f31e 100644 --- a/src/common.h +++ b/src/common.h @@ -66,6 +66,7 @@ bool is_design_unit(tree_t t); bool is_literal(tree_t t); bool is_uninstantiated_package(tree_t pack); bool is_uninstantiated_subprogram(tree_t decl); +bool is_anonymous_subtype(type_t type); tree_t search_decls(tree_t container, ident_t name, int nth); bool is_open_coded_builtin(subprogram_kind_t kind); bool attribute_has_param(attr_kind_t attr); diff --git a/src/dump.c b/src/dump.c index ba87df85..74fc6a48 100644 --- a/src/dump.c +++ b/src/dump.c @@ -467,7 +467,7 @@ static void dump_elem_constraints(type_t type) { if (type_is_array(type) && type_has_elem(type)) { type_t elem = type_elem(type); - if (type_kind(elem) == T_SUBTYPE && !type_has_ident(elem)) { + if (is_anonymous_subtype(elem)) { // Anonymous subtype created for element constraints assert(type_constraints(elem) == 1); dump_constraint(type_constraint(elem, 0)); @@ -478,7 +478,7 @@ static void dump_elem_constraints(type_t type) static void dump_type(type_t type) { - if (type_kind(type) == T_SUBTYPE && !type_has_ident(type)) { + if (is_anonymous_subtype(type)) { // Anonymous subtype print_syntax("%s", type_pp(type)); if (type_ident(type) == type_ident(type_base(type))) { @@ -1698,6 +1698,7 @@ void vhdl_dump(tree_t t, int indent) break; case T_PORT_DECL: case T_GENERIC_DECL: + case T_PARAM_DECL: dump_port(t, indent); break; case T_RANGE: diff --git a/src/parse.c b/src/parse.c index 56df5126..1e8414df 100644 --- a/src/parse.c +++ b/src/parse.c @@ -889,6 +889,20 @@ static bool is_bit_or_std_ulogic(type_t type) return name == well_known(W_STD_BIT) || name == well_known(W_IEEE_ULOGIC); } +static type_t get_element_subtype(type_t type) +{ + type_t elem = type_elem(type); + if (is_anonymous_subtype(elem)) { + // Create a distinct subtype to ensure any errors are only + // reported once + type_t sub = type_new(T_SUBTYPE); + type_set_base(sub, elem); + return sub; + } + else + return elem; +} + static void declare_predefined_ops(tree_t container, type_t t) { // Prefined operators are defined in LRM 93 section 7.2 @@ -930,10 +944,12 @@ static void declare_predefined_ops(tree_t container, type_t t) // Operators on arrays declare_binary(container, eq, t, t, std_bool, S_ARRAY_EQ); declare_binary(container, neq, t, t, std_bool, S_ARRAY_NEQ); + if (dimension_of(t) == 1) { - type_t elem = type_elem(t); - const bool ordered = (standard() >= STD_19) - ? type_is_scalar(elem) : type_is_discrete(elem); + type_t elem = get_element_subtype(t); + const bool scalar_elem = type_is_scalar(elem); + const bool ordered = + standard() >= STD_19 ? scalar_elem : type_is_discrete(elem); if (ordered) { declare_binary(container, cmp_lt, t, t, std_bool, S_ARRAY_LT); @@ -954,7 +970,7 @@ static void declare_predefined_ops(tree_t container, type_t t) declare_binary(container, max_i, t, t, t, S_MAXIMUM); } - if (type_is_scalar(elem)) { + if (scalar_elem) { declare_unary(container, min_i, t, elem, S_MINIMUM); declare_unary(container, max_i, t, elem, S_MAXIMUM); } @@ -2031,7 +2047,7 @@ static type_t apply_element_attribute(tree_t aref) return type_new(T_NONE); } - return type_elem(type); + return get_element_subtype(type); } static type_t apply_designated_subtype_attribute(tree_t aref) diff --git a/src/rt/wave.c b/src/rt/wave.c index 6483db77..aefe11de 100644 --- a/src/rt/wave.c +++ b/src/rt/wave.c @@ -250,8 +250,7 @@ static fst_unit_t *fst_make_unit_map(type_t type) static fst_type_t *fst_type_for(type_t type, const loc_t *loc) { - const type_kind_t kind = type_kind(type); - if (kind == T_SUBTYPE && !type_has_ident(type)) { + if (is_anonymous_subtype(type)) { // Do not cache anonymous subtypes return fst_type_for(type_base(type), loc); } diff --git a/src/vhpi/vhpi-model.c b/src/vhpi/vhpi-model.c index 85e52a95..763cc9f5 100644 --- a/src/vhpi/vhpi-model.c +++ b/src/vhpi/vhpi-model.c @@ -3066,7 +3066,7 @@ static c_typeDecl *build_arrayTypeDecl(type_t type, tree_t decl, td->composite.typeDecl.wrapped = !type_const_bounds(type); type_t elem = type_elem(type); - if (type_is_array(elem) && !type_has_ident(elem)) { + if (type_is_array(elem) && is_anonymous_subtype(elem)) { // Anonymous subtype may need to access parent type to // get non-constant bounds td->ElemType = build_arrayTypeDecl(elem, decl, parent, obj, @@ -3167,7 +3167,7 @@ static c_typeDecl *build_dynamicSubtype(c_typeDecl *base, void *ptr, static c_typeDecl *build_typeDecl(type_t type, c_vhpiObject *obj) { type_t base = type; - while (type_kind(base) == T_SUBTYPE && !type_has_ident(base)) + while (is_anonymous_subtype(base)) base = type_base(base); // Anonymous subtypes have no declaration ident_t id = type_ident(base); diff --git a/test/bounds/cons1.vhd b/test/bounds/cons1.vhd new file mode 100644 index 00000000..1186eebd --- /dev/null +++ b/test/bounds/cons1.vhd @@ -0,0 +1,16 @@ +package cons1 is + type t_rec1 is record + f1 : string; + f2 : string(-1 to 0); -- Error + end record; + + constant c1 : t_rec1(f1(0 to 4)) := (f1 => "hello", f2 => "12"); -- Error + + subtype t_string_sub is string(0 to 5); -- Error + subtype t_sub_sub is t_string_sub; -- OK + + type t_array is array (natural range <>) of string(0 to 3); -- Error + + constant c2 : t_array'element; -- OK + constant c3 : t_array'element; -- OK +end package; diff --git a/test/test_bounds.c b/test/test_bounds.c index c190670a..b85dd71d 100644 --- a/test/test_bounds.c +++ b/test/test_bounds.c @@ -1,5 +1,5 @@ // -// Copyright (C) 2013-2023 Nick Gasson +// Copyright (C) 2013-2024 Nick Gasson // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -703,6 +703,27 @@ START_TEST(test_issue819) } END_TEST +START_TEST(test_cons1) +{ + set_standard(STD_08); + + input_from_file(TESTDIR "/bounds/cons1.vhd"); + + const error_t expect[] = { + { 4, "left index -1 violates constraint POSITIVE" }, + { 7, "left index 0 violates constraint POSITIVE" }, + { 9, "left index 0 violates constraint POSITIVE" }, + { 12, "left index 0 violates constraint POSITIVE" }, + { -1, NULL } + }; + expect_errors(expect); + + parse_check_and_simplify(T_PACKAGE); + + check_expected_errors(); +} +END_TEST + Suite *get_bounds_tests(void) { Suite *s = suite_create("bounds"); @@ -740,6 +761,7 @@ Suite *get_bounds_tests(void) tcase_add_test(tc_core, test_issue800); tcase_add_test(tc_core, test_issue806); tcase_add_test(tc_core, test_issue819); + tcase_add_test(tc_core, test_cons1); suite_add_tcase(s, tc_core); return s; -- 2.39.2