[Bug c/61328] New: valgrind finds problem in find_bswap_or_nop_1

[Bug c/61328] New: valgrind finds problem in find_bswap_or_nop_1

[dcb314 at hotmail dot com]

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

            Bug ID: 61328
           Summary: valgrind finds problem in find_bswap_or_nop_1
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com

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

I just compiled the attached code with gcc trunk 20140526 with flag -O2 under
valgrind and it said

==24014== Conditional jump or move depends on uninitialised value(s)
==24014==    at 0xA8F1F5: find_bswap_or_nop_1(gimple_statement_base*,
symbolic_number*, int) (tree-ssa-math-opts.c:1918)
==24014==    by 0xA8FD14: (anonymous
namespace)::pass_optimize_bswap::execute(function*) (tree-ssa-math-opts.c:2034)
==24014==    by 0x8AA350: execute_one_pass(opt_pass*) (passes.c:2180)
==24014==    by 0x8AAAF7: execute_pass_list_1(opt_pass*) (passes.c:2233)
==24014==    by 0x8AADC7: execute_pass_list(function*, opt_pass*)
(passes.c:2234)
==24014==    by 0x64757C: expand_function(cgraph_node*) (cgraphunit.c:1783)
==24014==    by 0x64926C: compile() [clone .part.31] (cgraphunit.c:1917)

tree-ssa-math-opts.c:1918 is

      if (n1.size != n2.size || !source_expr2)
        return NULL_TREE;

Maybe

      if (!source_expr2)
        return NULL_TREE;

      if (n1.size != n2.size)
        return NULL_TREE;

would be better code.

[Bug c/61328] valgrind finds problem in find_bswap_or_nop_1

[thomas.preudhomme at arm dot com]

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

--- Comment #1 from Thomas Preud'homme <thomas.preudhomme at arm dot com> ---
*facepalm*

Yes indeed. Does this qualify for an obvious fix as per commiting rules?

[Bug c/61328] valgrind finds problem in find_bswap_or_nop_1

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Thomas Preud'homme from comment #1)
> *facepalm*
> 
> Yes indeed. Does this qualify for an obvious fix as per commiting rules?

Yes.

[Bug c/61328] valgrind finds problem in find_bswap_or_nop_1

[dcb314 at hotmail dot com]

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

--- Comment #3 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to David Binderman from comment #0)
> Maybe
>  
>       if (!source_expr2)
>         return NULL_TREE;
>  
>       if (n1.size != n2.size)
>         return NULL_TREE;
>  
> would be better code.

I tried out my proposed solution and it didn't seem to help.

Current working assumption is that something somewhere isn't
setting the size field. Maybe blindly zeroing out the entire 
struct might be a way forward.

[Bug c/61328] valgrind finds problem in find_bswap_or_nop_1

[thomas.preudhomme at arm dot com]

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

--- Comment #4 from Thomas Preud'homme <thomas.preudhomme at arm dot com> ---
Oh I think I see. When I wrote find_bswap_or_nop_load () I assumed that it
would only return in find_bswap_or_nop_1 as called in the GIMPLE_UNARY_RHS
case. It seems I was wrong.

[Bug tree-optimization/61328] valgrind finds problem in find_bswap_or_nop_1

[thopre01 at gcc dot gnu.org]

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

--- Comment #5 from thopre01 at gcc dot gnu.org ---
Author: thopre01
Date: Tue Jun  3 09:29:06 2014
New Revision: 211166

URL: http://gcc.gnu.org/viewcvs?rev=211166&root=gcc&view=rev
Log:
2014-06-03  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    PR tree-optimization/61328
    * tree-ssa-math-opts.c (init_symbolic_number): Extract symbolic number
        initialization from find_bswap_or_nop_1.
        (find_bswap_or_nop_1): Test return value of find_bswap_or_nop_1 stored
        in source_expr2 before using the size value the function sets. Also
        make use of init_symbolic_number () in both the old place and
        find_bswap_or_nop_load () to avoid reading uninitialized memory when
        doing recursion in the GIMPLE_BINARY_RHS case.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/61328] valgrind finds problem in find_bswap_or_nop_1

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #6 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Assuming fixed.