From 8ecc85e788c61b8d8abe43d432f9ae56ae46b9da Mon Sep 17 00:00:00 2001 From: Nick Gasson Date: Mon, 29 Aug 2022 22:43:57 +0100 Subject: [PATCH] Detect duplicate choices in array case statement. Issue #528 --- NEWS.md | 2 ++ src/bounds.c | 31 +++++++++++++++++++++++++++++++ src/common.c | 23 ++++++++++++----------- src/common.h | 1 + src/vcode.c | 5 +++++ test/bounds/case3.vhd | 23 +++++++++++++++++++++++ test/test_bounds.c | 19 +++++++++++++++++++ 7 files changed, 93 insertions(+), 11 deletions(-) create mode 100644 test/bounds/case3.vhd diff --git a/NEWS.md b/NEWS.md index 52900eff..60c76c5c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -27,6 +27,8 @@ - Real valued signals can now be dumped in FST files (#524). - Fixed signal assignment delay with side effects being evaluated twice (#527). +- An error is now reported for duplicate choices in array-type case + statements (#528). ## Version 1.7.0 - 2022-08-07 - *Breaking change:* In-tree builds are no longer supported: use a diff --git a/src/bounds.c b/src/bounds.c index be42674f..9e9bd8bf 100644 --- a/src/bounds.c +++ b/src/bounds.c @@ -1001,6 +1001,23 @@ static void bounds_check_scalar_case(tree_t t, type_t type) bounds_free_intervals(&covered); } +static void bounds_check_duplicate_choice(tree_t old, tree_t new, int length) +{ + for (int i = 0; i < length; i++) { + if (get_case_choice_char(old, i) != get_case_choice_char(new, i)) + return; + } + + diag_t *d = diag_new(DIAG_ERROR, tree_loc(new)); + diag_printf(d, "duplicate choice in case statement"); + diag_hint(d, tree_loc(new), "repeated here"); + diag_hint(d, tree_loc(old), "previous choice for this value"); + diag_hint(d, NULL, "each value of the subtype of the expression must be " + "represented once and only once in the set of choices"); + diag_lrm(d, STD_93, "8.8"); + diag_emit(d); +} + static void bounds_check_array_case(tree_t t, type_t type) { int64_t expect = -1; @@ -1019,8 +1036,11 @@ static void bounds_check_array_case(tree_t t, type_t type) int64_t have = 0, length = -1; const int nassocs = tree_assocs(t); + int64_t *hashes LOCAL = xmalloc_array(nassocs, sizeof(int64_t)); + for (int i = 0; i < nassocs; i++) { tree_t a = tree_assoc(t, i); + hashes[i] = INT64_MAX; switch (tree_subkind(a)) { case A_OTHERS: @@ -1051,6 +1071,17 @@ static void bounds_check_array_case(tree_t t, type_t type) diag_emit(d); return; } + + hashes[i] = encode_case_choice(name, choice_length, 0); + + for (int j = 0; j < i; j++) { + if (hashes[j] == INT64_MAX) + continue; + else if (hashes[i] == hashes[j]) { + tree_t old = tree_name(tree_assoc(t, j)); + bounds_check_duplicate_choice(old, name, choice_length); + } + } } } break; diff --git a/src/common.c b/src/common.c index a72a6a10..d14faf98 100644 --- a/src/common.c +++ b/src/common.c @@ -1440,14 +1440,16 @@ tree_t primary_unit_of(tree_t unit) } } -static unsigned encode_case_choice_at_depth(tree_t value, int depth) +unsigned get_case_choice_char(tree_t value, int depth) { switch (tree_kind(value)) { case T_STRING: - { + if (depth < tree_chars(value)) { tree_t ch = tree_char(value, depth); return tree_pos(tree_ref(ch)); } + else + return ~0; // Out of bounds case T_AGGREGATE: { @@ -1471,17 +1473,18 @@ static unsigned encode_case_choice_at_depth(tree_t value, int depth) return assume_int(tree_value(a)); } } + + // This will produce an error during bounds checking + return ~0; } - break; case T_REF: { tree_t decl = tree_ref(value); assert(tree_kind(decl) == T_CONST_DECL || tree_kind(decl) == T_ALIAS); assert(tree_has_value(decl)); - return encode_case_choice_at_depth(tree_value(decl), depth); + return get_case_choice_char(tree_value(decl), depth); } - break; case T_ARRAY_SLICE: { @@ -1489,7 +1492,7 @@ static unsigned encode_case_choice_at_depth(tree_t value, int depth) tree_t r = tree_range(value, 0); const int64_t rleft = assume_int(tree_left(r)); const int64_t offset = rebase_index(tree_type(base), 0, rleft); - return encode_case_choice_at_depth(base, depth + offset); + return get_case_choice_char(base, depth + offset); } case T_FCALL: @@ -1505,7 +1508,7 @@ static unsigned encode_case_choice_at_depth(tree_t value, int depth) "side of concatenation"); if (depth < left_len || i + 1 == nparams) - return encode_case_choice_at_depth(left, depth); + return get_case_choice_char(left, depth); depth -= left_len; } @@ -1516,8 +1519,6 @@ static unsigned encode_case_choice_at_depth(tree_t value, int depth) fatal_at(tree_loc(value), "unsupported tree type %s in case choice", tree_kind_str(tree_kind(value))); } - - fatal_at(tree_loc(value), "cannot find element %d in choice", depth); } int64_t encode_case_choice(tree_t value, int length, int bits) @@ -1526,11 +1527,11 @@ int64_t encode_case_choice(tree_t value, int length, int bits) for (int i = 0; i < length; i++) { if (bits > 0) { enc <<= bits; - enc |= encode_case_choice_at_depth(value, i); + enc |= get_case_choice_char(value, i); } else { enc *= 0x27d4eb2d; - enc += encode_case_choice_at_depth(value, i); + enc += get_case_choice_char(value, i); } } diff --git a/src/common.h b/src/common.h index 9f8271cc..4e6f0158 100644 --- a/src/common.h +++ b/src/common.h @@ -73,6 +73,7 @@ tree_t name_to_ref(tree_t name); const char *port_mode_str(port_mode_t mode); void mangle_one_type(text_buf_t *buf, type_t type); tree_t primary_unit_of(tree_t unit); +unsigned get_case_choice_char(tree_t value, int depth); int64_t encode_case_choice(tree_t value, int length, int bits); void to_string(text_buf_t *tb, type_t type, int64_t value); tree_t longest_static_prefix(tree_t expr); diff --git a/src/vcode.c b/src/vcode.c index 49cbf52b..031f12a4 100644 --- a/src/vcode.c +++ b/src/vcode.c @@ -4923,6 +4923,11 @@ void emit_case(vcode_reg_t value, vcode_block_t def, const vcode_reg_t *cases, for (int i = 0; i < ncases; i++) { vcode_add_arg(op, cases[i]); vcode_add_target(op, blocks[i]); + +#ifdef DEBUG + for (int j = 0; j < i; j++) + VCODE_ASSERT(cases[i] != cases[j], "duplicate case choice"); +#endif } } diff --git a/test/bounds/case3.vhd b/test/bounds/case3.vhd new file mode 100644 index 00000000..31e25137 --- /dev/null +++ b/test/bounds/case3.vhd @@ -0,0 +1,23 @@ +entity case3 is +end entity; + +architecture test of case3 is + signal s : bit_vector(1 to 3); + signal t : bit; +begin + + p1: process (s) is + begin + case s is + when "100" => + t <= '1'; + when "101" => + t <= '0'; + when "100" => -- Error + t <= '1'; + when others => + null; + end case; + end process; + +end architecture; diff --git a/test/test_bounds.c b/test/test_bounds.c index 97b40e9f..42bf8602 100644 --- a/test/test_bounds.c +++ b/test/test_bounds.c @@ -530,6 +530,24 @@ START_TEST(test_issue477b) } END_TEST +START_TEST(test_case3) +{ + input_from_file(TESTDIR "/bounds/case3.vhd"); + + const error_t expect[] = { + { 16, "duplicate choice in case statement" }, + { -1, NULL } + }; + expect_errors(expect); + + tree_t a = parse_check_and_simplify(T_ENTITY, T_ARCH); + fail_unless(error_count() == 0); + + bounds_check(a); + check_expected_errors(); +} +END_TEST + Suite *get_bounds_tests(void) { Suite *s = suite_create("bounds"); @@ -556,6 +574,7 @@ Suite *get_bounds_tests(void) tcase_add_test(tc_core, test_case2); tcase_add_test(tc_core, test_issue477a); tcase_add_test(tc_core, test_issue477b); + tcase_add_test(tc_core, test_case3); suite_add_tcase(s, tc_core); return s; -- 2.39.2