public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] bpf: remove huge memory waste with string allocation.
@ 2024-04-17 18:44 Cupertino Miranda
  2024-04-17 19:21 ` David Faust
  0 siblings, 1 reply; 2+ messages in thread
From: Cupertino Miranda @ 2024-04-17 18:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: jose.marchesi, david.faust, elena.zannoni, Cupertino Miranda

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): Correct
	string allocation.
---
 gcc/config/bpf/core-builtins.cc | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
index e03e986e2c1..beb039ea6e0 100644
--- a/gcc/config/bpf/core-builtins.cc
+++ b/gcc/config/bpf/core-builtins.cc
@@ -870,12 +870,14 @@ process_enum_value (struct cr_builtins *data)
       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)
 	    {
-	      char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1);
-	      sprintf (tmp, "%d", index);
-	      ret.str = (const char *) tmp;
-
+	      /* 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));
 	      break;
 	    }
 	  index++;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] bpf: remove huge memory waste with string allocation.
  2024-04-17 18:44 [PATCH v2] bpf: remove huge memory waste with string allocation Cupertino Miranda
@ 2024-04-17 19:21 ` David Faust
  0 siblings, 0 replies; 2+ messages in thread
From: David Faust @ 2024-04-17 19:21 UTC (permalink / raw)
  To: Cupertino Miranda; +Cc: gcc-patches, jose.marchesi, elena.zannoni



On 4/17/24 11:44, Cupertino Miranda wrote:
> 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): Correct
> 	string allocation.
> ---
>  gcc/config/bpf/core-builtins.cc | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
> index e03e986e2c1..beb039ea6e0 100644
> --- a/gcc/config/bpf/core-builtins.cc
> +++ b/gcc/config/bpf/core-builtins.cc
> @@ -870,12 +870,14 @@ process_enum_value (struct cr_builtins *data)
>        unsigned int index = 0;
>        for (tree l = TYPE_VALUES (type); l; l = TREE_CHAIN (l))
>  	{
> +	  gcc_assert (index < (1 << 16));

Since the index here is computed from the TREE node and not BTF, is it
somehow already enforced that this code won't be run if the enum can't
be represented in BTF? 

If not, I don't think the assert is a good idea. The TREE node could be
an enum too large for BTF, but not too large for gcc.  So the assert
will cause an ICE for something that is a representation limit (in BTF
and by extension .BTF.ext i.e. CO-RE) rather than a compiler bug.
IMO an error or warning makes more sense here, something like:

  if (index >= (1 << 16))
    {
      bpf_error ("enumerator in CO-RE relocation cannot be represented");
      break;
    }

WDYT?

>  	  if (TREE_VALUE (l) == expr)
>  	    {
> -	      char *tmp = (char *) ggc_alloc_atomic ((index / 10) + 1);
> -	      sprintf (tmp, "%d", index);
> -	      ret.str = (const char *) tmp;
> -
> +	      /* 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));
>  	      break;
>  	    }
>  	  index++;

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-17 19:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 18:44 [PATCH v2] bpf: remove huge memory waste with string allocation Cupertino Miranda
2024-04-17 19:21 ` David Faust

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).