[Bug c/107734] New: valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[Bug c/107734] New: valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

            Bug ID: 107734
           Summary: valgrind error for
                    gcc/testsuite/cc.target/i386/pr46051.c
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

I just built a valgrind version of today's gcc trunk. 

When compiling source code file gcc.target/i386/pr46051.c,
I got the following:

$ ~/gcc/results.20221117.valgrind/bin/gcc -c -O2 ./gcc.target/i386/pr46051.c
==639651== Conditional jump or move depends on uninitialised value(s)
==639651==    at 0x11DFAE7: bitmap_set_bit (sbitmap.h:137)
==639651==    by 0x11DFAE7: gimple_simplify_122(gimple_match_op*, gimple**,
tree
_node* (*)(tree_node*), tree_node*, tree_node**, tree_code)
(gimple-match.cc:489
33)
==639651==    by 0x1154189: gimple_simplify_MULT_EXPR(gimple_match_op*,
gimple**
, tree_node* (*)(tree_node*), code_helper, tree_node*, tree_node*, tree_node*)
(
gimple-match.cc:131594)
==639651==    by 0x10771D1: gimple_resimplify2(gimple**, gimple_match_op*,
tree_
node* (*)(tree_node*)) (gimple-match-head.cc:323)

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---
The valgrind problem doesn't seem to occur with git hash 05432288d4e56055,
dated 20221113, so the bug is recent.

I used git hash 2b2f2ee49a33419f for today's build.

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
git blame says

dc95e1e970 (Hongyu Wang     2022-01-17 13:01:51 +0800 8292)            if
(!bitmap_set_bit (seen, sel[i].to_constant ()))

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #3 from David Binderman <dcb314 at hotmail dot com> ---

Another test case:

./gcc.target/i386/pr53366-2.c
==666641== Conditional jump or move depends on uninitialised value(s)
==666641==    at 0x11DFAE7: bitmap_set_bit (sbitmap.h:137)
==666641==    by 0x11DFAE7: gimple_simplify_122(gimple_match_op*, gimple**,
tree_node* (*)(tree_node*), tree_node*, tree_node**, tree_code)
(gimple-match.cc:48933)

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #4 from David Binderman <dcb314 at hotmail dot com> ---
A third:

./gcc.target/i386/pr61403.c
==749959== Conditional jump or move depends on uninitialised value(s)
==749959==    at 0x11DFAE7: bitmap_set_bit (sbitmap.h:137)
==749959==    by 0x11DFAE7: gimple_simplify_122(gimple_match_op*, gimple**,
tree_node* (*)(tree_node*), tree_node*, tree_node**, tree_code)
(gimple-match.cc:48933)

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #5 from David Binderman <dcb314 at hotmail dot com> ---
I have reduced one of the test cases downto this code:

float val1f[][2], val2f[][2], chkf[][2];
foof_i;
foof() {
  int j;
  foof_i = 0;
  for (; foof_i < 8; foof_i++) {
    float tmp = val1f[foof_i][j] * val2f[foof_i][j];
    j = 0;
    for (; j < 2; j++)
      chkf[foof_i][j] = tmp;
  }
}

[Bug c/107734] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #6 from David Binderman <dcb314 at hotmail dot com> ---
I am trying a bisect with git hash b4fca4fc70dc76cf.

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|valgrind error for          |[13 Regression] valgrind
                   |gcc/testsuite/cc.target/i38 |error for
                   |6/pr46051.c                 |gcc/testsuite/cc.target/i38
                   |                            |6/pr46051.c
   Target Milestone|---                         |13.0
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Keywords|                            |ice-checking,
                   |                            |ice-on-valid-code
   Last reconfirmed|                            |2022-11-17
          Component|c                           |middle-end
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have the obvious patch.
diff --git a/gcc/match.pd b/gcc/match.pd
index 5aba1653b80..a4d1386fd9f 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -8288,6 +8288,8 @@ and,
           if (sel.encoding ().encoded_full_vector_p ())
             {
               auto_sbitmap seen (nelts);
+              bitmap_clear (seen);
+
               unsigned HOST_WIDE_INT count = 0, i;

               for (i = 0; i < nelts; i++)

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

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

commit r13-4132-gee892832ea19b21a3420ef042e582204fac852a2
Author: Andrew Pinski <apinski@marvell.com>
Date:   Thu Nov 17 17:48:00 2022 +0000

    Fix PR 107734: valgrind errors with sbitmap in match.pd

    sbitmap is a simple bitmap and the memory allocated is not cleared
    on creation; you have to clear it or set it to all ones before using
    it.  This is unlike bitmap which is a sparse bitmap and the entries are
    cleared as created.
    The code added in r13-4044-gdc95e1e9702f2f missed that.
    This patch fixes that mistake.

    Committed as obvious after a bootstrap and test on x86_64-linux-gnu.

    gcc/ChangeLog:

            PR middle-end/107734
            * match.pd (perm + vector op pattern): Clear the sbitmap before
            use.

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[dcb314 at hotmail dot com]

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

--- Comment #10 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to Andrew Pinski from comment #9)
> Fixed.

Thanks for that. 

Would it ok to manually check all uses of sbitmap, to make sure they initialise
bits appropriately, or would it be better to define a constructor which sets 
the internal bitmap data to something sensible ?

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to David Binderman from comment #10)
> (In reply to Andrew Pinski from comment #9)
> > Fixed.
> 
> Thanks for that. 
> 
> Would it ok to manually check all uses of sbitmap, to make sure they
> initialise bits appropriately, or would it be better to define a constructor
> which sets 
> the internal bitmap data to something sensible ?

I suspect most are done correctly either having bitmap_clear or bitmap_ones
right after the creation of the sbitmap (I looked at a few when I was writing
this patch).
Someone could go and audit all of them but I am working on other things at this
point really.

[Bug middle-end/107734] [13 Regression] valgrind error for gcc/testsuite/cc.target/i386/pr46051.c

[wwwhhhyyy333 at gmail dot com]

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

--- Comment #12 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
(In reply to Andrew Pinski from comment #9)
> Fixed.

Thanks for the fix! I was not aware that sbitmap does not have a default
constructor :(.