Bug 86914 - [8 Regression] -O2 generates wrong code with strlen() of pointers within one-element arrays of structures
Summary: [8 Regression] -O2 generates wrong code with strlen() of pointers within one-...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.1.0
: P2 normal
Target Milestone: 8.3
Assignee: Martin Sebor
URL:
Keywords: patch, wrong-code
: 87167 (view as bug list)
Depends on:
Blocks: strlen
  Show dependency treegraph
 
Reported: 2018-08-10 21:24 UTC by Even Rouault
Modified: 2018-08-31 14:26 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 7.1.0
Known to fail: 8.2.0
Last reconfirmed: 2018-08-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Even Rouault 2018-08-10 21:24:25 UTC
The following code
{{{
#include <string.h>

struct s
{
    int i;
    char c[1];
};

size_t foo(struct s* p)
{
    return strlen(p->c+1);
}
}}}

compiled with gcc 8.1 -O2 generates the following code
{{{
0000000000000000 <foo>:
   0:	31 c0                	xor    %eax,%eax
   2:	c3                   	retq   
}}}

returning 0

Previous gcc versions generate the "correct" code
{{{
0000000000000000 <foo>:
   0:	48 83 c7 05          	add    $0x5,%rdi
   4:	e9 00 00 00 00       	jmpq   9 <foo+0x9>	5: R_X86_64_PC32	strlen-0x4
}}}

According to https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html , """Although using one-element arrays this way is discouraged, GCC handles accesses to trailing one-element array members analogously to zero-length arrays. """

If modifying c[1] to be c[0], gcc 8.1 -O2 generates the correct code. If changing strlen(p->c+1) to strlen(p->c), the correct code is although generated

So it looks to be a too agressive optimization.
Comment 1 Even Rouault 2018-08-11 12:05:31 UTC
Interstingly with gcc 8.2, char c[0] doesn't generate the appropriate code. The only way of making it work is to use char c[] syntax
Comment 2 Martin Sebor 2018-08-11 17:58:21 UTC
Confirmed.  Most likely caused by r255790.

The strlen pass sees:

  _1 = &MEM[(void *)p_3(D) + 5B];
  _2 = __builtin_strlen (_1);

The code in maybe_set_strlen_range() tries to avoid using the size of the last member array (GCC treats all such arrays as flexible array members) by checking the result of array_at_struct_end_p() but the function doesn't handle MEM_REF and returns false.  The optimization is disabled if the array is a bona fide C11 flexible array member because the size of such an array is unknown.

Enhancing array_at_struct_end_p() to deal with MEM_REF or adding an array_not_at_struct_end_p() that returns a conservative result would fix this but the strlen range optimization will likely be disabled regardless.
Comment 3 Martin Sebor 2018-08-11 17:58:49 UTC
Let me handle this.
Comment 4 Martin Sebor 2018-08-14 19:03:09 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00823.html
Comment 5 Martin Sebor 2018-08-28 00:26:22 UTC
Author: msebor
Date: Tue Aug 28 00:25:50 2018
New Revision: 263905

URL: https://gcc.gnu.org/viewcvs?rev=263905&root=gcc&view=rev
Log:
PR tree-optimization/86914 - wrong code with strlen() of poor-man's flexible array member plus offset

gcc/ChangeLog:

	PR tree-optimization/86914
	* tree-ssa-strlen.c (maybe_set_strlen_range): Avoid MEM_REF.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86914
	* gcc.dg/strlenopt-57.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/strlenopt-57.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-strlen.c
Comment 6 Martin Sebor 2018-08-28 00:27:10 UTC
Patch committed to trunk in r263905.
Comment 7 Martin Sebor 2018-08-28 16:44:30 UTC
Author: msebor
Date: Tue Aug 28 16:43:58 2018
New Revision: 263925

URL: https://gcc.gnu.org/viewcvs?rev=263925&root=gcc&view=rev
Log:
PR tree-optimization/86914 - wrong code with strlen() of poor-man's flexible array member plus offset

gcc/ChangeLog:

	PR tree-optimization/86914
	* tree-ssa-strlen.c (maybe_set_strlen_range): Avoid MEM_REF.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86914
	* gcc.dg/strlenopt-57.c: New test.


Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/strlenopt-57.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/tree-ssa-strlen.c
Comment 8 Martin Sebor 2018-08-28 16:45:00 UTC
Backported to GCC 8 in r263925.
Comment 9 Martin Sebor 2018-08-31 14:26:10 UTC
*** Bug 87167 has been marked as a duplicate of this bug. ***