| Message ID | 20190412141246.GK21066@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix up ARM target attribute handling (PR target/89093) | expand |
On 12/04/2019 15:12, Jakub Jelinek wrote: > Hi! > > Just something I've noticed while looking at Ramana's patch. > > As can be seen on the testcase, on arm we accept arbitrary garbage > after arm or thumb prefixes, is that really desirable? > While for fpu= or arch= we reject garbage after it and so do for > target attribute arg starting with +. > > Ok if this passes bootstrap/regtest? > Bah, that's not supposed to be there, Yes that's ok . > Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64 > does the same), do we really want to support > __attribute__((target (" arm"))) ? > Looked at other targets and can't see anything like that being supported > elsewhere. No, that's not right. we should get rid of this. Thanks a lot for fixing this up Jakub. Ramana > > 2019-04-12 Jakub Jelinek <jakub@redhat.com> > > PR target/89093 > * config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp > instead of strncmp when checking for thumb and arm. Formatting fixes. > > * gcc.target/arm/pr89093.c: New test. > > --- gcc/config/arm/arm.c.jj 2019-04-09 15:18:37.879816537 +0200 > +++ gcc/config/arm/arm.c 2019-04-12 15:36:36.993102230 +0200 > @@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg > while (ISSPACE (*q)) ++q; > > argstr = NULL; > - if (!strncmp (q, "thumb", 5)) > - opts->x_target_flags |= MASK_THUMB; > + if (!strcmp (q, "thumb")) > + opts->x_target_flags |= MASK_THUMB; > > - else if (!strncmp (q, "arm", 3)) > - opts->x_target_flags &= ~MASK_THUMB; > + else if (!strcmp (q, "arm")) > + opts->x_target_flags &= ~MASK_THUMB; > > else if (!strncmp (q, "fpu=", 4)) > { > int fpu_index; > - if (! opt_enum_arg_to_value (OPT_mfpu_, q+4, > + if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4, > &fpu_index, CL_TARGET)) > { > error ("invalid fpu for target attribute or pragma %qs", q); > @@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg > } > else if (!strncmp (q, "arch=", 5)) > { > - char* arch = q+5; > + char *arch = q + 5; > const arch_option *arm_selected_arch > = arm_parse_arch_option_name (all_architectures, "arch", arch); > > --- gcc/testsuite/gcc.target/arm/pr89093.c.jj 2019-04-12 16:05:47.477069147 +0200 > +++ gcc/testsuite/gcc.target/arm/pr89093.c 2019-04-12 16:05:15.948591951 +0200 > @@ -0,0 +1,7 @@ > +/* PR target/89093 */ > +/* { dg-do compile } */ > + > +__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error "unknown target attribute or pragma 'arm.foobar'" } */ > +__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbozoo1'" } */ > +__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbique'" } */ > +__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error "unknown target attribute or pragma 'thumb981'" } */ > > Jakub >
--- gcc/config/arm/arm.c.jj 2019-04-09 15:18:37.879816537 +0200 +++ gcc/config/arm/arm.c 2019-04-12 15:36:36.993102230 +0200 @@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg while (ISSPACE (*q)) ++q; argstr = NULL; - if (!strncmp (q, "thumb", 5)) - opts->x_target_flags |= MASK_THUMB; + if (!strcmp (q, "thumb")) + opts->x_target_flags |= MASK_THUMB; - else if (!strncmp (q, "arm", 3)) - opts->x_target_flags &= ~MASK_THUMB; + else if (!strcmp (q, "arm")) + opts->x_target_flags &= ~MASK_THUMB; else if (!strncmp (q, "fpu=", 4)) { int fpu_index; - if (! opt_enum_arg_to_value (OPT_mfpu_, q+4, + if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4, &fpu_index, CL_TARGET)) { error ("invalid fpu for target attribute or pragma %qs", q); @@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg } else if (!strncmp (q, "arch=", 5)) { - char* arch = q+5; + char *arch = q + 5; const arch_option *arm_selected_arch = arm_parse_arch_option_name (all_architectures, "arch", arch); --- gcc/testsuite/gcc.target/arm/pr89093.c.jj 2019-04-12 16:05:47.477069147 +0200 +++ gcc/testsuite/gcc.target/arm/pr89093.c 2019-04-12 16:05:15.948591951 +0200 @@ -0,0 +1,7 @@ +/* PR target/89093 */ +/* { dg-do compile } */ + +__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error "unknown target attribute or pragma 'arm.foobar'" } */ +__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbozoo1'" } */ +__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbique'" } */ +__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error "unknown target attribute or pragma 'thumb981'" } */
Hi! Just something I've noticed while looking at Ramana's patch. As can be seen on the testcase, on arm we accept arbitrary garbage after arm or thumb prefixes, is that really desirable? While for fpu= or arch= we reject garbage after it and so do for target attribute arg starting with +. Ok if this passes bootstrap/regtest? Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64 does the same), do we really want to support __attribute__((target (" arm"))) ? Looked at other targets and can't see anything like that being supported elsewhere. 2019-04-12 Jakub Jelinek <jakub@redhat.com> PR target/89093 * config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp instead of strncmp when checking for thumb and arm. Formatting fixes. * gcc.target/arm/pr89093.c: New test. Jakub