[Bug bootstrap/63853] [5.0 Regression] The use of strchrnul breaks bootstrap on x86_64-apple-darwin14.
jakub at gcc dot gnu.org
gcc-bugzilla@gcc.gnu.org
Thu Nov 13 18:57:00 GMT 2014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63853
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #9)
> I just completed a clean bootstrap of r217511 with the following patch
This is ok for trunk so that we don't have bootstrap breakages for too long.
That said, most of the strchrnul uses look to me like completely unnecessary.
> --- ../_clean/gcc/gcc.c 2014-11-13 15:29:00.000000000 +0100
> +++ gcc/gcc.c 2014-11-13 17:59:47.000000000 +0100
> @@ -3375,12 +3375,16 @@ handle_foffload_option (const char *arg)
> if (arg[0] == '-')
> return;
>
> - end = strchrnul (arg, '=');
> + end = strchr (arg, '=');
> + if (end == NULL)
> + end = strchr (arg, '\0');
> cur = arg;
>
> while (cur < end)
> {
> - next = strchrnul (cur, ',');
> + next = strchr (cur, ',');
> + if (next == NULL)
> + next = strchr (cur, '\0');
E.g. here. If strchr (cur, ',') returns NULL, then what
strchr (cur, '\0'); returns is either equal to end, or > end, so we can just
set next = end;.
> strncpy (target, cur, next - cur);
memcpy should be used instead.
> - n = strchrnul (c, ',');
> + n = strchr (c, ',');
> + if (n == NULL)
> + n = strchr (c, '\0');
>
> if (strlen (target) == (size_t) (n - c)
Here, strlen (target) should be known, isn't it next - cur ?
(and another spot with that).
Also,
offload_targets = xstrdup (target);
(could have instead just set offload_targets = target;
and ensure it isn't freed, say by setting target = NULL).
And:
offload_targets
= XRESIZEVEC (char, offload_targets,
strlen (offload_targets) + strlen (target) + 2);
if (strlen (offload_targets) != 0)
strcat (offload_targets, ":");
strcat (offload_targets, target);
Compute strlen (offload_targets) once, strlen (target) is known (next - cur),
and don't compute it 5 times instead.
size_t offload_targets_len = strlen (offload_targets);
offload_targets = XRESIZEVEC (char, offload_targets,
offload_targets_len + (next - cur)
+ 2);
if (offload_targets_len)
offload_targets[offload_targets_len++] = ':';
memcpy (offload_targets + offload_targets_len, target, next -
cur);
? Especially if (strlen (offload_targets) != 0) is very expensive way of
if (*offload_targets)
The tree-ssa-strlen.c pass will presumably optimize some of this, but given
the conditional likely not all of that.
Sorry for not catching that during review.
s + strcspn (s, "=") instead of strchrnul (s, '=') is reasonable too.
More information about the Gcc-bugs
mailing list