Hi Cupertino,
On 4/11/24 04:11, Cupertino Miranda wrote:
> Code was allocating way too much space for the string.
A little bit more description would not hurt. Perhaps you could say
something like:
"The BPF backend was allocating an unnecessarily large string when
constructing CO-RE relocations for enum types."
>
> gcc/ChangeLog:
> * config/bpf/core-builtins.cc (process_enum_value): Corrected
> string allocation.
nit: present tense, i.e. "Correct" rather than "Corrected"
> ---
> gcc/config/bpf/core-builtins.cc | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
> index e03e986e2c1..ead1777d465 100644
> --- a/gcc/config/bpf/core-builtins.cc
> +++ b/gcc/config/bpf/core-builtins.cc
> @@ -872,10 +872,11 @@ process_enum_value (struct cr_builtins *data)
> {
> if (TREE_VALUE (l) == expr)
> {
> - char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1);
> + /* Array size is 21 = ceil(log_10(2^64)) + 1 to hold string
> + representations of 64 bit integers. */
> + char tmp[21];
> sprintf (tmp, "%d", index);
It looks like `index' is an `unsigned int', so this sprintf should use
%u rather %d, no?
Also, it occurs to me that the `vlen' of BTF types is only 16 bits, so
BTF has no way currently to represent enums with more than 65535
enumerators. It may be worth adding a sanity check to bail out (error)
here if we're going to claim an index higher than that. And if that is
validated before the printf, the buffer can be 6 bytes ("65535\0").
> - ret.str = (const char *) tmp;
> -
> + ret.str = CONST_CAST (char *, ggc_strdup(tmp));
> break;
> }
> index++;
