| Message ID | VI1PR0802MB2368E5B2285B1CEEA2633EEF9B709@VI1PR0802MB2368.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Fix the mve multilib for the broken cmse support (pr99939). | expand |
On 12/04/2021 14:04, Srinath Parvathaneni via Gcc-patches wrote: > Hi, > > The current CMSE support in the multilib build for "-march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto" > is broken as specified in PR99939 and this patch fixes the issue. > > Regression tested on arm-none-eabi and found no regressions. > > Ok for master? and Ok for GCC-10 branch? > > Regards, > Srinath. > > gcc/testsuite/ChangeLog: > > 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> > > PR target/99939 > * gcc.target/arm/cmse/cmse-20.c: New test. > > libgcc/ChangeLog: > > 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> > > PR target/99939 > * config/arm/t-arm: Make changes to use cmse.c for all the > armv8.1-m.main mulitlibs. > > > > ############### Attachment also inlined for ease of reply ############### > > > diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c > new file mode 100644 > index 0000000000000000000000000000000000000000..7e2739e14792624adf5b4280ca58a5d8320acbf0 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c > @@ -0,0 +1,28 @@ > +/* { dg-do run } */ > +/* { dg-additional-options "-mcmse -Wl,--section-start,.gnu.sgstubs=0x00190000" } */ > + > +#include <arm_cmse.h> > +#include <stdlib.h> > +#include <stdio.h> > + > +void __attribute__((cmse_nonsecure_entry)) > +secure_fun (int a, int *p) > +{ > + void *b = cmse_check_address_range ((void *)p, a, 1); > + > + if (b == NULL) > + __builtin_abort (); > + printf("%d", *((int *)b)); > +} > + > +int > +main (void) > +{ > + int *ptr; > + int size = 1; > + ptr = (int *) calloc (1, sizeof(int *)); > + *ptr = 1315852292; > + secure_fun (size, ptr); > + free (ptr); > + return 0; > +} > diff --git a/libgcc/config/arm/t-arm b/libgcc/config/arm/t-arm > index 3625a2590beec4e4e0e0881be9ad284c595c7190..949e2ee06653680211ff2dcf0b55a41a6aedc31c 100644 > --- a/libgcc/config/arm/t-arm > +++ b/libgcc/config/arm/t-arm > @@ -9,11 +9,12 @@ CMSE_OPTS:=-mcmse > endif > > ifdef HAVE_CMSE > -ifndef HAVE_V81M > -libgcc-objects += cmse.o cmse_nonsecure_call.o > +libgcc-objects += cmse.o > > cmse.o: $(srcdir)/config/arm/cmse.c > $(gcc_compile) -c $(CMSE_OPTS) $< > +ifndef HAVE_V81M > +libgcc-objects += cmse_nonsecure_call.o > cmse_nonsecure_call.o: $(srcdir)/config/arm/cmse_nonsecure_call.S > $(gcc_compile) -c $< > endif > So if I have two object files using CMSE and one is built with v8m, but the other with v8.1m, when I link them, the needed additional support for the v8m object file will be missing the library support. Wouldn't it be better to just build the cmse_nonsecure_call code unconditionally? It won't be called if it's not needed, but will be there if something does require it. R.
Hi Richard, > -----Original Message----- > From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> > Sent: 13 April 2021 14:55 > To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>; gcc- > patches@gcc.gnu.org > Cc: Richard Earnshaw <Richard.Earnshaw@arm.com> > Subject: Re: [GCC][Patch] arm: Fix the mve multilib for the broken cmse > support (pr99939). > > > > On 12/04/2021 14:04, Srinath Parvathaneni via Gcc-patches wrote: > > Hi, > > > > The current CMSE support in the multilib build for "-march=armv8.1- > m.main+mve -mfloat-abi=hard -mfpu=auto" > > is broken as specified in PR99939 and this patch fixes the issue. > > > > Regression tested on arm-none-eabi and found no regressions. > > > > Ok for master? and Ok for GCC-10 branch? > > > > Regards, > > Srinath. > > > > gcc/testsuite/ChangeLog: > > > > 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> > > > > PR target/99939 > > * gcc.target/arm/cmse/cmse-20.c: New test. > > > > libgcc/ChangeLog: > > > > 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> > > > > PR target/99939 > > * config/arm/t-arm: Make changes to use cmse.c for all the > > armv8.1-m.main mulitlibs. > > > > > > > > ############### Attachment also inlined for ease of reply > ############### > > > > > > diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c > > b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c > > new file mode 100644 > > index > > > 0000000000000000000000000000000000000000..7e2739e14792624adf5b428 > 0ca58 > > a5d8320acbf0 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c > > @@ -0,0 +1,28 @@ > > +/* { dg-do run } */ > > +/* { dg-additional-options "-mcmse > > +-Wl,--section-start,.gnu.sgstubs=0x00190000" } */ > > + > > +#include <arm_cmse.h> > > +#include <stdlib.h> > > +#include <stdio.h> > > + > > +void __attribute__((cmse_nonsecure_entry)) > > +secure_fun (int a, int *p) > > +{ > > + void *b = cmse_check_address_range ((void *)p, a, 1); > > + > > + if (b == NULL) > > + __builtin_abort (); > > + printf("%d", *((int *)b)); > > +} > > + > > +int > > +main (void) > > +{ > > + int *ptr; > > + int size = 1; > > + ptr = (int *) calloc (1, sizeof(int *)); > > + *ptr = 1315852292; > > + secure_fun (size, ptr); > > + free (ptr); > > + return 0; > > +} > > diff --git a/libgcc/config/arm/t-arm b/libgcc/config/arm/t-arm index > > > 3625a2590beec4e4e0e0881be9ad284c595c7190..949e2ee06653680211ff2dcf > 0b55 > > a41a6aedc31c 100644 > > --- a/libgcc/config/arm/t-arm > > +++ b/libgcc/config/arm/t-arm > > @@ -9,11 +9,12 @@ CMSE_OPTS:=-mcmse > > endif > > > > ifdef HAVE_CMSE > > -ifndef HAVE_V81M > > -libgcc-objects += cmse.o cmse_nonsecure_call.o > > +libgcc-objects += cmse.o > > > > cmse.o: $(srcdir)/config/arm/cmse.c > > $(gcc_compile) -c $(CMSE_OPTS) $< > > +ifndef HAVE_V81M > > +libgcc-objects += cmse_nonsecure_call.o > > cmse_nonsecure_call.o: $(srcdir)/config/arm/cmse_nonsecure_call.S > > $(gcc_compile) -c $< > > endif > > > > So if I have two object files using CMSE and one is built with v8m, but the > other with v8.1m, when I link them, the needed additional support for the > v8m object file will be missing the library support. > > Wouldn't it be better to just build the cmse_nonsecure_call code > unconditionally? It won't be called if it's not needed, but will be there if > something does require it. I have modified the patch to build the cmse_nonsecure_call code unconditionally, I have attached the diff and cover letter in this email. Please let me know if it is ok for master? Regards, Srinath. > > R. Hi, The current CMSE support in the multilib build for "-march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto" is broken as specified in PR99939 and this patch fixes the issue. Regression tested on arm-none-eabi and found no regressions. Ok for master? and Ok for GCC-10 branch? Regards, Srinath. gcc/testsuite/ChangeLog: 2021-06-01 Srinath Parvathaneni <srinath.parvathaneni@arm.com> * gcc.target/arm/cmse/cmse-18.c: Modify * gcc.target/arm/cmse/cmse-20.c: New test. libgcc/ChangeLog: 2021-06-01 Srinath Parvathaneni <srinath.parvathaneni@arm.com> * config/arm/cmse_nonsecure_call.S: Modify to add __ARM_FEATURE_MVE macro check. * config/arm/t-arm: Make changes to link cmse.o and cmse_nonsecure_call.o on finding -mcmse gcc options.
On 01/06/2021 18:16, Srinath Parvathaneni via Gcc-patches wrote: > Hi Richard, > >> -----Original Message----- >> From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> >> Sent: 13 April 2021 14:55 >> To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>; gcc- >> patches@gcc.gnu.org >> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com> >> Subject: Re: [GCC][Patch] arm: Fix the mve multilib for the broken cmse >> support (pr99939). >> >> >> >> On 12/04/2021 14:04, Srinath Parvathaneni via Gcc-patches wrote: >>> Hi, >>> >>> The current CMSE support in the multilib build for "-march=armv8.1- >> m.main+mve -mfloat-abi=hard -mfpu=auto" >>> is broken as specified in PR99939 and this patch fixes the issue. >>> >>> Regression tested on arm-none-eabi and found no regressions. >>> >>> Ok for master? and Ok for GCC-10 branch? >>> >>> Regards, >>> Srinath. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> >>> >>> PR target/99939 >>> * gcc.target/arm/cmse/cmse-20.c: New test. >>> >>> libgcc/ChangeLog: >>> >>> 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> >>> >>> PR target/99939 >>> * config/arm/t-arm: Make changes to use cmse.c for all the >>> armv8.1-m.main mulitlibs. >>> >>> >>> >>> ############### Attachment also inlined for ease of reply >> ############### >>> >>> >>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c >>> b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c >>> new file mode 100644 >>> index >>> >> 0000000000000000000000000000000000000000..7e2739e14792624adf5b428 >> 0ca58 >>> a5d8320acbf0 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c >>> @@ -0,0 +1,28 @@ >>> +/* { dg-do run } */ >>> +/* { dg-additional-options "-mcmse >>> +-Wl,--section-start,.gnu.sgstubs=0x00190000" } */ >>> + >>> +#include <arm_cmse.h> >>> +#include <stdlib.h> >>> +#include <stdio.h> >>> + >>> +void __attribute__((cmse_nonsecure_entry)) >>> +secure_fun (int a, int *p) >>> +{ >>> + void *b = cmse_check_address_range ((void *)p, a, 1); >>> + >>> + if (b == NULL) >>> + __builtin_abort (); >>> + printf("%d", *((int *)b)); >>> +} >>> + >>> +int >>> +main (void) >>> +{ >>> + int *ptr; >>> + int size = 1; >>> + ptr = (int *) calloc (1, sizeof(int *)); >>> + *ptr = 1315852292; >>> + secure_fun (size, ptr); >>> + free (ptr); >>> + return 0; >>> +} >>> diff --git a/libgcc/config/arm/t-arm b/libgcc/config/arm/t-arm index >>> >> 3625a2590beec4e4e0e0881be9ad284c595c7190..949e2ee06653680211ff2dcf >> 0b55 >>> a41a6aedc31c 100644 >>> --- a/libgcc/config/arm/t-arm >>> +++ b/libgcc/config/arm/t-arm >>> @@ -9,11 +9,12 @@ CMSE_OPTS:=-mcmse >>> endif >>> >>> ifdef HAVE_CMSE >>> -ifndef HAVE_V81M >>> -libgcc-objects += cmse.o cmse_nonsecure_call.o >>> +libgcc-objects += cmse.o >>> >>> cmse.o: $(srcdir)/config/arm/cmse.c >>> $(gcc_compile) -c $(CMSE_OPTS) $< >>> +ifndef HAVE_V81M >>> +libgcc-objects += cmse_nonsecure_call.o >>> cmse_nonsecure_call.o: $(srcdir)/config/arm/cmse_nonsecure_call.S >>> $(gcc_compile) -c $< >>> endif >>> >> >> So if I have two object files using CMSE and one is built with v8m, but the >> other with v8.1m, when I link them, the needed additional support for the >> v8m object file will be missing the library support. >> >> Wouldn't it be better to just build the cmse_nonsecure_call code >> unconditionally? It won't be called if it's not needed, but will be there if >> something does require it. > > I have modified the patch to build the cmse_nonsecure_call code unconditionally, > I have attached the diff and cover letter in this email. > > Please let me know if it is ok for master? > > Regards, > Srinath. >> >> R. > gcc/testsuite/ChangeLog: 2021-06-01 Srinath Parvathaneni <srinath.parvathaneni@arm.com> * gcc.target/arm/cmse/cmse-18.c: Modify Incomplete. I know you've modified it, but how? * gcc.target/arm/cmse/cmse-20.c: New test. libgcc/ChangeLog: 2021-06-01 Srinath Parvathaneni <srinath.parvathaneni@arm.com> * config/arm/cmse_nonsecure_call.S: Modify to add __ARM_FEATURE_MVE macro check. "Add __ARM_FEATURE_MVE macro check." is sufficient (I know you've modified it). * config/arm/t-arm: Make changes to link cmse.o and cmse_nonsecure_call.o on finding -mcmse gcc options. Again, "Make changes to " is redundant. diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c new file mode 100644 index 0000000000000000000000000000000000000000..7e2739e14792624adf5b4280ca58a5d8320acbf0 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c @@ -0,0 +1,28 @@ +/* { dg-do run } */ +/* { dg-additional-options "-mcmse -Wl,--section-start,.gnu.sgstubs=0x00190000" } */ Why does this need a different stubs address to all the other executable CMSE tests? Can't it use 0x00400000 like them? R.
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c new file mode 100644 index 0000000000000000000000000000000000000000..7e2739e14792624adf5b4280ca58a5d8320acbf0 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c @@ -0,0 +1,28 @@ +/* { dg-do run } */ +/* { dg-additional-options "-mcmse -Wl,--section-start,.gnu.sgstubs=0x00190000" } */ + +#include <arm_cmse.h> +#include <stdlib.h> +#include <stdio.h> + +void __attribute__((cmse_nonsecure_entry)) +secure_fun (int a, int *p) +{ + void *b = cmse_check_address_range ((void *)p, a, 1); + + if (b == NULL) + __builtin_abort (); + printf("%d", *((int *)b)); +} + +int +main (void) +{ + int *ptr; + int size = 1; + ptr = (int *) calloc (1, sizeof(int *)); + *ptr = 1315852292; + secure_fun (size, ptr); + free (ptr); + return 0; +} diff --git a/libgcc/config/arm/t-arm b/libgcc/config/arm/t-arm index 3625a2590beec4e4e0e0881be9ad284c595c7190..949e2ee06653680211ff2dcf0b55a41a6aedc31c 100644 --- a/libgcc/config/arm/t-arm +++ b/libgcc/config/arm/t-arm @@ -9,11 +9,12 @@ CMSE_OPTS:=-mcmse endif ifdef HAVE_CMSE -ifndef HAVE_V81M -libgcc-objects += cmse.o cmse_nonsecure_call.o +libgcc-objects += cmse.o cmse.o: $(srcdir)/config/arm/cmse.c $(gcc_compile) -c $(CMSE_OPTS) $< +ifndef HAVE_V81M +libgcc-objects += cmse_nonsecure_call.o cmse_nonsecure_call.o: $(srcdir)/config/arm/cmse_nonsecure_call.S $(gcc_compile) -c $< endif
Hi, The current CMSE support in the multilib build for "-march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto" is broken as specified in PR99939 and this patch fixes the issue. Regression tested on arm-none-eabi and found no regressions. Ok for master? and Ok for GCC-10 branch? Regards, Srinath. gcc/testsuite/ChangeLog: 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> PR target/99939 * gcc.target/arm/cmse/cmse-20.c: New test. libgcc/ChangeLog: 2021-04-12 Srinath Parvathaneni <srinath.parvathaneni@arm.com> PR target/99939 * config/arm/t-arm: Make changes to use cmse.c for all the armv8.1-m.main mulitlibs. ############### Attachment also inlined for ease of reply ############### diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c new file mode 100644 index 0000000000000000000000000000000000000000..7e2739e14792624adf5b4280ca58a5d8320acbf0 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-20.c @@ -0,0 +1,28 @@ +/* { dg-do run } */ +/* { dg-additional-options "-mcmse -Wl,--section-start,.gnu.sgstubs=0x00190000" } */ + +#include <arm_cmse.h> +#include <stdlib.h> +#include <stdio.h> + +void __attribute__((cmse_nonsecure_entry)) +secure_fun (int a, int *p) +{ + void *b = cmse_check_address_range ((void *)p, a, 1); + + if (b == NULL) + __builtin_abort (); + printf("%d", *((int *)b)); +} + +int +main (void) +{ + int *ptr; + int size = 1; + ptr = (int *) calloc (1, sizeof(int *)); + *ptr = 1315852292; + secure_fun (size, ptr); + free (ptr); + return 0; +} diff --git a/libgcc/config/arm/t-arm b/libgcc/config/arm/t-arm index 3625a2590beec4e4e0e0881be9ad284c595c7190..949e2ee06653680211ff2dcf0b55a41a6aedc31c 100644 --- a/libgcc/config/arm/t-arm +++ b/libgcc/config/arm/t-arm @@ -9,11 +9,12 @@ CMSE_OPTS:=-mcmse endif ifdef HAVE_CMSE -ifndef HAVE_V81M -libgcc-objects += cmse.o cmse_nonsecure_call.o +libgcc-objects += cmse.o cmse.o: $(srcdir)/config/arm/cmse.c $(gcc_compile) -c $(CMSE_OPTS) $< +ifndef HAVE_V81M +libgcc-objects += cmse_nonsecure_call.o cmse_nonsecure_call.o: $(srcdir)/config/arm/cmse_nonsecure_call.S $(gcc_compile) -c $< endif