[Bug c/94130] New: Unintended result with optimization option when assigning two structures, memset and 0

[Bug c/94130] New: Unintended result with optimization option when assigning two structures, memset and 0

[minzkn at minzkn dot com]

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

            Bug ID: 94130
           Summary: Unintended result with optimization option when
                    assigning two structures, memset and 0
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: minzkn at minzkn dot com
  Target Milestone: ---

Created attachment 48014
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48014&action=edit
test source

It seems that there is an optimization bug (-O1) for gcc version 7.4.0 and
above optimization level 1. (Same as gcc 7.5, it seems to occur from 7.x to
latest 9.2.)

The following verification code was written shortly and when compiled with
optimization options such as -O1, -O2, -O3, and -Os instead of -O0, it was
confirmed that the correct result was not produced.

/ *
1. Assign memset to the member of the first structure along with the address of
the second structure.
2. Initialize the second structure to the desired member value. Only reset to 0
3. Call the first structure by passing it to the dump function. This is not
what you want.
4. Put a memory barrier between steps 1 and 2 to avoid the problem.
* /

#include <stdio.h>
#include <memory.h>

struct myreq {
    char m_name[6];
    void *m_data;
};

struct myvalue {
    unsigned int m_type;
    unsigned int m_value;
};

#define mytest_type 1234
#define mytest_value 0 /* only zero case issue */

#define mytest_barrier() __asm__ __volatile__("": : :"memory")

static void myprint(struct myreq *s_myreq)
{
    struct myvalue *s_myvalue = (struct myvalue *)s_myreq->m_data;

    printf(
        "%s: n=%s, cmd=%u, data=%u\n",
        ((s_myvalue->m_type == mytest_type) && (s_myvalue->m_value ==
mytest_value)) ? "NO BUG" : "!! BUG",
        s_myreq->m_name,
        s_myvalue->m_type,
        s_myvalue->m_value
    );
}

static void mytest_with_barrier(void)
{
    struct myreq s_myreq;
    struct myvalue s_myvalue;

    memset(&s_myreq, 0, sizeof(s_myreq));
    strncpy(s_myreq.m_name, "Hello", sizeof(s_myreq.m_name));

    s_myreq.m_data = memset(&s_myvalue, 0, sizeof(s_myvalue));
    /* memory barrier */ mytest_barrier();
    s_myvalue.m_type = mytest_type;
    s_myvalue.m_value = mytest_value;

    myprint(&s_myreq);
}

static void mytest_without_barrier(void)
{
    struct myreq s_myreq;
    struct myvalue s_myvalue;

    memset(&s_myreq, 0, sizeof(s_myreq));
    strncpy(s_myreq.m_name, "Hello", sizeof(s_myreq.m_name));

    s_myreq.m_data = memset(&s_myvalue, 0, sizeof(s_myvalue));
    s_myvalue.m_type = mytest_type;
    s_myvalue.m_value = mytest_value;

    myprint(&s_myreq);
}

int main(void)
{
    mytest_with_barrier();
    mytest_without_barrier();

    return(0);
}


>>> "gcc -O2" OUTPUT
NO BUG: n=Hello, cmd=1234, data=0
!! BUG: n=Hello, cmd=0, data=1819043144

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |8.5
   Last reconfirmed|                            |2020-03-11
      Known to work|                            |6.5.0
     Ever confirmed|0                           |1
           Priority|P3                          |P2
            Summary|Unintended result with      |[8/9/10 Regression]
                   |optimization option when    |Unintended result with
                   |assigning two structures,   |optimization option when
                   |memset and 0                |assigning two structures,
                   |                            |memset and 0
      Known to fail|                            |7.5.0, 8.3.0, 9.2.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  DSE prunes the memset from the wrong side it seems.  Jeff?

Breaks with -O -fno-inline already, fixed with -fno-tree-dse.

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Doing

    memset(&s_myvalue, 0, sizeof(s_myvalue));
    s_myreq.m_data = &s_myvalue;

also works around the issue.  Odd.

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r7-5965-g7c9560a578a06125cb30458a26605f91feb29b0d

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly reduced testcase:
struct A { char a[6]; void *b; };
struct B { unsigned int c, d; };

static void __attribute__((noipa))
foo (struct A *x)
{
  struct B *b = (struct B *) x->b;
  if (b->c != 1234 || b->d)
    __builtin_abort ();
}

int
main ()
{
  struct A a;
  struct B b;
  __builtin_memset (&a, 0, sizeof (a));
  __builtin_strncpy (a.a, "Hello", sizeof (a.a));
  a.b = __builtin_memset (&b, 0, sizeof (b));
  b.c = 1234;
  b.d = 0;
  foo (&a);
  return 0;
}

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And the b.d = 0; store isn't needed either.

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Even simpler testcase:
int
main ()
{
  int a[8];
  char *b = __builtin_memset (a, 0, sizeof (a));
  a[0] = 1;
  a[1] = 2;
  a[2] = 3;
  if (b != (char *) a)
    __builtin_abort ();
  else
    asm volatile ("" : : "g" (a) : "memory");
  return 0;
}

[Bug tree-optimization/94130] [8/9/10 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48018
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48018&action=edit
gcc10-pr94130.patch

Untested fix.

[Bug tree-optimization/94130] [8/9 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.0
            Summary|[8/9/10 Regression]         |[8/9 Regression] Unintended
                   |Unintended result with      |result with optimization
                   |optimization option when    |option when assigning two
                   |assigning two structures,   |structures, memset and 0
                   |memset and 0                |

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

[Bug tree-optimization/94130] [8/9 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[marxin at gcc dot gnu.org]

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

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

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r10-7140-g349ab34dc64a10fe0b1eda66d13b62862878b73e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 12 09:34:00 2020 +0100

    tree-dse: Fix mem* head trimming if call has lhs [PR94130]

    As the testcase shows, if DSE decides to head trim
{mem{set,cpy,move},strncpy}
    and the call has lhs, it is incorrect to leave the lhs as is, because it
    will then point to the adjusted address (base + head_trim) instead of the
    original base.
    The following patch fixes that by dropping the lhs of the call and
assigning
    lhs the original base in a following statement.

    2020-03-12  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/94130
            * tree-ssa-dse.c: Include gimplify.h.
            (increment_start_addr): If stmt has lhs, drop the lhs from call and
            set it after the call to the original value of the first argument.
            Formatting fixes.
            (decrement_count): Formatting fix.

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

[Bug tree-optimization/94130] [8/9 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r9-8392-ga545ffafa380fa958393e1dfbf7f5f8f129bc5cf
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 12 09:34:00 2020 +0100

    tree-dse: Fix mem* head trimming if call has lhs [PR94130]

    As the testcase shows, if DSE decides to head trim
{mem{set,cpy,move},strncpy}
    and the call has lhs, it is incorrect to leave the lhs as is, because it
    will then point to the adjusted address (base + head_trim) instead of the
    original base.
    The following patch fixes that by dropping the lhs of the call and
assigning
    lhs the original base in a following statement.

    2020-03-12  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/94130
            * tree-ssa-dse.c: Include gimplify.h.
            (increment_start_addr): If stmt has lhs, drop the lhs from call and
            set it after the call to the original value of the first argument.
            Formatting fixes.
            (decrement_count): Formatting fix.

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

[Bug tree-optimization/94130] [8 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9 Regression] Unintended |[8 Regression] Unintended
                   |result with optimization    |result with optimization
                   |option when assigning two   |option when assigning two
                   |structures, memset and 0    |structures, memset and 0

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 9.4+ too.

[Bug tree-optimization/94130] [8 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:9001bc36447e015283a5f1a0a924bd355f9d9df3

commit r8-10463-g9001bc36447e015283a5f1a0a924bd355f9d9df3
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 12 09:34:00 2020 +0100

    tree-dse: Fix mem* head trimming if call has lhs [PR94130]

    As the testcase shows, if DSE decides to head trim
{mem{set,cpy,move},strncpy}
    and the call has lhs, it is incorrect to leave the lhs as is, because it
    will then point to the adjusted address (base + head_trim) instead of the
    original base.
    The following patch fixes that by dropping the lhs of the call and
assigning
    lhs the original base in a following statement.

    2020-03-12  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/94130
            * tree-ssa-dse.c: Include gimplify.h.
            (increment_start_addr): If stmt has lhs, drop the lhs from call and
            set it after the call to the original value of the first argument.
            Formatting fixes.
            (decrement_count): Formatting fix.

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

    (cherry picked from commit a545ffafa380fa958393e1dfbf7f5f8f129bc5cf)

[Bug tree-optimization/94130] [8 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

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

[Bug tree-optimization/94130] [8 Regression] Unintended result with optimization option when assigning two structures, memset and 0

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jonathon.reinhart at gmail dot com

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
*** Bug 99431 has been marked as a duplicate of this bug. ***