df: Fix up handling of paradoxical subregs in debug insns [PR101170]

Hi!

The recent addition of gcc_assert (regno < endregno); triggers during
glibc build on m68k.
The problem is that RA decisions shouldn't depend on expressions in
DEBUG_INSNs and those expressions can contain paradoxical subregs of certain
pseudos.  If RA then decides to allocate the pseudo to a register
with very small hard register REGNO, we can trigger the new assert,
as (int) subreg_regno_offset may be negative on big endian and the small
REGNO + the negative offset can wrap around.

The following patch in that case records the range from the REGNO 0 to
endregno, before the addition of the assert as both regno and endregno are
unsigned it wouldn't record anything at all silently.

Bootstrapped/regtested on x86_64-linux and i686-linux and tested with a
cross compiler to m68k-liux on the testcase, ok for trunk?

2021-06-24  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/101170
	* df-scan.c (df_ref_record): For paradoxical big-endian SUBREGs
	where regno + subreg_regno_offset wraps around use 0 as starting
	regno.

	* gcc.dg/pr101170.c: New test.


	Jakub

On Thu, 24 Jun 2021, Jakub Jelinek wrote:

> Hi!
> 
> The recent addition of gcc_assert (regno < endregno); triggers during
> glibc build on m68k.
> The problem is that RA decisions shouldn't depend on expressions in
> DEBUG_INSNs and those expressions can contain paradoxical subregs of certain
> pseudos.  If RA then decides to allocate the pseudo to a register
> with very small hard register REGNO, we can trigger the new assert,
> as (int) subreg_regno_offset may be negative on big endian and the small
> REGNO + the negative offset can wrap around.

Hm, I wonder if we should reset the debug_insn if the RA made a decision
that produces such non-sensical result for a debug_insn use?

> The following patch in that case records the range from the REGNO 0 to
> endregno, before the addition of the assert as both regno and endregno are
> unsigned it wouldn't record anything at all silently.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux and tested with a
> cross compiler to m68k-liux on the testcase, ok for trunk?

OK.

Thanks,
Richard.

> 2021-06-24  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/101170
> 	* df-scan.c (df_ref_record): For paradoxical big-endian SUBREGs
> 	where regno + subreg_regno_offset wraps around use 0 as starting
> 	regno.
> 
> 	* gcc.dg/pr101170.c: New test.
> 
> --- gcc/df-scan.c.jj	2021-06-22 10:04:46.371208994 +0200
> +++ gcc/df-scan.c	2021-06-23 12:46:51.654678805 +0200
> @@ -2576,9 +2576,21 @@ df_ref_record (enum df_ref_class cl,
>  
>        if (GET_CODE (reg) == SUBREG)
>  	{
> -	  regno += subreg_regno_offset (regno, GET_MODE (SUBREG_REG (reg)),
> -					SUBREG_BYTE (reg), GET_MODE (reg));
> -	  endregno = regno + subreg_nregs (reg);
> +	  int off = subreg_regno_offset (regno, GET_MODE (SUBREG_REG (reg)),
> +					 SUBREG_BYTE (reg), GET_MODE (reg));
> +	  unsigned int nregno = regno + off;
> +	  endregno = nregno + subreg_nregs (reg);
> +	  if (off < 0 && regno < (unsigned) -off)
> +	    /* Deal with paradoxical SUBREGs on big endian where
> +	       in debug insns the hard reg number might be smaller
> +	       than -off, such as (subreg:DI (reg:SI 0 [+4 ]) 0));
> +	       RA decisions shouldn't be affected by debug insns
> +	       and so RA can decide to put pseudo into a hard reg
> +	       with small REGNO, even when it is referenced in
> +	       a paradoxical SUBREG in a debug insn.  */
> +	    regno = 0;
> +	  else
> +	    regno = nregno;
>  	}
>        else
>  	endregno = END_REGNO (reg);
> --- gcc/testsuite/gcc.dg/pr101170.c.jj	2021-06-23 12:27:08.866593960 +0200
> +++ gcc/testsuite/gcc.dg/pr101170.c	2021-06-23 12:26:55.823769555 +0200
> @@ -0,0 +1,37 @@
> +/* PR middle-end/101170 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +#include <stdarg.h>
> +
> +struct S { int a; int b[4]; } s;
> +va_list ap;
> +int i;
> +long long l;
> +
> +struct S
> +foo (int x)
> +{
> +  struct S a = {};
> +  do
> +    if (x)
> +      return a;
> +  while (1);
> +}
> +
> +__attribute__((noipa)) void
> +bar (void)
> +{
> +  for (; i; i++)
> +    l |= va_arg (ap, long long) << s.b[i];
> +  if (l)
> +    foo (l);
> +}
> +
> +void
> +baz (int v, ...)
> +{
> +  va_start (ap, v);
> +  bar ();
> +  va_end (ap);
> +}
> 
> 	Jakub
> 
>

On Thu, Jun 24, 2021 at 12:13:18PM +0200, Richard Biener wrote:
> > The recent addition of gcc_assert (regno < endregno); triggers during
> > glibc build on m68k.
> > The problem is that RA decisions shouldn't depend on expressions in
> > DEBUG_INSNs and those expressions can contain paradoxical subregs of certain
> > pseudos.  If RA then decides to allocate the pseudo to a register
> > with very small hard register REGNO, we can trigger the new assert,
> > as (int) subreg_regno_offset may be negative on big endian and the small
> > REGNO + the negative offset can wrap around.
> 
> Hm, I wonder if we should reset the debug_insn if the RA made a decision
> that produces such non-sensical result for a debug_insn use?

For debug info purposes it isn't necessarily invalid.
If we extract later on only the well defined bits from the paradoxical
subreg (e.g. by using a lowpart subreg of the debug expr), or AND it with
a constant mask that only contains bits on the low part positions, then it
is fine.  And otherwise, it is useless for debug info anyway, we can't
express that in DWARF (low bits set to ..., high bits undefined) - there
the whole thing would be undefined.

	Jakub