[PATCH 0/2] PR77822

[PATCH 0/2] PR77822

[Dominik Vogt]

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

Re: [PATCH 1/2] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 1/2] PR77822

[Segher Boessenkool]

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

Re: [PATCH 1/2] PR77822

[Dominik Vogt]

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

Re: [PATCH 1/2] PR77822

[Segher Boessenkool]

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

Re: [PATCH 1/2] PR77822

[Dominik Vogt]

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

Re: [PATCH 1/2 v3] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 1/2 v3] PR77822

[Jeff Law]

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

Re: [PATCH 1/2 v3] PR77822

[Dominik Vogt]

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

Re: [PATCH 1/2 v3] PR77822

[Jeff Law]

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

Re: [PATCH 1/2 v3] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 1/2 v3] PR77822

[Jeff Law]

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

Re: [PATCH 1/2 v3] PR77822

[Andreas Krebbel]

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-

Re: [PATCH 2/2] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 2/2 v3] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 2/2 v3] PR77822

[Dominik Vogt]

[-- 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

Re: [PATCH 2/2 v3] PR77822

[Andreas Krebbel]

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-