Hi David, everyone, Following Davids last review I decided to properly detect error cases, as suggested. The error however should be reported earlier in compilation in pack_enum_valud function, where all the errors are reported.
Thanks for the quick and detailed reviews. Regards, Cupertino The BPF backend was allocating an unnecessarily large string when constructing CO-RE relocations for enum types. This patch further verifies if an enumerator is valid for CO-RE representability and returns an error in those cases. gcc/ChangeLog: * config/bpf/core-builtins.cc (get_index_for_enum_value): Create function. (pack_enum_value): Check for enumerator and error out. (process_enum_value): Correct string allocation. --- gcc/config/bpf/core-builtins.cc | 57 ++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc index e03e986e2c1..829acea98f7 100644 --- a/gcc/config/bpf/core-builtins.cc +++ b/gcc/config/bpf/core-builtins.cc @@ -795,6 +795,23 @@ process_field_expr (struct cr_builtins *data) static GTY(()) hash_map<tree, tree> *bpf_enum_mappings; tree enum_value_type = NULL_TREE; +static int +get_index_for_enum_value (tree type, tree expr) +{ + gcc_assert (TREE_CODE (expr) == CONST_DECL + && TREE_CODE (type) == ENUMERAL_TYPE); + + unsigned int index = 0; + for (tree l = TYPE_VALUES (type); l; l = TREE_CHAIN (l)) + { + gcc_assert (index < (1 << 16)); + if (TREE_VALUE (l) == expr) + return index; + index++; + } + return -1; +} + /* Pack helper for the __builtin_preserve_enum_value. */ static struct cr_local @@ -846,6 +863,16 @@ pack_enum_value_fail: ret.reloc_data.default_value = integer_one_node; } + if (ret.fail == false ) + { + int index = get_index_for_enum_value (type, tmp); + if (index == -1 || index >= (1 << 16)) + { + bpf_error ("enum value in CO-RE builtin cannot be represented"); + ret.fail = true; + } + } + ret.reloc_data.type = type; ret.reloc_data.kind = kind; return ret; @@ -864,25 +891,17 @@ process_enum_value (struct cr_builtins *data) struct cr_final ret = { NULL, type, data->kind }; - if (TREE_CODE (expr) == CONST_DECL - && TREE_CODE (type) == ENUMERAL_TYPE) - { - unsigned int index = 0; - for (tree l = TYPE_VALUES (type); l; l = TREE_CHAIN (l)) - { - if (TREE_VALUE (l) == expr) - { - char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1); - sprintf (tmp, "%d", index); - ret.str = (const char *) tmp; - - break; - } - index++; - } - } - else - gcc_unreachable (); + gcc_assert (TREE_CODE (expr) == CONST_DECL + && TREE_CODE (type) == ENUMERAL_TYPE); + + int index = get_index_for_enum_value (type, expr); + gcc_assert (index != -1 && index < (1 << 16)); + + /* Index can only be a value up to 2^16. Should always fit + in 6 chars. */ + char tmp[6]; + sprintf (tmp, "%u", index); + ret.str = CONST_CAST (char *, ggc_strdup(tmp)); return ret; } -- 2.30.2