GCC generates bad code with zero length array and -O3

Bug #1685385 reported by Steve Ellcey
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gcc
Fix Released
Medium
gcc-5 (Ubuntu)
New
Undecided
Unassigned
gcc-6 (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

With the program below, if I compile it with '-O3 -DFLEX' I get good code but if I compile it with '-O3 -UFLEX' then I get bad code. The program is (unfortunately) not runnable but if you generate
assembly language you can see that the good version has a loop with ldr/str/ldr/str and the bad
version has a loop with ldr/ldr/str/str. I.e. the second load has been moved to be before the first store and if they are referencing the same memory that will result in incorrect behavior.

It is unclear to me why declaring the array 'o' at the end of struct 's' as a zero length array
instead of a C99 flexible array would affect the code generation but it does. The version with
the flexible array works, the version with a zero length array does not work (i.e. it moves the
second load up to before the first store).

Test case (compile with -O3 and either -DFLEX or -UFLEX):

struct q {
 int b;
};
struct r {
   int n;
   struct q slot[0];
};
struct s {
   int n;
#ifdef FLEX
 long int o[];
#else
 long int o[0];
#endif
};
extern int x, y, m;
extern struct s *a;
extern struct r *b;
extern void bar();
int foo() {
   int i,j;
   for (i = 0; i < m; i++) {
    a->o[i] = sizeof(*a);
    b = ((struct r *)(((char *)a) + a->o[a->n]));
 for (j = 0; j < 10; j++) {
  b->slot[j].b = 0;
    }
        bar();
  }
}

ProblemType: Bug
DistroRelease: Ubuntu 16.04
Package: gcc 4:5.3.1-1ubuntu1
Uname: Linux 4.10.0-rc2-00045-g2748079 aarch64
ApportVersion: 2.20.1-0ubuntu2.5
Architecture: arm64
Date: Fri Apr 21 16:07:53 2017
JournalErrors:
 Error: command ['journalctl', '-b', '--priority=warning', '--lines=1000'] failed with exit code 1: Failed to search journal ACL: Operation not supported
 No journal files were opened due to insufficient permissions.
ProcEnviron:
 TERM=xterm-256color
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: gcc-defaults
UpgradeStatus: No upgrade log present (probably fresh install)

Revision history for this message
Steve Ellcey (sellcey) wrote :
Revision history for this message
In , 3-sje (3-sje) wrote :

Created attachment 41274
Test case

GCC should treat zero length arrays like C99 flexible arrays when they are
at the end of a structure. I.e. recognize that accesses to that array may
go beyond the end of the structure. GCC has been treating zero length
arrays and flexible arrays differently since at least 5.0.

This was found on aarch64 but the bug is not platform specific. A test case
is attached, it cannot be run but if compiled for aarch64 with
-O2 -fno-strict-aliasing and either -UFLEX or -DFLEX you can see the
different code. In the -UFLEX case it generates a load/load/store/store
sequence and in the -DFLEX case it generates load/store/load/store for
the code in the main loop.

See also:

https://gcc.gnu.org/ml/gcc/2017-04/msg00118.html
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01257.html

Revision history for this message
In , Rguenth (rguenth) wrote :

Mine.

Revision history for this message
In , Rguenth (rguenth) wrote :

Author: rguenth
Date: Thu Apr 27 12:34:45 2017
New Revision: 247327

URL: https://gcc.gnu.org/viewcvs?rev=247327&root=gcc&view=rev
Log:
2017-04-27 Richard Biener <email address hidden>

 PR middle-end/80533
 * emit-rtl.c (set_mem_attributes_minus_bitpos): When
 stripping ARRAY_REFs from MEM_EXPR make sure we're not
 keeping a reference to a trailing array.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/emit-rtl.c

Revision history for this message
In , Rguenth (rguenth) wrote :

Fixed on trunk sofar.

Revision history for this message
Steve Ellcey (sellcey) wrote :

This bug has been fixed on the GCC mainline at FSF. It is PR 80533 on the FSF Bugzilla.

Changed in gcc-defaults:
importance: Unknown → Medium
status: Unknown → In Progress
Matthias Klose (doko)
affects: gcc-defaults → gcc
affects: gcc-defaults (Ubuntu) → gcc-6 (Ubuntu)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gcc-6 - 6.3.0-16ubuntu6

---------------
gcc-6 (6.3.0-16ubuntu6) artful; urgency=medium

  * Update to SVN 20170506 (r247720) from the gcc-6-branch.
    - Fix PR target/77728 (ARM), PR target/68491 (x86), PR fortran/80392,
      PR libgomp/80394, PR c/79940, PR c++/79572, PR c++/79641, PR c/80097,
      PR c++/79512, PR rtl-optimization/80501, PR sanitizer/80349,
      PR rtl-optimization/80385, PR libgomp/80394, PR c++/80297, PR debug/80321,
      PR target/80286 (x86), PR debug/79255, PR debug/80025, PR sanitizer/80168,
      PR rtl-optimization/80112, PR c++/80129, PR sanitizer/79944,
      PR target/79932 (x86), PR target/79932 (x86), PR c/79940,
      PR rtl-optimization/79901, PR target/79807 (x86), PR c++/79681,
      PR target/79729 (x86), PR middle-end/79396, PR target/79570,
      PR target/79494 (x86), PR target/79568 (x86), PR target/79559 (x86),
      PR c++/80363, PR c++/80176, PR c++/79572, PR c++/80141, PR c++/79896,
      PR c++/79664, PR c++/79639, PR c++/79512.
  * Fix dependency on gcc-base package for rtlibs stage build (Helmut Grohne).
  * Remove libquadmath/gdtoa license from debian/copyright (files removed).
  * Stop building packages now built by the gcc-7 source.
  * Fix PR rtl-optimization/60818, taken from the trunk (Adrian Glaubitz).
    Closes: #861945.
  * Fix building libgfortran, libgphobos and libmpx when building without
    common libs.

 -- Matthias Klose <email address hidden> Sun, 07 May 2017 09:21:42 +0200

Changed in gcc-6 (Ubuntu):
status: New → Fix Released
Revision history for this message
In , Rguenth (rguenth) wrote :

Fixed on all still maintained branches.

Changed in gcc:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.