Hi!
On Fri, Jul 16, 2021 at 11:35:25AM -0700, apinski--- via Gcc-patches wrote:
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -5799,7 +5799,7 @@ parse_optimize_options (tree args, bool attr_p)
>
> if (TREE_CODE (value) == INTEGER_CST)
> {
> - char buffer[20];
> + char buffer[HOST_BITS_PER_LONG / 3 + 4];
> sprintf (buffer, "-O%ld", (long) TREE_INT_CST_LOW (value));
> vec_safe_push (optimize_args, ggc_strdup (buffer));
> }
This calculation is correct, assuming "value" is non-negative. You can
easily avoid all of that though:
int n = snprintf (0, 0, "-O%ld", (long) TREE_INT_CST_LOW (value));
char buffer[n + 1];
sprintf (buffer, "-O%ld", (long) TREE_INT_CST_LOW (value));
vec_safe_push (optimize_args, ggc_strdup (buffer));
This creates a variable length allocation on the stack though. You can
do better (for some value of "better") by using the known longest value:
(again assuming non-negative):
int n = snprintf (0, 0, "-O%ld", LONG_MAX);
char buffer[n + 1];
sprintf (buffer, "-O%ld", (long) TREE_INT_CST_LOW (value));
vec_safe_push (optimize_args, ggc_strdup (buffer));
This does not call snprintf, GCC can evaluate that at compile time.
This pattern works well in portable code.
But we can do better still:
char *buffer;
asprintf (&buffer, "-O%ld", (long) TREE_INT_CST_LOW (value));
vec_safe_push (optimize_args, ggc_strdup (buffer));
free (buffer);
(asprintf is in libiberty already). And we could avoid the ggc_strdup
if we made a variant of asprintf that marks the GGC itself (the aprintf
implementation already factors calculating the length needed, it will
be easy to do this).
Segher
