[Bug c/103255] New: [10 Regression] optimization breaks address of struct member

[Bug c/103255] New: [10 Regression] optimization breaks address of struct member

[lipnitsk at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

            Bug ID: 103255
           Summary: [10 Regression] optimization breaks address of struct
                    member
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lipnitsk at gmail dot com
  Target Milestone: ---

---CODE---
#include <stdio.h>
#include <stdint.h>

struct header {
        uint32_t a;
        uint32_t b;
        uint32_t c;
};

int main(void)
{
        struct header *hdr = NULL;
        unsigned long ofs;

        for (ofs = 0x20; ofs <= 0x20; ofs += 0x1000) {
                struct header *tmp;
                tmp = (struct header *)(0x555555558060 + ofs);
                printf("a: 0x%08x", tmp->a);
                if (tmp->a == 0) {
                        hdr = tmp;
                        break;
                }
        }

        if (hdr == NULL)
                return 0;

        printf("found at %p %p %p\n", hdr, &hdr->b, &hdr->c);
        return 0;
}
---END CODE---

After disabling ASLR and compiling with -Os (ensure that 0x555555558080 is
valid on your system, or just look at assembly output) I get:

a: 0x00000000found at 0x555555558080 0x555555558080 0x555555558080

This reproduces on trunk[0]

This reproduces on multiple targets, including MIPS and AARCH64[1]. This has
been linked to -ftree-vrp on AARCH64[2].

[0]: https://godbolt.org/z/KPor6qf34
[1]: https://github.com/openwrt/openwrt/pull/4732#issuecomment-968412881
[2]: https://github.com/openwrt/openwrt/pull/4732#issuecomment-968901545

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.3
   Last reconfirmed|                            |2021-11-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
Here is a testcase which does not need any (predefined location) memory access
to get the failure:
struct header {
        unsigned a;
        unsigned b;
        unsigned c;
};

#define ADDR 0x400000
#define OFF 0x20

int main(void)
{
        struct header *hdr = 0;
        unsigned long ofs;
    volatile int t = 1;

        for (ofs = OFF; ofs <= OFF; ofs += 0x1000) {
                struct header *tmp;
                tmp = (struct header *)(ADDR + ofs);
                if (t) {
                        hdr = tmp;
                        break;
                }
        }

        if (hdr == 0)
                return 0;
    unsigned *tt = &hdr->b;
    if ((__SIZE_TYPE__)tt != (ADDR + OFF + __builtin_offsetof(struct header,
b)))
      __builtin_abort();
        return 0;
}

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member

[lipnitsk at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

--- Comment #2 from Ilya Lipnitskiy <lipnitsk at gmail dot com> ---
Thanks for coming up with a better test case!

Looks like tree-vrp is related for x64 as well[0]. -Os -fno-tree-vrp or -O2
-fno-tree-vrp appears to work-around the problem.

[0]: https://github.com/openwrt/openwrt/pull/4732#issuecomment-969194808

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12 Regression]          |[11/12 Regression]
                   |optimization breaks address |optimization breaks address
                   |of struct member            |of struct member since
                   |                            |r11-4984-g47923622c663ffad
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |amacleod at redhat dot com,
                   |                            |marxin at gcc dot gnu.org

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r11-4984-g47923622c663ffad.

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[nbd at nbd dot name]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Felix Fietkau <nbd at nbd dot name> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nbd at nbd dot name

--- Comment #4 from Felix Fietkau <nbd at nbd dot name> ---
(In reply to Martin Liška from comment #3)
> Started with r11-4984-g47923622c663ffad.
A revert of that commit in 11.2.0 fixes the issue for me.

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is in:
      else if (flag_delete_null_pointer_checks
               && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr)))
        {
         /* For -fdelete-null-pointer-checks -fno-wrapv-pointer we don't
         allow going from non-NULL pointer to NULL.  */
           if(!range_includes_zero_p (&r))
            return true;
        }

If !off_cst (not this case), I think before return true here we need
r = range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt)));
- we are going from the base pointer which is known not to be NULL to that +
some unknown offset, all we can say is that the result is not NULL, but we
don't really know the exact details.
For the off_cst case (probably only when it is actually constant, i.e. off_cst
&&  off.to_constant (&offi), we could use something based on r, but will need
to add the offi / BITS_PER_UNIT addend to the range, probably verify the result
doesn't include zero and if it would, use that range_nonzero too.

For the addition, not sure if that would be range_op_handler (PLUS_EXPR,
type)->fold_range or range_op_handler (POINTER_PLUS_EXPR, type)->fold_range or
what.

Because, with the code as is, we end up with the original r for the base
hdr_3: struct header * [4194336B, 4194336B]
and instead of returning range unsigned int * [4194340B, 4194340B] we return
incorrect unsigned int * [4194336B, 4194336B]

Or does this code rely on pointer ranges to be always just range_zero,
range_nonzero or varying and nothing else?  If so, it should normalize
INTEGER_CSTs used in addresses etc. into range_zero or range_nonzero and
nothing else...

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

--- Comment #6 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Jakub Jelinek from comment #5)
> I think the bug is in:
>       else if (flag_delete_null_pointer_checks
>                && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr)))
>         {
>          /* For -fdelete-null-pointer-checks -fno-wrapv-pointer we don't
>          allow going from non-NULL pointer to NULL.  */
>            if(!range_includes_zero_p (&r))
>             return true;
>         }
> 
> If !off_cst (not this case), I think before return true here we need
> r = range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt)));
> - we are going from the base pointer which is known not to be NULL to that +
> some unknown offset, all we can say is that the result is not NULL, but we
> don't really know the exact details.
> For the off_cst case (probably only when it is actually constant, i.e.
> off_cst &&  off.to_constant (&offi), we could use something based on r, but
> will need to add the offi / BITS_PER_UNIT addend to the range, probably
> verify the result doesn't include zero and if it would, use that
> range_nonzero too.
> 
> For the addition, not sure if that would be range_op_handler (PLUS_EXPR,
> type)->fold_range or range_op_handler (POINTER_PLUS_EXPR, type)->fold_range
> or what.
> 
> Because, with the code as is, we end up with the original r for the base
> hdr_3: struct header * [4194336B, 4194336B]
> and instead of returning range unsigned int * [4194340B, 4194340B] we return
> incorrect unsigned int * [4194336B, 4194336B]
> 
> Or does this code rely on pointer ranges to be always just range_zero,
> range_nonzero or varying and nothing else?  If so, it should normalize
> INTEGER_CSTs used in addresses etc. into range_zero or range_nonzero and
> nothing else...

That code is an import from vr-values, and in gcc 10 it did just what we do:
          if ((off_cst && known_eq (off, 0))
              || (flag_delete_null_pointer_checks
                  && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))))
            {
              const value_range_equiv *vr
                = get_value_range (TREE_OPERAND (base, 0));
              if (!range_includes_zero_p (vr))
                return true;
            }

Perhaps it later sanitizes it to non-zero...  Ah, indeed:

else if (vrp_stmt_computes_nonzero (stmt))
    {
      vr->set_nonzero (type);
      vr->equiv_clear ();
    }


nothing is suppose to depend on the zero/nonzero directly. Pointer tracking is
a little schizophrenic.  Sometimes it just tracks zero/nonzero, other times it
uses integral calculations to come up with ranges.. especially if its gone back
or forth to an integral value via casting. 

If we know its a constant, and we know the offset is a constant, then there
would be no harm in doing the math and coming up with the right value...  just
as you suggest, with a check to make sure if zero has reappeared that we remove
it.   

Or we could restore previous behaviour if we simply changed it to non-zero as
well before returning true.

Either would be correct/fine.

[Bug tree-optimization/103255] [11/12 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:c39cb6bf835ca12e590eaa6f90222e51be207c50

commit r12-5336-gc39cb6bf835ca12e590eaa6f90222e51be207c50
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Nov 17 13:45:53 2021 +0100

    ranger: Fix up fold_using_range::range_of_address [PR103255]

    If on &base->member the offset isn't constant or isn't zero and
    -fdelete-null-pointer-checks and not -fwrapv-pointer and base has a range
    that doesn't include NULL, we return the range of the base.
    Usually it isn't a big deal, because for most pointers we just use
    varying, range_zero and range_nonzero ranges and nothing beyond that,
    but if a pointer is initialized from a constant, we actually track the
    exact range and in that case this causes miscompilation.
    As discussed on IRC, I think doing something like:
                  offset_int off2;
                  if (off_cst && off.is_constant (&off2))
                    {
                      tree cst = wide_int_to_tree (sizetype, off2 /
BITS_PER_UNIT);
                      // adjust range r with POINTER_PLUS_EXPR cst
                      if (!range_includes_zero_p (&r))
                        return true;
                    }
                  // Fallback
                  r = range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt)));
                  return true;
    could work, given that most of the pointer ranges are just the simple ones
    perhaps it is too much for little benefit.

    2021-11-17  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/103255
            * gimple-range-fold.cc (fold_using_range::range_of_address): Return
            range_nonzero rather than unadjusted base's range.  Formatting
fixes.

            * gcc.c-torture/execute/pr103255.c: New test.

[Bug tree-optimization/103255] [11 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12 Regression]          |[11 Regression]
                   |optimization breaks address |optimization breaks address
                   |of struct member since      |of struct member since
                   |r11-4984-g47923622c663ffad  |r11-4984-g47923622c663ffad

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug tree-optimization/103255] [11 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a6219e8e0719b14f474b0dcaa7bde2f4e57474f9

commit r11-9332-ga6219e8e0719b14f474b0dcaa7bde2f4e57474f9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Nov 17 13:45:53 2021 +0100

    ranger: Fix up fold_using_range::range_of_address [PR103255]

    If on &base->member the offset isn't constant or isn't zero and
    -fdelete-null-pointer-checks and not -fwrapv-pointer and base has a range
    that doesn't include NULL, we return the range of the base.
    Usually it isn't a big deal, because for most pointers we just use
    varying, range_zero and range_nonzero ranges and nothing beyond that,
    but if a pointer is initialized from a constant, we actually track the
    exact range and in that case this causes miscompilation.
    As discussed on IRC, I think doing something like:
                  offset_int off2;
                  if (off_cst && off.is_constant (&off2))
                    {
                      tree cst = wide_int_to_tree (sizetype, off2 /
BITS_PER_UNIT);
                      // adjust range r with POINTER_PLUS_EXPR cst
                      if (!range_includes_zero_p (&r))
                        return true;
                    }
                  // Fallback
                  r = range_nonzero (TREE_TYPE (gimple_assign_rhs1 (stmt)));
                  return true;
    could work, given that most of the pointer ranges are just the simple ones
    perhaps it is too much for little benefit.

    2021-11-17  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/103255
            * gimple-range.cc (fold_using_range::range_of_address): Return
            range_nonzero rather than unadjusted base's range.  Formatting
fixes.

            * gcc.c-torture/execute/pr103255.c: New test.

    (cherry picked from commit c39cb6bf835ca12e590eaa6f90222e51be207c50)

[Bug tree-optimization/103255] [11 Regression] optimization breaks address of struct member since r11-4984-g47923622c663ffad

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103255

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.3 too.