On Sat, Nov 23, 2024 at 07:26:55PM +0100, Heiko Eißfeldt wrote:
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -993,9 +993,12 @@ decode_reg_name_and_count (const char *asmspec, int
> *pnregs)
> break;
> if (asmspec[0] != 0 && i < 0)
> {
> - i = atoi (asmspec);
> - if (i < FIRST_PSEUDO_REGISTER && i >= 0 && reg_names[i][0])
> - return i;
> + char *pend{};
Why the {} strtol will write to pend no matter what, so it doesn't need to
be cleared.
> + errno = 0;
> + long j = strtol (asmspec, &pend, 10);
> + if (errno != ERANGE && j <= INT_MAX
> + && j < FIRST_PSEUDO_REGISTER && j >= 0 && reg_names[j][0])
> + return j;
If all conditions don't fit on a single line, all should be written
on separate lines, so
if (errno != ERANGE
&& ...
&& ...)
I'm not sure what is the point of the j <= INT_MAX check,
FIRST_PSEUDO_REGISTER will surely be smaller than INT_MAX on all targets.
Furthermore, when strtol is used, I wonder if that
for (i = strlen (asmspec) - 1; i >= 0; i--)
if (! ISDIGIT (asmspec[i]))
break;
isn't just a waste of time, if it wouldn't be enough to test that
ISDIGIT (asmspec[0]) and *pend == '\0' after the strtol call (if not,
it should fallthrough to the name matching rather than just return -2.
So
- for (i = strlen (asmspec) - 1; i >= 0; i--)
- if (! ISDIGIT (asmspec[i]))
- break;
- if (asmspec[0] != 0 && i < 0)
- {
- i = atoi (asmspec);
- if (i < FIRST_PSEUDO_REGISTER && i >= 0 && reg_names[i][0])
- return i;
- else
- return -2;
- }
+ if (ISDIGIT (asmspec[0]))
+ {
+ char *pend;
+ errno = 0;
+ unsigned long j = strtoul (asmspec, &pend, 10);
+ if (*pend == '\0')
+ {
+ if (errno != ERANGE
+ && j < FIRST_PSEUDO_REGISTER
+ && reg_names[j][0])
+ return j;
+ else
+ return -2;
+ }
+ }
Jakub
