Re: [PATCH] handle local aggregate initialization in strlen (PR 83821)

[Martin Sebor]

Hi Marc,

Can you please comment/respond to Jeff's question below and
confirm whether my understanding of the restriction (below)
is correct?

Thanks
Martin

On 04/30/2018 11:50 AM, Jeff Law wrote:

On 01/12/2018 02:30 PM, Martin Sebor wrote:

A failure in a test for the recently enhanced -Warray-bounds
warning exposed an unnecessarily broad restriction in the strlen
pass that prevents it from tracking the length of a member string
of locally defined and initialized struct:

  void f (void)
  {
    struct { char s[8]; int i } a = { "1234", 5 };

    if (strlen (a.s) != 4)   // not folded
      abort ();
   }

IIUC, the restriction was in place to account for writes into
an array changing or invalidating the length of a string stored
in its initial elements.  This would happen if the write either
changed the string's terminating nul byte, or if it reset one
of the prior non-nul bytes.

To reflect just this intent the restriction can be tightened
up to improve the pass' ability to track even the lengths of
string members of locally initialized aggregates.  Besides
leading to better code this change also clears up the test
failure.

Tested on x86_64-linux.

Martin


gcc-83821.diff


PR tree-optimization/83821 - local aggregate initialization defeats strlen 
optimization

gcc/ChangeLog:

        PR tree-optimization/83821
        * tree-ssa-strlen.c (maybe_invalidate): Consider the length of
        a string when available.
        (handle_char_store): Reset calloc statement on a non-nul store.

gcc/testsuite/ChangeLog:

        PR tree-optimization/83821
        * c-c++-common/Warray-bounds-4.c: Remove XFAIL.
        * gcc.dg/strlenopt-43.c: New test.
        * gcc.dg/strlenopt-44.c: Same.
        * gcc.dg/tree-ssa/calloc-4.c: Same.

I see what you're trying to do.  But I'm really struggling to understand
Marc G's comment "Do not use si->nonzero_chars" since that's precisely
what your patch does.

Your patch seems reasonable on the surface, but I fear there's something
I'm missing.  Can you reach out to Marc G. to see if he recalls the
rational behind the comment.

The comment in its original form was introduced here:

commit 9f15ed6e5c148ded6e7942e75595d91151792c9b
Author: glisse <glisse@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Jun 24 18:50:00 2014 +0000

    2014-06-24  Marc Glisse  <marc.gli...@inria.fr>

            PR tree-optimization/57742
    gcc/
            * tree-ssa-strlen.c (get_string_length): Ignore malloc.
            (handle_builtin_malloc, handle_builtin_memset): New functions.
            (strlen_optimize_stmt): Call them.
            * passes.def: Move strlen after loop+dom but before vrp.
    gcc/testsuite/
            * g++.dg/tree-ssa/calloc.C: New testcase.
            * gcc.dg/tree-ssa/calloc-1.c: Likewise.
            * gcc.dg/tree-ssa/calloc-2.c: Likewise.
            * gcc.dg/strlenopt-9.c: Adapt.


    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@211956
138bc75d-0d04-0410-961f-82ee72b054a4


Jeff