Hi David,
Thanks for the review.
Will apply the changes.
Nice catch and optimization with the vlen size.
Cupertino
David Faust writes:
> 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++;
