On Thu, Nov 28, 2024 at 08:15:08PM +0100, Heiko Eißfeldt wrote:
> > There are three important parts missing.
> >
> > I don't see you in MAINTAINERS file, so you need to decide if you assign
> > copyright to FSF or submit this under DCO.
> I wonder if it is ok to add myself to MAINTAINERS file?
No. That happens only if/when one gets account on sourceware, as a first
commit under the new rights.
And account is given only after a few earlier contributions where somebody
commits it for you.
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,16 @@
> +2024-11-28 Heiko Eißfeldt <he...@hexco.de>
> +
> + PR middle-end/114540
> + * varasm.cc (decode_reg_name_and_count): Use strtoul instead of atoi
> + and simplify verification the whole asmspec contains just decimal
> + digits.
> +
> + * gcc.dg/pr114540.c: New test.
> +
> + Signed-off-by: Heiko Eißfeldt <he...@hexco.de>
> + Co-authored-by: Jakub Jelinek <ja...@redhat.com>
We don't put the ChangeLog changes into the patch being committed,
that would result in patch conflicts all the time, so the ChangeLog
entry is written in the plaintext at the end of the commit message
instead and a daily script processes commits and populates ChangeLog files.
> + asm("":::"2147483647&"); /* INT_MAX + garbage char { dg-error
> "unknown register name" } */
I chose to use 4& instead of 2147483647& because for 2147483647 it would be
rejected anyway, no matter whether there is garbage after it or not.
And I've added one case with garbage in between.
> +
> + register int a asm("2147483648"); /* INT_MAX + 1 {
> dg-error "invalid register name for" } */
> + register int b asm("4294967296"); /* UINT_MAX + 1 {
> dg-error "invalid register name for" } */
> + register int c asm("18446744073709551616"); /* ULONG_MAX + 1 {
> dg-error "invalid register name for" } */
> + register int d asm("9223372036854775808"); /* LONG_MAX + 1 {
> dg-error "invalid register name for" } */
> + register int e asm("9223372036854775807"); /* LONG_MAX {
> dg-error "invalid register name for" } */
> + register int f asm("2147483647"); /* INT_MAX {
> dg-error "invalid register name for" } */
> + register int g asm("2147483647&"); /* INT_MAX + garbage char {
> dg-error "invalid register name for" } */
> + register int h asm("0"); /* real reg */
This one doesn't really work on all targets, e.g. on hppa-linux
I get
error: the register specified for ‘h’ is not general enough to be used as a
register variable
So, I've guarded the 0 cases with #if check for a couple of targets which
handle register 0 well.
Here is what I've committed in https://gcc.gnu.org/r15-5877 for you:
replace atoi with strtoul in varasm.cc (decode_reg_name_and_count) [PR114540]
The function uses atoi, which can silently return valid numbers even for
some too large numbers in the string.
Furthermore, the verification that all the characters in asmspec are
decimal digits can be simplified when using strotoul, we can check just
the first digit and whether the end pointer points to '\0'.
2024-12-03 Heiko Eißfeldt <he...@hexco.de>
PR middle-end/114540
* varasm.cc (decode_reg_name_and_count): Use strtoul instead of atoi
and simplify verification that the whole asmspec contains just decimal
digits.
* gcc.dg/pr114540.c: New test.
Signed-off-by: Heiko Eißfeldt <he...@hexco.de>
Co-authored-by: Jakub Jelinek <ja...@redhat.com>
--- gcc/varasm.cc.jj 2024-11-28 11:42:02.289812658 +0100
+++ gcc/varasm.cc 2024-12-02 19:49:40.602841404 +0100
@@ -990,16 +990,21 @@ decode_reg_name_and_count (const char *a
asmspec = strip_reg_name (asmspec);
/* Allow a decimal number as a "register name". */
- for (i = strlen (asmspec) - 1; i >= 0; i--)
- if (! ISDIGIT (asmspec[i]))
- break;
- if (asmspec[0] != 0 && i < 0)
+ if (ISDIGIT (asmspec[0]))
{
- i = atoi (asmspec);
- if (i < FIRST_PSEUDO_REGISTER && i >= 0 && reg_names[i][0])
- return i;
- else
- return -2;
+ char *pend;
+ errno = 0;
+ unsigned long j = strtoul (asmspec, &pend, 10);
+ if (*pend == '\0')
+ {
+ static_assert (FIRST_PSEUDO_REGISTER <= INT_MAX, "");
+ if (errno != ERANGE
+ && j < FIRST_PSEUDO_REGISTER
+ && reg_names[j][0])
+ return j;
+ else
+ return -2;
+ }
}
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
--- gcc/testsuite/gcc.dg/pr114540.c.jj 2024-12-02 19:49:40.603841390 +0100
+++ gcc/testsuite/gcc.dg/pr114540.c 2024-12-02 19:58:29.021425513 +0100
@@ -0,0 +1,31 @@
+/* PR middle-end/114540 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+void
+foo ()
+{
+ asm ("" : : : "2147483648"); /* { dg-error "unknown register
name" } */
+ asm ("" : : : "4294967296"); /* { dg-error "unknown register
name" } */
+ asm ("" : : : "18446744073709551616"); /* { dg-error "unknown register
name" } */
+ asm ("" : : : "9223372036854775808"); /* { dg-error "unknown
register name" } */
+ asm ("" : : : "9223372036854775807"); /* { dg-error "unknown
register name" } */
+ asm ("" : : : "2147483647"); /* { dg-error "unknown register
name" } */
+ asm ("" : : : "4&"); /* { dg-error "unknown register
name" } */
+ asm ("" : : : "1'0"); /* { dg-error "unknown
register name" } */
+#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) ||
defined(__s390__) || defined(__aarch64__) || defined(__arm__)
+ asm ("" : : : "0");
+#endif
+
+ register int a asm("2147483648"); /* { dg-error "invalid register
name for" } */
+ register int b asm("4294967296"); /* { dg-error "invalid register
name for" } */
+ register int c asm("18446744073709551616"); /* { dg-error "invalid register
name for" } */
+ register int d asm("9223372036854775808"); /* { dg-error "invalid register
name for" } */
+ register int e asm("9223372036854775807"); /* { dg-error "invalid register
name for" } */
+ register int f asm("2147483647"); /* { dg-error "invalid register
name for" } */
+ register int g asm("4&"); /* { dg-error "invalid register
name for" } */
+ register int h asm("1'0"); /* { dg-error "invalid register
name for" } */
+#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) ||
defined(__s390__) || defined(__aarch64__) || defined(__arm__)
+ register int i asm("0");
+#endif
+}
Jakub
