* [PATCH 0/2] PR77822 @ 2016-11-17 15:53 Dominik Vogt 2016-11-17 15:54 ` [PATCH 1/2] PR77822 Dominik Vogt 2016-11-17 15:54 ` [PATCH 2/2] PR77822 Dominik Vogt 0 siblings, 2 replies; 17+ messages in thread From: Dominik Vogt @ 2016-11-17 15:53 UTC (permalink / raw) To: gcc-patches; +Cc: Andreas Krebbel The following two patches fix PR 77822 on s390x for gcc-7. As the macro doing the argument range checks can be used on other targets as well, I've put it in system.h (couldn't think of a better place; maybe rtl.h?). Bootstrapped on s390x biarch, regression tested on s390x biarch and s390, all on a zEC12 with -march=zEC12. Please check the commit messages for details. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PR77822 2016-11-17 15:53 [PATCH 0/2] PR77822 Dominik Vogt @ 2016-11-17 15:54 ` Dominik Vogt 2016-11-18 9:31 ` Segher Boessenkool 2016-11-17 15:54 ` [PATCH 2/2] PR77822 Dominik Vogt 1 sibling, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-17 15:54 UTC (permalink / raw) To: gcc-patches, Andreas Krebbel [-- Attachment #1: Type: text/plain, Size: 528 bytes --] On Thu, Nov 17, 2016 at 04:53:03PM +0100, Dominik Vogt wrote: > The following two patches fix PR 77822 on s390x for gcc-7. As the > macro doing the argument range checks can be used on other targets > as well, I've put it in system.h (couldn't think of a better > place; maybe rtl.h?). > > Bootstrapped on s390x biarch, regression tested on s390x biarch > and s390, all on a zEC12 with -march=zEC12. > > Please check the commit messages for details. Common code patch. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0001-ChangeLog --] [-- Type: text/plain, Size: 53 bytes --] gcc/ChangeLog * system.h (SIZE_POS_IN_RANGE): New. [-- Attachment #3: 0001-PR-target-77822-Add-helper-macro-SIZE_POS_IN_RANGE-t.patch --] [-- Type: text/plain, Size: 1267 bytes --] From 44654374a0d62e7bc91356fc2189fac5cd99ae12 Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:18 +0100 Subject: [PATCH 1/2] PR target/77822: Add helper macro SIZE_POS_IN_RANGE to system.h. The macro can be used to validate the arguments of zero_extract and sign_extract to fix this problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822 --- gcc/system.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gcc/system.h b/gcc/system.h index 8c6127c..cc68f07 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -316,6 +316,14 @@ extern int errno; ((unsigned HOST_WIDE_INT) (VALUE) - (unsigned HOST_WIDE_INT) (LOWER) \ <= (unsigned HOST_WIDE_INT) (UPPER) - (unsigned HOST_WIDE_INT) (LOWER)) +/* A convenience macro to determine whether a SIZE lies inclusively + within [1, RANGE], POS lies inclusively within between + [0, RANGE - 1] and the sum lies inclusively within [1, RANGE]. */ +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \ + (IN_RANGE (POS, 0, (RANGE) - 1) \ + && IN_RANGE (SIZE, 1, RANGE) \ + && IN_RANGE ((SIZE) + (POS), 1, RANGE)) + /* Infrastructure for defining missing _MAX and _MIN macros. Note that macros defined with these cannot be used in #if. */ -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PR77822 2016-11-17 15:54 ` [PATCH 1/2] PR77822 Dominik Vogt @ 2016-11-18 9:31 ` Segher Boessenkool 2016-11-18 12:09 ` Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Segher Boessenkool @ 2016-11-18 9:31 UTC (permalink / raw) To: vogt, gcc-patches, Andreas Krebbel Hi Dominik, On Thu, Nov 17, 2016 at 04:53:47PM +0100, Dominik Vogt wrote: > +/* A convenience macro to determine whether a SIZE lies inclusively > + within [1, RANGE], POS lies inclusively within between > + [0, RANGE - 1] and the sum lies inclusively within [1, RANGE]. */ > +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \ > + (IN_RANGE (POS, 0, (RANGE) - 1) \ > + && IN_RANGE (SIZE, 1, RANGE) \ > + && IN_RANGE ((SIZE) + (POS), 1, RANGE)) You should put parens around every use of SIZE, POS, RANGE -- there could be a comma operator in the macro argument. You don't check if SIZE+POS overflows / wraps around. If you really don't care about that, you can lose the > + && IN_RANGE (SIZE, 1, RANGE) \ part as well? Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PR77822 2016-11-18 9:31 ` Segher Boessenkool @ 2016-11-18 12:09 ` Dominik Vogt 2016-11-18 14:02 ` Segher Boessenkool 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-18 12:09 UTC (permalink / raw) To: Segher Boessenkool; +Cc: gcc-patches, Andreas Krebbel On Fri, Nov 18, 2016 at 03:31:40AM -0600, Segher Boessenkool wrote: > Hi Dominik, > > On Thu, Nov 17, 2016 at 04:53:47PM +0100, Dominik Vogt wrote: > > +/* A convenience macro to determine whether a SIZE lies inclusively > > + within [1, RANGE], POS lies inclusively within between > > + [0, RANGE - 1] and the sum lies inclusively within [1, RANGE]. */ > > +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \ > > + (IN_RANGE (POS, 0, (RANGE) - 1) \ > > + && IN_RANGE (SIZE, 1, RANGE) \ > > + && IN_RANGE ((SIZE) + (POS), 1, RANGE)) > > You should put parens around every use of SIZE, POS, RANGE -- there could > be a comma operator in the macro argument. Good point. That wouldn't matter for a backend macro, but in system.h it does. > You don't check if SIZE+POS overflows / wraps around. If you really don't > care about that, you can lose the > > > + && IN_RANGE (SIZE, 1, RANGE) \ > > part as well? The macro is _intended_ for checking the (possibly bogus) arguments of zero_extract and sing_extract that combine may generate. SIZE is some unsigned value, but POS might be unsigned or signed, and actually have a negative value (INTVAL (operands[x]) or UINTVAL (operands[x]))), and RANGE is typically 64 or another small value. Anyway, the macro should work for any inputs (except RANGE <= 0). How about this: -- /* A convenience macro to determine whether a SIZE lies inclusively within [1, UPPER], POS lies inclusively within between [0, UPPER - 1] and the sum lies inclusively within [1, UPPER]. UPPER must be >= 1. */ #define SIZE_POS_IN_RANGE(SIZE, POS, UPPER) \ (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (UPPER) - 1) \ && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (UPPER) \ - (unsigned HOST_WIDE_INT)(POS))) -- IN_RANGE(POS...) makes sure that POS is a non-negative number smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there some case that the new macro does not handle? Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PR77822 2016-11-18 12:09 ` Dominik Vogt @ 2016-11-18 14:02 ` Segher Boessenkool 2016-11-18 15:29 ` Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Segher Boessenkool @ 2016-11-18 14:02 UTC (permalink / raw) To: vogt, gcc-patches, Andreas Krebbel On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: > How about this: > > -- > /* A convenience macro to determine whether a SIZE lies inclusively > within [1, UPPER], POS lies inclusively within between > [0, UPPER - 1] and the sum lies inclusively within [1, UPPER]. > UPPER must be >= 1. */ > #define SIZE_POS_IN_RANGE(SIZE, POS, UPPER) \ > (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (UPPER) - 1) \ > && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (UPPER) \ > - (unsigned HOST_WIDE_INT)(POS))) ^ missing space here > IN_RANGE(POS...) makes sure that POS is a non-negative number > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there > some case that the new macro does not handle? I think it works fine. With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like IN_RANGE, i.e. UPPER is inclusive then. Dunno. Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] PR77822 2016-11-18 14:02 ` Segher Boessenkool @ 2016-11-18 15:29 ` Dominik Vogt 2016-11-21 11:04 ` [PATCH 1/2 v3] PR77822 Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-18 15:29 UTC (permalink / raw) To: gcc-patches On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: > > IN_RANGE(POS...) makes sure that POS is a non-negative number > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there > > some case that the new macro does not handle? > > I think it works fine. > > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like > IN_RANGE, i.e. UPPER is inclusive then. Dunno. Yeah, maybe rather call it RANGE to avoid too much similarity. Some name that is so vague that one has to read the documentation. I'll post an updated patch later. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-18 15:29 ` Dominik Vogt @ 2016-11-21 11:04 ` Dominik Vogt 2016-11-23 20:22 ` Jeff Law 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-21 11:04 UTC (permalink / raw) To: gcc-patches, Andreas Krebbel; +Cc: Segher Boessenkool [-- Attachment #1: Type: text/plain, Size: 908 bytes --] On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote: > On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: > > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: > > > IN_RANGE(POS...) makes sure that POS is a non-negative number > > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there > > > some case that the new macro does not handle? > > > > I think it works fine. > > > > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like > > IN_RANGE, i.e. UPPER is inclusive then. Dunno. > > Yeah, maybe rather call it RANGE to avoid too much similarity. > Some name that is so vague that one has to read the documentation. > I'll post an updated patch later. Third version of the patch attached. The only difference is that the macro argument name has been changed back to RANGE. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0001-v3-ChangeLog --] [-- Type: text/plain, Size: 70 bytes --] gcc/ChangeLog PR target/77822 * system.h (SIZE_POS_IN_RANGE): New. [-- Attachment #3: 0001-v3-PR-target-77822-Add-helper-macro-SIZE_POS_IN_RANGE-t.patch --] [-- Type: text/plain, Size: 1376 bytes --] From 8e02352c70d478c74155d5d127560da31363dd8a Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:18 +0100 Subject: [PATCH 1/2] PR target/77822: Add helper macro SIZE_POS_IN_RANGE to system.h. The macro can be used to validate the arguments of zero_extract and sign_extract to fix this problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822 --- gcc/system.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gcc/system.h b/gcc/system.h index 8c6127c..6c1228d 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -316,6 +316,15 @@ extern int errno; ((unsigned HOST_WIDE_INT) (VALUE) - (unsigned HOST_WIDE_INT) (LOWER) \ <= (unsigned HOST_WIDE_INT) (UPPER) - (unsigned HOST_WIDE_INT) (LOWER)) +/* A convenience macro to determine whether SIZE lies inclusively + within [1, RANGE], POS lies inclusively within between + [0, RANGE - 1] and the sum lies inclusively within [1, RANGE]. + RANGE must be >= 1, but SIZE and POS may be negative. */ +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \ + (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (RANGE) - 1) \ + && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (RANGE) \ + - (unsigned HOST_WIDE_INT)(POS))) + /* Infrastructure for defining missing _MAX and _MIN macros. Note that macros defined with these cannot be used in #if. */ -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-21 11:04 ` [PATCH 1/2 v3] PR77822 Dominik Vogt @ 2016-11-23 20:22 ` Jeff Law 2016-11-24 9:59 ` Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Jeff Law @ 2016-11-23 20:22 UTC (permalink / raw) To: vogt, gcc-patches, Andreas Krebbel, Segher Boessenkool On 11/21/2016 04:03 AM, Dominik Vogt wrote: > On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote: >> > On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: >>> > > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: >>>> > > > IN_RANGE(POS...) makes sure that POS is a non-negative number >>>> > > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there >>>> > > > some case that the new macro does not handle? >>> > > >>> > > I think it works fine. >>> > > >>> > > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like >>> > > IN_RANGE, i.e. UPPER is inclusive then. Dunno. >> > >> > Yeah, maybe rather call it RANGE to avoid too much similarity. >> > Some name that is so vague that one has to read the documentation. >> > I'll post an updated patch later. > Third version of the patch attached. The only difference is that > the macro argument name has been changed back to RANGE. > > Ciao > > Dominik ^_^ ^_^ > > -- Dominik Vogt IBM Germany > > > 0001-v3-ChangeLog > > > gcc/ChangeLog > > PR target/77822 > * system.h (SIZE_POS_IN_RANGE): New. OK. Though system.h seems like an unfortunate place. Would rtl.h work better since this is really about verifying the arguments to things like {zero,sign}_extract which are RTL concepts. jeff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-23 20:22 ` Jeff Law @ 2016-11-24 9:59 ` Dominik Vogt 2016-11-24 15:33 ` Jeff Law 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-24 9:59 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, Andreas Krebbel, Segher Boessenkool On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote: > On 11/21/2016 04:03 AM, Dominik Vogt wrote: > >On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote: > >>> On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: > >>>> > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: > >>>>> > > IN_RANGE(POS...) makes sure that POS is a non-negative number > >>>>> > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there > >>>>> > > some case that the new macro does not handle? > >>>> > > >>>> > I think it works fine. > >>>> > > >>>> > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like > >>>> > IN_RANGE, i.e. UPPER is inclusive then. Dunno. > >>> > >>> Yeah, maybe rather call it RANGE to avoid too much similarity. > >>> Some name that is so vague that one has to read the documentation. > >>> I'll post an updated patch later. > >Third version of the patch attached. The only difference is that > >the macro argument name has been changed back to RANGE. > > > > PR target/77822 > > * system.h (SIZE_POS_IN_RANGE): New. > OK. Though system.h seems like an unfortunate place. Would rtl.h > work better since this is really about verifying the arguments to > things like {zero,sign}_extract which are RTL concepts. I was unsure whether it's better to give the macro a name not related to *_extract and put it into system.h or make it specific to extracts and put it in rtl.h. It's probably not really reuseable elsewhere, so when moving it to rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay? Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-24 9:59 ` Dominik Vogt @ 2016-11-24 15:33 ` Jeff Law 2016-11-25 8:30 ` Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Jeff Law @ 2016-11-24 15:33 UTC (permalink / raw) To: vogt, gcc-patches, Andreas Krebbel, Segher Boessenkool On 11/24/2016 02:59 AM, Dominik Vogt wrote: > On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote: >> On 11/21/2016 04:03 AM, Dominik Vogt wrote: >>> On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote: >>>>> On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: >>>>>>> On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: >>>>>>>>> IN_RANGE(POS...) makes sure that POS is a non-negative number >>>>>>>>> smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there >>>>>>>>> some case that the new macro does not handle? >>>>>>> >>>>>>> I think it works fine. >>>>>>> >>>>>>> With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like >>>>>>> IN_RANGE, i.e. UPPER is inclusive then. Dunno. >>>>> >>>>> Yeah, maybe rather call it RANGE to avoid too much similarity. >>>>> Some name that is so vague that one has to read the documentation. >>>>> I'll post an updated patch later. >>> Third version of the patch attached. The only difference is that >>> the macro argument name has been changed back to RANGE. > >>> >>> PR target/77822 >>> * system.h (SIZE_POS_IN_RANGE): New. >> OK. Though system.h seems like an unfortunate place. Would rtl.h >> work better since this is really about verifying the arguments to >> things like {zero,sign}_extract which are RTL concepts. > > I was unsure whether it's better to give the macro a name not > related to *_extract and put it into system.h or make it specific > to extracts and put it in rtl.h. Yea. What tends to tip the balance for me is that it's likely not reusable outside extraction argument testing. > > It's probably not really reuseable elsewhere, so when moving it to > rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay? Yea, that sounds perfect. Jeff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-24 15:33 ` Jeff Law @ 2016-11-25 8:30 ` Dominik Vogt 2016-11-28 22:29 ` Jeff Law 2016-12-02 8:39 ` Andreas Krebbel 0 siblings, 2 replies; 17+ messages in thread From: Dominik Vogt @ 2016-11-25 8:30 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, Andreas Krebbel, Segher Boessenkool [-- Attachment #1: Type: text/plain, Size: 1152 bytes --] On Thu, Nov 24, 2016 at 08:33:32AM -0700, Jeff Law wrote: > On 11/24/2016 02:59 AM, Dominik Vogt wrote: > >On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote: > >>> PR target/77822 > >>> * system.h (SIZE_POS_IN_RANGE): New. > >>OK. Though system.h seems like an unfortunate place. Would rtl.h > >>work better since this is really about verifying the arguments to > >>things like {zero,sign}_extract which are RTL concepts. > > > >I was unsure whether it's better to give the macro a name not > >related to *_extract and put it into system.h or make it specific > >to extracts and put it in rtl.h. > Yea. What tends to tip the balance for me is that it's likely not > reusable outside extraction argument testing. > > > > >It's probably not really reuseable elsewhere, so when moving it to > >rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay? > Yea, that sounds perfect. Version 4 of the patch attached. Bootstrapped and regression tested on s390x biarch and s390. Changes since the last version: v4: Rename SIZE_POS_IN_RANGE to EXTRACT_ARGS_IN_RANGE and move it to rtl.h. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0001-v4-ChangeLog --] [-- Type: text/plain, Size: 71 bytes --] gcc/ChangeLog PR target/77822 * rtl.h (EXTRACT_ARGS_IN_RANGE): New. [-- Attachment #3: 0001-v4-PR-target-77822-Add-helper-macro-EXTRACT_ARGS_IN_RAN.patch --] [-- Type: text/plain, Size: 1390 bytes --] From 8e5bcea2983b50ba133a83086fcc21c31d8dbabe Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:18 +0100 Subject: [PATCH 1/2] PR target/77822: Add helper macro EXTRACT_ARGS_IN_RANGE to system.h. The macro can be used to validate the arguments of zero_extract and sign_extract to fix this problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822 --- gcc/rtl.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gcc/rtl.h b/gcc/rtl.h index 660d381..1847bce 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -2694,6 +2694,16 @@ get_full_set_src_cost (rtx x, machine_mode mode, struct full_rtx_costs *c) } #endif +/* A convenience macro to validate the arguments of a zero_extract + expression. It determines whether SIZE lies inclusively within + [1, RANGE], POS lies inclusively within between [0, RANGE - 1] + and the sum lies inclusively within [1, RANGE]. RANGE must be + >= 1, but SIZE and POS may be negative. */ +#define EXTRACT_ARGS_IN_RANGE(SIZE, POS, RANGE) \ + (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (RANGE) - 1) \ + && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (RANGE) \ + - (unsigned HOST_WIDE_INT)(POS))) + /* In explow.c */ extern HOST_WIDE_INT trunc_int_for_mode (HOST_WIDE_INT, machine_mode); extern rtx plus_constant (machine_mode, rtx, HOST_WIDE_INT, bool = false); -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-25 8:30 ` Dominik Vogt @ 2016-11-28 22:29 ` Jeff Law 2016-12-02 8:39 ` Andreas Krebbel 1 sibling, 0 replies; 17+ messages in thread From: Jeff Law @ 2016-11-28 22:29 UTC (permalink / raw) To: vogt, gcc-patches, Andreas Krebbel, Segher Boessenkool On 11/25/2016 01:29 AM, Dominik Vogt wrote: > On Thu, Nov 24, 2016 at 08:33:32AM -0700, Jeff Law wrote: >> On 11/24/2016 02:59 AM, Dominik Vogt wrote: >>> On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote: >>>>> PR target/77822 >>>>> * system.h (SIZE_POS_IN_RANGE): New. >>>> OK. Though system.h seems like an unfortunate place. Would rtl.h >>>> work better since this is really about verifying the arguments to >>>> things like {zero,sign}_extract which are RTL concepts. >>> >>> I was unsure whether it's better to give the macro a name not >>> related to *_extract and put it into system.h or make it specific >>> to extracts and put it in rtl.h. >> Yea. What tends to tip the balance for me is that it's likely not >> reusable outside extraction argument testing. >> >>> >>> It's probably not really reuseable elsewhere, so when moving it to >>> rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay? >> Yea, that sounds perfect. > > Version 4 of the patch attached. Bootstrapped and regression > tested on s390x biarch and s390. Changes since the last version: > > v4: > Rename SIZE_POS_IN_RANGE to EXTRACT_ARGS_IN_RANGE and move > it to rtl.h. OK for the trunk -- just in case you were waiting on an explicit ack of V4. jeff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] PR77822 2016-11-25 8:30 ` Dominik Vogt 2016-11-28 22:29 ` Jeff Law @ 2016-12-02 8:39 ` Andreas Krebbel 1 sibling, 0 replies; 17+ messages in thread From: Andreas Krebbel @ 2016-12-02 8:39 UTC (permalink / raw) To: Dominik Vogt; +Cc: gcc-patches On Fri, Nov 25, 2016 at 09:29:46AM +0100, Dominik Vogt wrote: > gcc/ChangeLog > > PR target/77822 > * rtl.h (EXTRACT_ARGS_IN_RANGE): New. Applied. Thanks! -Andreas- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] PR77822 2016-11-17 15:53 [PATCH 0/2] PR77822 Dominik Vogt 2016-11-17 15:54 ` [PATCH 1/2] PR77822 Dominik Vogt @ 2016-11-17 15:54 ` Dominik Vogt 2016-11-21 11:05 ` [PATCH 2/2 v3] PR77822 Dominik Vogt 1 sibling, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-17 15:54 UTC (permalink / raw) To: gcc-patches, Andreas Krebbel [-- Attachment #1: Type: text/plain, Size: 529 bytes --] On Thu, Nov 17, 2016 at 04:53:03PM +0100, Dominik Vogt wrote: > The following two patches fix PR 77822 on s390x for gcc-7. As the > macro doing the argument range checks can be used on other targets > as well, I've put it in system.h (couldn't think of a better > place; maybe rtl.h?). > > Bootstrapped on s390x biarch, regression tested on s390x biarch > and s390, all on a zEC12 with -march=zEC12. > > Please check the commit messages for details. S390 backend patch. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0002-ChangeLog --] [-- Type: text/plain, Size: 427 bytes --] gcc/ChangeLog * config/s390/s390.md ("extzv") ("*extzv<mode><clobbercc_or_nocc>") ("*extzvdi<clobbercc_or_nocc>_lshiftrt") ("*<risbg_n>_ior_and_sr_ze") ("*extract1bitdi<clobbercc_or_nocc>") ("*insv<mode><clobbercc_or_nocc>", "*insv_rnsbg_noshift") ("*insv_rnsbg_srl", "*insv<mode>_mem_reg") ("*insvdi_mem_reghigh", "*insvdi_reg_imm"): Use SIZE_POS_IN_RANGE to validate the arguments of zero_extract and sign_extract. [-- Attachment #3: 0002-PR-target-77822-S390-Validate-argument-range-of-zero.patch --] [-- Type: text/plain, Size: 4772 bytes --] From 2e20be51e86cde2e02f8f02e0367456d3fe8d92b Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:48 +0100 Subject: [PATCH 2/2] PR target/77822: S390: Validate argument range of {zero,sign}_extract. With some undefined code, combine generates patterns where the arguments to *_extract are out of range, e.b. a negative bit position. If the s390 backend accepts these, they lead to not just undefined behaviour but invalid assembly instructions (argument out of the allowed range). So this patch makes sure that the rtl expressions with out of range arguments are rejected. --- gcc/config/s390/s390.md | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md index fedd6f9..d2dfb1e 100644 --- a/gcc/config/s390/s390.md +++ b/gcc/config/s390/s390.md @@ -3741,6 +3741,8 @@ (clobber (reg:CC CC_REGNUM))])] "TARGET_Z10" { + if (! SIZE_POS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), 64)) + FAIL; /* Starting with zEC12 there is risbgn not clobbering CC. */ if (TARGET_ZEC12) { @@ -3760,7 +3762,9 @@ (match_operand 2 "const_int_operand" "") ; size (match_operand 3 "const_int_operand" ""))) ; start ] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), + GET_MODE_BITSIZE (<MODE>mode))" "<risbg_n>\t%0,%1,64-%2,128+63,<bitoff_plus>%3+%2" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3773,6 +3777,7 @@ (lshiftrt:DI (match_operand:DI 3 "register_operand" "d") (match_operand:DI 4 "nonzero_shift_count_operand" "")))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && 64 - UINTVAL (operands[4]) >= UINTVAL (operands[1])" "<risbg_n>\t%0,%3,%2,%2+%1-1,128-%2-%1-%4" [(set_attr "op_type" "RIE") @@ -3791,6 +3796,7 @@ (match_operand 5 "const_int_operand" "")) ; start 4)))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[4]), INTVAL (operands[5]), 64) && UINTVAL (operands[2]) == (~(0ULL) << UINTVAL (operands[4]))" "<risbg_n>\t%0,%3,64-%4,63,%4+%5" [(set_attr "op_type" "RIE") @@ -3804,7 +3810,8 @@ (const_int 1) ; size (match_operand 2 "const_int_operand" "")) ; start (const_int 0)))] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (1, INTVAL (operands[2]), 64)" "<risbg_n>\t%0,%1,64-1,128+63,%2+1" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3919,6 +3926,8 @@ (match_operand 2 "const_int_operand" "I")) ; pos (match_operand:GPR 3 "nonimmediate_operand" "d"))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), + GET_MODE_BITSIZE (<MODE>mode)) && (INTVAL (operands[1]) + INTVAL (operands[2])) <= <bitsize>" "<risbg_n>\t%0,%3,<bitoff_plus>%2,<bitoff_plus>%2+%1-1,<bitsize>-%2-%1" [(set_attr "op_type" "RIE") @@ -4214,6 +4223,7 @@ (match_operand:DI 3 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[1]) + INTVAL (operands[2]) == 64" "rnsbg\t%0,%3,%2,63,0" [(set_attr "op_type" "RIE")]) @@ -4230,6 +4240,7 @@ (match_operand:DI 4 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[3]) == 64 - INTVAL (operands[1]) - INTVAL (operands[2])" "rnsbg\t%0,%4,%2,%2+%1-1,%3" [(set_attr "op_type" "RIE")]) @@ -4239,7 +4250,8 @@ (match_operand 1 "const_int_operand" "n,n") (const_int 0)) (match_operand:W 2 "register_operand" "d,d"))] - "INTVAL (operands[1]) > 0 + "SIZE_POS_IN_RANGE (INTVAL (operands[1]), 0, 64) + && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" { @@ -4260,6 +4272,7 @@ (lshiftrt:DI (match_operand:DI 2 "register_operand" "d") (const_int 32)))] "TARGET_ZARCH + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), 0, 64) && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" @@ -4278,6 +4291,7 @@ (match_operand 1 "const_int_operand" "n")) (match_operand:DI 2 "const_int_operand" "n"))] "TARGET_ZARCH + && SIZE_POS_IN_RANGE (16, INTVAL (operands[1]), 64) && INTVAL (operands[1]) >= 0 && INTVAL (operands[1]) < BITS_PER_WORD && INTVAL (operands[1]) % 16 == 0" -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2 v3] PR77822 2016-11-17 15:54 ` [PATCH 2/2] PR77822 Dominik Vogt @ 2016-11-21 11:05 ` Dominik Vogt 2016-11-25 8:26 ` Dominik Vogt 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-21 11:05 UTC (permalink / raw) To: gcc-patches, Andreas Krebbel [-- Attachment #1: Type: text/plain, Size: 979 bytes --] On Thu, Nov 17, 2016 at 04:54:17PM +0100, Dominik Vogt wrote: > On Thu, Nov 17, 2016 at 04:53:03PM +0100, Dominik Vogt wrote: > > The following two patches fix PR 77822 on s390x for gcc-7. As the > > macro doing the argument range checks can be used on other targets > > as well, I've put it in system.h (couldn't think of a better > > place; maybe rtl.h?). > > > > Bootstrapped on s390x biarch, regression tested on s390x biarch > > and s390, all on a zEC12 with -march=zEC12. > > > > Please check the commit messages for details. > > S390 backend patch. New version of the patch attached. Changes since the first version of the patchset: v2: Add s390 test cases. Support .cxx tests in s390.exp. Put all arguments of SIZE_POS_IN_RANGE in parentheses. Rewrite SIZE_POS_IN_RANGE macro to handle wrapping SIZE + POS. v3: Rename macro argument from UPPER back to RANGE. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0002-v3-ChangeLog --] [-- Type: text/plain, Size: 621 bytes --] gcc/ChangeLog PR target/77822 * config/s390/s390.md ("extzv") ("*extzv<mode><clobbercc_or_nocc>") ("*extzvdi<clobbercc_or_nocc>_lshiftrt") ("*<risbg_n>_ior_and_sr_ze") ("*extract1bitdi<clobbercc_or_nocc>") ("*insv<mode><clobbercc_or_nocc>", "*insv_rnsbg_noshift") ("*insv_rnsbg_srl", "*insv<mode>_mem_reg") ("*insvdi_mem_reghigh", "*insvdi_reg_imm"): Use SIZE_POS_IN_RANGE to validate the arguments of zero_extract and sign_extract. gcc/testsuite/ChangeLog PR target/77822 * gcc.target/s390/s390.exp: Support .cxx tests. * gcc.target/s390/pr77822-2.c: New test. * gcc.target/s390/pr77822-1.cxx: New test. [-- Attachment #3: 0002-v3-PR-target-77822-S390-Validate-argument-range-of-zero.patch --] [-- Type: text/plain, Size: 10619 bytes --] From 7cdadeef8e47baf66d938f80950dd364cc9371e8 Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:48 +0100 Subject: [PATCH 2/2] PR target/77822: S390: Validate argument range of {zero,sign}_extract. With some undefined code, combine generates patterns where the arguments to *_extract are out of range, e.b. a negative bit position. If the s390 backend accepts these, they lead to not just undefined behaviour but invalid assembly instructions (argument out of the allowed range). So this patch makes sure that the rtl expressions with out of range arguments are rejected. --- gcc/config/s390/s390.md | 20 +- gcc/testsuite/gcc.target/s390/pr77822-1.cxx | 21 ++ gcc/testsuite/gcc.target/s390/pr77822-2.c | 307 ++++++++++++++++++++++++++++ gcc/testsuite/gcc.target/s390/s390.exp | 8 +- 4 files changed, 349 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/s390/pr77822-1.cxx create mode 100644 gcc/testsuite/gcc.target/s390/pr77822-2.c diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md index a449b03..d0664a8 100644 --- a/gcc/config/s390/s390.md +++ b/gcc/config/s390/s390.md @@ -3741,6 +3741,8 @@ (clobber (reg:CC CC_REGNUM))])] "TARGET_Z10" { + if (! SIZE_POS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), 64)) + FAIL; /* Starting with zEC12 there is risbgn not clobbering CC. */ if (TARGET_ZEC12) { @@ -3760,7 +3762,9 @@ (match_operand 2 "const_int_operand" "") ; size (match_operand 3 "const_int_operand" ""))) ; start ] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), + GET_MODE_BITSIZE (<MODE>mode))" "<risbg_n>\t%0,%1,64-%2,128+63,<bitoff_plus>%3+%2" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3773,6 +3777,7 @@ (lshiftrt:DI (match_operand:DI 3 "register_operand" "d") (match_operand:DI 4 "nonzero_shift_count_operand" "")))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && 64 - UINTVAL (operands[4]) >= UINTVAL (operands[1])" "<risbg_n>\t%0,%3,%2,%2+%1-1,128-%2-%1-%4" [(set_attr "op_type" "RIE") @@ -3791,6 +3796,7 @@ (match_operand 5 "const_int_operand" "")) ; start 4)))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[4]), INTVAL (operands[5]), 64) && UINTVAL (operands[2]) == (~(0ULL) << UINTVAL (operands[4]))" "<risbg_n>\t%0,%3,64-%4,63,%4+%5" [(set_attr "op_type" "RIE") @@ -3804,7 +3810,8 @@ (const_int 1) ; size (match_operand 2 "const_int_operand" "")) ; start (const_int 0)))] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (1, INTVAL (operands[2]), 64)" "<risbg_n>\t%0,%1,64-1,128+63,%2+1" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3919,6 +3926,8 @@ (match_operand 2 "const_int_operand" "I")) ; pos (match_operand:GPR 3 "nonimmediate_operand" "d"))] "<z10_or_zEC12_cond> + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), + GET_MODE_BITSIZE (<MODE>mode)) && (INTVAL (operands[1]) + INTVAL (operands[2])) <= <bitsize>" "<risbg_n>\t%0,%3,<bitoff_plus>%2,<bitoff_plus>%2+%1-1,<bitsize>-%2-%1" [(set_attr "op_type" "RIE") @@ -4214,6 +4223,7 @@ (match_operand:DI 3 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[1]) + INTVAL (operands[2]) == 64" "rnsbg\t%0,%3,%2,63,0" [(set_attr "op_type" "RIE")]) @@ -4230,6 +4240,7 @@ (match_operand:DI 4 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[3]) == 64 - INTVAL (operands[1]) - INTVAL (operands[2])" "rnsbg\t%0,%4,%2,%2+%1-1,%3" [(set_attr "op_type" "RIE")]) @@ -4239,7 +4250,8 @@ (match_operand 1 "const_int_operand" "n,n") (const_int 0)) (match_operand:W 2 "register_operand" "d,d"))] - "INTVAL (operands[1]) > 0 + "SIZE_POS_IN_RANGE (INTVAL (operands[1]), 0, 64) + && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" { @@ -4260,6 +4272,7 @@ (lshiftrt:DI (match_operand:DI 2 "register_operand" "d") (const_int 32)))] "TARGET_ZARCH + && SIZE_POS_IN_RANGE (INTVAL (operands[1]), 0, 64) && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" @@ -4278,6 +4291,7 @@ (match_operand 1 "const_int_operand" "n")) (match_operand:DI 2 "const_int_operand" "n"))] "TARGET_ZARCH + && SIZE_POS_IN_RANGE (16, INTVAL (operands[1]), 64) && INTVAL (operands[1]) >= 0 && INTVAL (operands[1]) < BITS_PER_WORD && INTVAL (operands[1]) % 16 == 0" diff --git a/gcc/testsuite/gcc.target/s390/pr77822-1.cxx b/gcc/testsuite/gcc.target/s390/pr77822-1.cxx new file mode 100644 index 0000000..bd5a9b4 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/pr77822-1.cxx @@ -0,0 +1,21 @@ +/* Regression test for PR/77822. */ + +/* { dg-do compile } */ +/* { dg-options "-O3 -march=zEC12" } */ + +class A { + void m_fn1(); + char m_datawidth; + char m_subunits; + int m_subunit_infos[]; +}; +int a; +long b; +void A::m_fn1() { + int c = 32, d = m_datawidth / c; + for (int e = 0; e < d; e++) { + int f = e * 32; + if (b >> f & 1) + m_subunit_infos[m_subunits] = a; + } +} diff --git a/gcc/testsuite/gcc.target/s390/pr77822-2.c b/gcc/testsuite/gcc.target/s390/pr77822-2.c new file mode 100644 index 0000000..6789152 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/pr77822-2.c @@ -0,0 +1,307 @@ +/* This testcase checks that the shift operand of r*sbg instructions is in + range. */ + +/* { dg-do compile } */ +/* { dg-options "-O3 -march=zEC12 -Wno-shift-count-overflow" } */ + +int g; + +void pos_ll_129 (long long b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_ll_134 (long long b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_ll_65 (long long b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_ll_70 (long long b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_ll_33 (long long b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_ll_38 (long long b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_ll_17 (long long b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_ll_22 (long long b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_ll_8 (long long b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_ll_13 (long long b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_l_129 (long b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_l_134 (long b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_l_65 (long b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_l_70 (long b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_l_33 (long b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_l_38 (long b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_l_17 (long b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_l_22 (long b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_l_8 (long b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_l_13 (long b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_i_129 (int b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_i_134 (int b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_i_65 (int b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_i_70 (int b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_i_33 (int b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_i_38 (int b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_i_17 (int b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_i_22 (int b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_i_8 (int b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_i_13 (int b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_s_129 (short b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_s_134 (short b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_s_65 (short b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_s_70 (short b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_s_33 (short b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_s_38 (short b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_s_17 (short b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_s_22 (short b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_s_8 (short b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_s_13 (short b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_c_129 (signed char b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_c_134 (signed char b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_c_65 (signed char b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_c_70 (signed char b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_c_33 (signed char b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_c_38 (signed char b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_c_17 (signed char b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_c_22 (signed char b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_c_8 (signed char b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_c_13 (signed char b) +{ + if (b >> 13 & 1) + g = b; +} diff --git a/gcc/testsuite/gcc.target/s390/s390.exp b/gcc/testsuite/gcc.target/s390/s390.exp index f4ad7a1..69c1f39 100644 --- a/gcc/testsuite/gcc.target/s390/s390.exp +++ b/gcc/testsuite/gcc.target/s390/s390.exp @@ -90,16 +90,16 @@ dg-init set md_tests $srcdir/$subdir/md/*.c # Main loop. -dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \ +dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.{c,S,cxx}] \ $md_tests]] "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*vector*/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*vector*/*.]] \ "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/target-attribute/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/target-attribute/*.{c,S,cxx}]] \ "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/md/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/md/*.{c,S,cxx}]] \ "" $DEFAULT_CFLAGS # Additional hotpatch torture tests. -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2 v3] PR77822 2016-11-21 11:05 ` [PATCH 2/2 v3] PR77822 Dominik Vogt @ 2016-11-25 8:26 ` Dominik Vogt 2016-12-02 8:38 ` Andreas Krebbel 0 siblings, 1 reply; 17+ messages in thread From: Dominik Vogt @ 2016-11-25 8:26 UTC (permalink / raw) To: gcc-patches, Andreas Krebbel [-- Attachment #1: Type: text/plain, Size: 1196 bytes --] On Mon, Nov 21, 2016 at 12:05:42PM +0100, Dominik Vogt wrote: > On Thu, Nov 17, 2016 at 04:54:17PM +0100, Dominik Vogt wrote: > > On Thu, Nov 17, 2016 at 04:53:03PM +0100, Dominik Vogt wrote: > > > The following two patches fix PR 77822 on s390x for gcc-7. As the > > > macro doing the argument range checks can be used on other targets > > > as well, I've put it in system.h (couldn't think of a better > > > place; maybe rtl.h?). > > > > > > Bootstrapped on s390x biarch, regression tested on s390x biarch > > > and s390, all on a zEC12 with -march=zEC12. > > > > > > Please check the commit messages for details. > > > > S390 backend patch. Version 4 of the patchset. Bootstrapped and regression tested on s390 biarch and s390. Changes since the first version of the patchset: v2: Add s390 test cases. Support .cxx tests in s390.exp. Put all arguments of SIZE_POS_IN_RANGE in parentheses. Rewrite SIZE_POS_IN_RANGE macro to handle wrapping SIZE + POS. v3: Rename macro argument from UPPER back to RANGE. v4: Rename SIZE_POS_IN_RANGE to EXTRACT_ARGS_IN_RANGE and move it to rtl.h. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany [-- Attachment #2: 0002-v4-ChangeLog --] [-- Type: text/plain, Size: 627 bytes --] gcc/ChangeLogb PR target/77822 * config/s390/s390.md ("extzv") ("*extzv<mode><clobbercc_or_nocc>") ("*extzvdi<clobbercc_or_nocc>_lshiftrt") ("*<risbg_n>_ior_and_sr_ze") ("*extract1bitdi<clobbercc_or_nocc>") ("*insv<mode><clobbercc_or_nocc>", "*insv_rnsbg_noshift") ("*insv_rnsbg_srl", "*insv<mode>_mem_reg") ("*insvdi_mem_reghigh", "*insvdi_reg_imm"): Use EXTRACT_ARGS_IN_RANGE to validate the arguments of zero_extract and sign_extract. gcc/testsuite/ChangeLogb PR target/77822 * gcc.target/s390/s390.exp: Support .cxx tests. * gcc.target/s390/pr77822-2.c: New test. * gcc.target/s390/pr77822-1.cxx: New test. [-- Attachment #3: 0002-v4-PR-target-77822-S390-Validate-argument-range-of-zero.patch --] [-- Type: text/plain, Size: 10671 bytes --] From bcf508a7e2f90c7e67702c437780cc12a3f74860 Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@linux.vnet.ibm.com> Date: Thu, 17 Nov 2016 14:49:48 +0100 Subject: [PATCH 2/2] PR target/77822: S390: Validate argument range of {zero,sign}_extract. With some undefined code, combine generates patterns where the arguments to *_extract are out of range, e.b. a negative bit position. If the s390 backend accepts these, they lead to not just undefined behaviour but invalid assembly instructions (argument out of the allowed range). So this patch makes sure that the rtl expressions with out of range arguments are rejected. --- gcc/config/s390/s390.md | 20 +- gcc/testsuite/gcc.target/s390/pr77822-1.cxx | 21 ++ gcc/testsuite/gcc.target/s390/pr77822-2.c | 307 ++++++++++++++++++++++++++++ gcc/testsuite/gcc.target/s390/s390.exp | 8 +- 4 files changed, 349 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.target/s390/pr77822-1.cxx create mode 100644 gcc/testsuite/gcc.target/s390/pr77822-2.c diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md index a449b03..aaf8427 100644 --- a/gcc/config/s390/s390.md +++ b/gcc/config/s390/s390.md @@ -3741,6 +3741,8 @@ (clobber (reg:CC CC_REGNUM))])] "TARGET_Z10" { + if (! EXTRACT_ARGS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), 64)) + FAIL; /* Starting with zEC12 there is risbgn not clobbering CC. */ if (TARGET_ZEC12) { @@ -3760,7 +3762,9 @@ (match_operand 2 "const_int_operand" "") ; size (match_operand 3 "const_int_operand" ""))) ; start ] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[2]), INTVAL (operands[3]), + GET_MODE_BITSIZE (<MODE>mode))" "<risbg_n>\t%0,%1,64-%2,128+63,<bitoff_plus>%3+%2" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3773,6 +3777,7 @@ (lshiftrt:DI (match_operand:DI 3 "register_operand" "d") (match_operand:DI 4 "nonzero_shift_count_operand" "")))] "<z10_or_zEC12_cond> + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && 64 - UINTVAL (operands[4]) >= UINTVAL (operands[1])" "<risbg_n>\t%0,%3,%2,%2+%1-1,128-%2-%1-%4" [(set_attr "op_type" "RIE") @@ -3791,6 +3796,7 @@ (match_operand 5 "const_int_operand" "")) ; start 4)))] "<z10_or_zEC12_cond> + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[4]), INTVAL (operands[5]), 64) && UINTVAL (operands[2]) == (~(0ULL) << UINTVAL (operands[4]))" "<risbg_n>\t%0,%3,64-%4,63,%4+%5" [(set_attr "op_type" "RIE") @@ -3804,7 +3810,8 @@ (const_int 1) ; size (match_operand 2 "const_int_operand" "")) ; start (const_int 0)))] - "<z10_or_zEC12_cond>" + "<z10_or_zEC12_cond> + && EXTRACT_ARGS_IN_RANGE (1, INTVAL (operands[2]), 64)" "<risbg_n>\t%0,%1,64-1,128+63,%2+1" ; dst, src, start, end, shift [(set_attr "op_type" "RIE") (set_attr "z10prop" "z10_super_E1")]) @@ -3919,6 +3926,8 @@ (match_operand 2 "const_int_operand" "I")) ; pos (match_operand:GPR 3 "nonimmediate_operand" "d"))] "<z10_or_zEC12_cond> + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), + GET_MODE_BITSIZE (<MODE>mode)) && (INTVAL (operands[1]) + INTVAL (operands[2])) <= <bitsize>" "<risbg_n>\t%0,%3,<bitoff_plus>%2,<bitoff_plus>%2+%1-1,<bitsize>-%2-%1" [(set_attr "op_type" "RIE") @@ -4214,6 +4223,7 @@ (match_operand:DI 3 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[1]) + INTVAL (operands[2]) == 64" "rnsbg\t%0,%3,%2,63,0" [(set_attr "op_type" "RIE")]) @@ -4230,6 +4240,7 @@ (match_operand:DI 4 "nonimmediate_operand" "d"))) (clobber (reg:CC CC_REGNUM))] "TARGET_Z10 + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), INTVAL (operands[2]), 64) && INTVAL (operands[3]) == 64 - INTVAL (operands[1]) - INTVAL (operands[2])" "rnsbg\t%0,%4,%2,%2+%1-1,%3" [(set_attr "op_type" "RIE")]) @@ -4239,7 +4250,8 @@ (match_operand 1 "const_int_operand" "n,n") (const_int 0)) (match_operand:W 2 "register_operand" "d,d"))] - "INTVAL (operands[1]) > 0 + "EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), 0, 64) + && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" { @@ -4260,6 +4272,7 @@ (lshiftrt:DI (match_operand:DI 2 "register_operand" "d") (const_int 32)))] "TARGET_ZARCH + && EXTRACT_ARGS_IN_RANGE (INTVAL (operands[1]), 0, 64) && INTVAL (operands[1]) > 0 && INTVAL (operands[1]) <= GET_MODE_BITSIZE (SImode) && INTVAL (operands[1]) % BITS_PER_UNIT == 0" @@ -4278,6 +4291,7 @@ (match_operand 1 "const_int_operand" "n")) (match_operand:DI 2 "const_int_operand" "n"))] "TARGET_ZARCH + && EXTRACT_ARGS_IN_RANGE (16, INTVAL (operands[1]), 64) && INTVAL (operands[1]) >= 0 && INTVAL (operands[1]) < BITS_PER_WORD && INTVAL (operands[1]) % 16 == 0" diff --git a/gcc/testsuite/gcc.target/s390/pr77822-1.cxx b/gcc/testsuite/gcc.target/s390/pr77822-1.cxx new file mode 100644 index 0000000..bd5a9b4 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/pr77822-1.cxx @@ -0,0 +1,21 @@ +/* Regression test for PR/77822. */ + +/* { dg-do compile } */ +/* { dg-options "-O3 -march=zEC12" } */ + +class A { + void m_fn1(); + char m_datawidth; + char m_subunits; + int m_subunit_infos[]; +}; +int a; +long b; +void A::m_fn1() { + int c = 32, d = m_datawidth / c; + for (int e = 0; e < d; e++) { + int f = e * 32; + if (b >> f & 1) + m_subunit_infos[m_subunits] = a; + } +} diff --git a/gcc/testsuite/gcc.target/s390/pr77822-2.c b/gcc/testsuite/gcc.target/s390/pr77822-2.c new file mode 100644 index 0000000..6789152 --- /dev/null +++ b/gcc/testsuite/gcc.target/s390/pr77822-2.c @@ -0,0 +1,307 @@ +/* This testcase checks that the shift operand of r*sbg instructions is in + range. */ + +/* { dg-do compile } */ +/* { dg-options "-O3 -march=zEC12 -Wno-shift-count-overflow" } */ + +int g; + +void pos_ll_129 (long long b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_ll_134 (long long b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_ll_65 (long long b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_ll_70 (long long b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_ll_33 (long long b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_ll_38 (long long b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_ll_17 (long long b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_ll_22 (long long b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_ll_8 (long long b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_ll_13 (long long b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_l_129 (long b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_l_134 (long b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_l_65 (long b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_l_70 (long b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_l_33 (long b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_l_38 (long b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_l_17 (long b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_l_22 (long b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_l_8 (long b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_l_13 (long b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_i_129 (int b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_i_134 (int b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_i_65 (int b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_i_70 (int b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_i_33 (int b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_i_38 (int b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_i_17 (int b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_i_22 (int b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_i_8 (int b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_i_13 (int b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_s_129 (short b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_s_134 (short b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_s_65 (short b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_s_70 (short b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_s_33 (short b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_s_38 (short b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_s_17 (short b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_s_22 (short b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_s_8 (short b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_s_13 (short b) +{ + if (b >> 13 & 1) + g = b; +} + +void pos_c_129 (signed char b) +{ + if (b >> 129 & 1) + g = b; +} + +void sizepos_c_134 (signed char b) +{ + if (b >> 134 & 1) + g = b; +} + +void pos_c_65 (signed char b) +{ + if (b >> 65 & 1) + g = b; +} + +void sizepos_c_70 (signed char b) +{ + if (b >> 70 & 1) + g = b; +} + +void pos_c_33 (signed char b) +{ + if (b >> 33 & 1) + g = b; +} + +void sizepos_c_38 (signed char b) +{ + if (b >> 38 & 1) + g = b; +} + +void pos_c_17 (signed char b) +{ + if (b >> 17 & 1) + g = b; +} + +void sizepos_c_22 (signed char b) +{ + if (b >> 22 & 1) + g = b; +} + +void pos_c_8 (signed char b) +{ + if (b >> 8 & 1) + g = b; +} + +void sizepos_c_13 (signed char b) +{ + if (b >> 13 & 1) + g = b; +} diff --git a/gcc/testsuite/gcc.target/s390/s390.exp b/gcc/testsuite/gcc.target/s390/s390.exp index f4ad7a1..69c1f39 100644 --- a/gcc/testsuite/gcc.target/s390/s390.exp +++ b/gcc/testsuite/gcc.target/s390/s390.exp @@ -90,16 +90,16 @@ dg-init set md_tests $srcdir/$subdir/md/*.c # Main loop. -dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \ +dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.{c,S,cxx}] \ $md_tests]] "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*vector*/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*vector*/*.]] \ "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/target-attribute/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/target-attribute/*.{c,S,cxx}]] \ "" $DEFAULT_CFLAGS -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/md/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/md/*.{c,S,cxx}]] \ "" $DEFAULT_CFLAGS # Additional hotpatch torture tests. -- 2.3.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2 v3] PR77822 2016-11-25 8:26 ` Dominik Vogt @ 2016-12-02 8:38 ` Andreas Krebbel 0 siblings, 0 replies; 17+ messages in thread From: Andreas Krebbel @ 2016-12-02 8:38 UTC (permalink / raw) To: Dominik Vogt; +Cc: gcc-patches On Fri, Nov 25, 2016 at 09:25:59AM +0100, Dominik Vogt wrote: > gcc/ChangeLogb > > PR target/77822 > * config/s390/s390.md ("extzv") > ("*extzv<mode><clobbercc_or_nocc>") > ("*extzvdi<clobbercc_or_nocc>_lshiftrt") > ("*<risbg_n>_ior_and_sr_ze") > ("*extract1bitdi<clobbercc_or_nocc>") > ("*insv<mode><clobbercc_or_nocc>", "*insv_rnsbg_noshift") > ("*insv_rnsbg_srl", "*insv<mode>_mem_reg") > ("*insvdi_mem_reghigh", "*insvdi_reg_imm"): Use EXTRACT_ARGS_IN_RANGE > to validate the arguments of zero_extract and sign_extract. > gcc/testsuite/ChangeLogb > > PR target/77822 > * gcc.target/s390/s390.exp: Support .cxx tests. > * gcc.target/s390/pr77822-2.c: New test. > * gcc.target/s390/pr77822-1.cxx: New test. Applied with testcase .cxx renamed to .C. -Andreas- ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-12-02 8:39 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-17 15:53 [PATCH 0/2] PR77822 Dominik Vogt 2016-11-17 15:54 ` [PATCH 1/2] PR77822 Dominik Vogt 2016-11-18 9:31 ` Segher Boessenkool 2016-11-18 12:09 ` Dominik Vogt 2016-11-18 14:02 ` Segher Boessenkool 2016-11-18 15:29 ` Dominik Vogt 2016-11-21 11:04 ` [PATCH 1/2 v3] PR77822 Dominik Vogt 2016-11-23 20:22 ` Jeff Law 2016-11-24 9:59 ` Dominik Vogt 2016-11-24 15:33 ` Jeff Law 2016-11-25 8:30 ` Dominik Vogt 2016-11-28 22:29 ` Jeff Law 2016-12-02 8:39 ` Andreas Krebbel 2016-11-17 15:54 ` [PATCH 2/2] PR77822 Dominik Vogt 2016-11-21 11:05 ` [PATCH 2/2 v3] PR77822 Dominik Vogt 2016-11-25 8:26 ` Dominik Vogt 2016-12-02 8:38 ` Andreas Krebbel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).