[Bug c/94187] New: valgrind error in count_nonzero_bytes ?

[Bug c/94187] New: valgrind error in count_nonzero_bytes ?

[dcb314 at hotmail dot com]

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

            Bug ID: 94187
           Summary: valgrind error in count_nonzero_bytes ?
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

For this C code:

int
foo (void)
{
  return *(int *) "";
}

I ran the code through a valgrind version of recent gcc trunk, 
with the compiler flag -O2, and got this:

./gcc.dg/pr68785.c
==49861== Invalid read of size 1
==49861==    at 0xD9CDDD: count_nonzero_bytes(tree_node*, unsigned long,
unsigned long, unsigned int*, bool*, bool*, bool*, vr_values const*,
ssa_name_limit_t&) (tree-ssa-strlen.c:4891)
==49861==    by 0xD9CF17: count_nonzero_bytes(tree_node*, unsigned long,
unsigned long, unsigned int*, bool*, bool*, bool*, vr_values const*,
ssa_name_limit_t&) (tree-ssa-strlen.c:4801)
==49861==    by 0xDA19EE: count_nonzero_bytes (tree-ssa-strlen.c:4920)
==49861==    by 0xDA19EE: handle_integral_assign(gimple_stmt_iterator*, bool*,
vr_values const*) (tree-ssa-strlen.c:5547)

This bug is strongly related to bug # 68785.

[Bug tree-optimization/94187] valgrind error in count_nonzero_bytes ?

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2020-03-16
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
I can't confirm that with current master with valgrind.
Similarly -fsanitize=address is also fine.

[Bug tree-optimization/94187] valgrind error in count_nonzero_bytes ?

[dcb314 at hotmail dot com]

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

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
I checked again and I see this problem in all my valgrind versions of gcc
from date 20200216 onwards. I have no earlier version.

I use the latest development version of valgrind. Latest release
version seems to be 3.15.0

Hardware is AMD 64 bit Piledriver.

[Bug tree-optimization/94187] valgrind error in count_nonzero_bytes ?

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I can reproduce it.
The bug is obvious.  We have exp STRING_CST "", nchars is 1, offset is 0 and
nbytes is 4 (because of the UB in the source code).

4812      const char *prep = NULL;
4813      if (TREE_CODE (exp) == STRING_CST)
4814        {
4815          unsigned nchars = TREE_STRING_LENGTH (exp);
4816          if (nchars < offset)
4817            return false;
4818    
4819          if (!nbytes)
4820            /* If NBYTES hasn't been determined earlier, either from
ADDR_EXPR
4821               (i.e., it's the size of a pointer), or from MEM_REF (as the
size
4822               of the access), set it here to the size of the string,
including
4823               all internal and trailing nuls if the string has any.  */
4824            nbytes = nchars - offset;
4825    
4826          prep = TREE_STRING_POINTER (exp) + offset;
4827        }
...
4887              /* When either ALLNUL is set and N is zero, also determine
4888                 whether all subsequent bytes after the first one (which
4889                 is nul) are zero or nonzero and clear ALLNUL if not.  */
4890              for (const char *p = prep; p != prep + nbytes; ++p)
4891                if (*p)
4892                  {
4893                    *allnul = false;
4894                    break;
4895                  }
Which means we happily read bytes from the "" string beyond the limit.

[Bug tree-optimization/94187] valgrind error in count_nonzero_bytes ?

[jakub at gcc dot gnu.org]

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

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

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

Untested fix.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.0
            Summary|valgrind error in           |[10 Regression] valgrind
                   |count_nonzero_bytes ?       |error in
                   |                            |count_nonzero_bytes since
                   |                            |r10-2101
           Priority|P3                          |P1

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r10-2101-gb631bdb3c16e85f35d38e39b3d315c35e4a5747c

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Actually, I'll take the testcase out, since gcc/testsuite/gcc.dg/pr68785.c is
identical except for -O3 instead of -O2, but the bug reproduces also at -O3.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[dcb314 at hotmail dot com]

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

--- Comment #7 from David Binderman <dcb314 at hotmail dot com> ---
Once a month or so, it might be worthwhile running a valgrind enabled
version of gcc over the C testsuite.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[dcb314 at hotmail dot com]

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

--- Comment #8 from David Binderman <dcb314 at hotmail dot com> ---
>From the testsuite, files ./gcc.dg/Warray-bounds-29.c and
./gcc.dg/Warray-bounds-32.c seem to cause similar problems with
function count_nonzero_bytes, so it might be worthwhile checking
that these two testcases are ok with the proposed patch.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to David Binderman from comment #7)
> Once a month or so, it might be worthwhile running a valgrind enabled
> version of gcc over the C testsuite.

I run ASAN and UBSAN GCC build on weekly basis. But I must confess that I
remove some problematic tests (for those we have a PRs).

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[dcb314 at hotmail dot com]

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

--- Comment #10 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to Martin Liška from comment #9)
> (In reply to David Binderman from comment #7)
> > Once a month or so, it might be worthwhile running a valgrind enabled
> > version of gcc over the C testsuite.
> 
> I run ASAN and UBSAN GCC build on weekly basis. But I must confess that I
> remove some problematic tests (for those we have a PRs).

So if I run a valgrind enabled gcc trunk over the C, C++ and Fortran
code in the testsuite once a week, then that should match your testing
activity.

There are about 5,800 Fortran source code files, about 16,100 C++
and about 35,800 C source code files. That's a lot. 

I'll start with Fortran, then add in C++ then C and see how I get on.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:741ff2a263fe0ddc343288331c0047c1a32af8b2

commit r10-7207-g741ff2a263fe0ddc343288331c0047c1a32af8b2
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Mar 17 10:43:46 2020 +0100

    strlen: Punt on UB reads past end of string literal [PR94187]

    The gcc.dg/pr68785.c test which contains:
    int
    foo (void)
    {
      return *(int *) "";
    }
    has UB in the program if it is ever called, but causes UB in the compiler
    as well as at least in theory non-reproduceable code generation.
    The problem is that nbytes is in this case 4, prep is the
    TREE_STRING_POINTER of a "" string literal with TREE_STRING_LENGTH of 1 and
    we do:
    4890              for (const char *p = prep; p != prep + nbytes; ++p)
    4891                if (*p)
    4892                  {
    4893                    *allnul = false;
    4894                    break;
    4895                  }
    and so read the bytes after the STRING_CST payload, which can be random.
    I think we should just punt in this case.

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

            PR tree-optimization/94187
            * tree-ssa-strlen.c (count_nonzero_bytes): Punt if
            nchars - offset < nbytes.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/94187] [10 Regression] valgrind error in count_nonzero_bytes since r10-2101

[dcb314 at hotmail dot com]

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

--- Comment #13 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to David Binderman from comment #10)
> There are about 5,800 Fortran source code files, about 16,100 C++
> and about 35,800 C source code files. That's a lot. 
> 
> I'll start with Fortran, then add in C++ then C and see how I get on.

Fortran has be done, C++ is just about possible, but C 
consumes days of processor time.

I've reported all the bugs I found for all three languages, 
but I think from now on I'll just do incremental testing i.e.
only test new source code files. Something like

$ find ~/gcc/trunk/gcc/testsuite -name \*.c -newer_than <LAST_WEEK> -print |
sort > file.c.list
$ for i in `cat file.c.list`
do
  echo $i
  valgrind_C_compiler -c -O3 $i
done

Not ideal, but about as much as I can achieve until I get access to faster
processors.