[Bug c/58270] New: Wrong code accessing array elements

[Bug c/58270] New: Wrong code accessing array elements

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

            Bug ID: 58270
           Summary: Wrong code accessing array elements
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: strasbur at chkw386 dot ch.pwr.wroc.pl
              Host: i386-pc-linux
            Target: i386-pc-linux

Created attachment 30717
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30717&action=edit
Isolated code giving wrong results

Array elements are accessed incorretly, if the array is passed in a global
structure.
This bug exists in 4.6 and 4.7 up to 4.7.2, no info about newer 4.7, 4.8 and
development.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[fweimer at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
The compiler is free to assume that both i1 and i2 are zero and the first store
is dead (because this is the only valid array index).  So if the buggy()
function stores a value of 1.0 at mem.dmem[0] unconditionally, this is still
correct.

struct {
    double dmem[1]; /* Change to dmem[2] and the bug disappears */
} mem;

void buggy(int i1, int i2) {
    mem.dmem[i1] = 0.5;
    mem.dmem[i2] = 1.;
}

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #2 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
OK, I'm not and expert, but mem is a global structure and it can be of
different size in other object file. The linker should assume the biggest of
all, correct?
The example I posted comes from f2c-translated FORTRAN77 code (it is cleared
from f2c references). It was a normal practice to mix C with FORTRAN for
dynamic memory allocation. The memory allocated via malloc() was referenced to
a small (one-element) static array. There was nothing illegal with this.
And how can the compiler assume freely that both i1 and i2 are zeros, if they
are passed as actual arguments?

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #3 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
The compiler option -fno-tree-dse does the job for me. Florian - thank you for
using the term "dead store" ;). I'm not sure whether it should be enabled by
default for a C compiler, but I'm not competent enough even to suggest a
solution.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #4 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Unfortunately, this is not the end of story. I'm going to attach a little more
complicated example, for which even using -fno-dse -fno-tree-dse does not help.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #5 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Created attachment 30719
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30719&action=edit
Second example, not working also with -fno-tree-dse -fno-dse

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #6 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Created attachment 30720
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30720&action=edit
File containing main() for the second example

[Bug c/58270] Wrong code while accessing array elements in a global structure

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #7 from Mikael Pettersson <mikpe at it dot uu.se> ---
Your examples are invalid C.  In one module you present the compiler with a
specific declaration, and complain when it utilizes constraints derived from
that declaration.  Then in another module you have an _incompatible_
declaration for the same object.  You can't expect to get away with that, even
if it seemed to work with an older compiler.

You should use a C99 "flexible array member", or a pointer (to an array of
unknown size).

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #8 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Mikael,
I cannot agree. Do not look at main.c, as the compiler doesn't know anything
about it while compiling buggy.c (this is the reason for which I keep main()
separately) and doesn't know that i1, i2 and i3 may be set to something > 0 at
runtime.
If it would be so much strict about declarations, it wouldn't also allow to
modify mem.dmem[1] - everything would go into mem.dmem[0].
However, it writes mem.dmem[1] only (!) if compiled without -fno-tree-dse and
mem.dmem[0] plus mem.dmem[1] with -fno-tree-dse.
The problem is that the compiler does not work predictably. BTW, correct size
of the mem structure (global variable) is ensured by the linker:
$ nm buggy.o
00000000 T buggy
0000000c C loc
00000008 C mem
I would also expect that if the compiler is instructed explicitly to release
some constraints, then these will be released.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |jakub at gcc dot gnu.org
         Resolution|---                         |INVALID

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
You are confusing how C/C++ commons work with how Fortran commons work.
Your examples are simply invalid C/C++.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #10 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Jakub,
I do not care about C++ (never understood it), but commons are just commons. I
see them from linker's perspective. How does the compiler treat variables
belonging to that common - this is a different story.
I examined the assembler outputs and I think that the real problem is that the
compiler treats one-element array (dmem) in buggy.c as ordinary variable.
somewhere.
If dmem is declared as two-element array (so that nobody can assume blindly to
which element data should go), then everything works correctly, regardless how
it is declared elsewhere.
It is an overoptimization IMHO, but I'm just a user.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> changed:

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

--- Comment #11 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
I'm not going to discuss whether my example is a valid C code or not, but in
FORTRAN it goes a similarly wrong way. The compiler treats incorrectly the
one-element array in a common.

[Bug c/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #12 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Created attachment 30740
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30740&action=edit
Example of failing FORTRAN code, with assembler output from gfortran 4.6.4

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

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

--- Comment #13 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Example of failing FORTRAN code, with assembler output from gfortran 4.6.4

This code is invalid:

5.7.2.5 Differences between named common and blank common

A blank common block has the same properties as a named common block, except
for the following.

...
\x0f Named common blocks of the same name shall be of the same size in all scoping
units of a program in which they appear, but blank common blocks may be of
di\verent sizes.
...

If you put the two *.f files in the same one and compile the result, you get
the following waring:

Warning: Named COMMON block 'mem' at (1) shall be of the same size as elsewhere
(24 vs 8 bytes)

and the executable gives the result you expect.
>From gcc-bugs-return-428822-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Sep 02 12:27:03 2013
Return-Path: <gcc-bugs-return-428822-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 23547 invoked by alias); 2 Sep 2013 12:27:03 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 23517 invoked by uid 48); 2 Sep 2013 12:27:00 -0000
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/58283] Incorrect debug info when precompilation is on
Date: Mon, 02 Sep 2013 12:27:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 4.8.1
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: rguenth at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-58283-4-2Mp0mQ3vO9@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-58283-4@http.gcc.gnu.org/bugzilla/>
References: <bug-58283-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-09/txt/msg00062.txt.bz2
Content-length: 786

http://gcc.gnu.org/bugzilla/show_bug.cgi?idX283

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
As said, it works for me.  Btw, including a PCH indirectly is not supported:

A precompiled header file can be used only when these conditions apply:

...

@item
A precompiled header can't be used once the first C token is seen.  You
can have preprocessor directives before a precompiled header; you cannot
include a precompiled header from inside another header.

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> changed:

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

--- Comment #14 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Marking the report as invalid doesn't solve the real problem.
I changed the common to unnamed and the situation is still the same.
main.f:
C Compile and link this file with buggy.f, using gfortran 4.6 (and probably
C any newer version), with optimization enabled (at least -O1).
C Run with: echo 1 2 3 | ./a.out
C expected (correct) result: 1. 2. 2.
      program main
      integer*4 i1,i2,i3
      real*8 dmem
      common dmem(3)
      read (*,*) i1,i2,i3
      call buggy(i1,i2,i3)
      write (*,*) dmem(1),dmem(2),dmem(3)
      end 
buggy.f:
      subroutine buggy(i1, i2, i3)
      integer*4 i1, i2, i3
      real*8 dmem
      common dmem(1)
      dmem(i1)=1.
      dmem(i2)=2.
      dmem(i3)=2.
      return
      end
Better?

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-09-02
     Ever confirmed|0                           |1

--- Comment #15 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
It is invalid to use

      subroutine buggy(i1, i2, i3)
      integer*4 i1, i2, i3
      real*8 dmem
      common dmem(1)
      dmem(i1)=1.
      dmem(i2)=2.
      dmem(i3)=2.
      return
      end

with any i* different from 1. If you compile the code with -fbounds-check (or
for recent gfortran, -fcheck=bounds) you get for 'echo 1 2 3'

Fortran runtime error: Index '2' of dimension 1 of array 'dmem' above upper
bound of 1

As the code is invalid if one of the i* is not one, the compiler can do
whatever it finds appropriate, e.g., set i1=i2=i3=1 (only valid case) and
discard the other assignments.

AFAICT, the following works as I expect (4.0, 2.0, 3.0):

[macbook] dominiq/Downloads% cat buggy.f90
      subroutine buggy(i1, i2, i3)
      integer*4 i1, i2, i3
      real*8 dmem
      common dmem(1)
      dmem(i1)=4.
!      dmem(i2)=2.
!      dmem(i3)=2.
      return
      end
[macbook] dominiq/Downloads% cat main.f90
! Compile and link this file with buggy.f, using gfortran 4.6 (and probably
! any newer version), with optimization enabled (at least -O1).
! Run with: echo 1 2 3 | ./a.out
! expected (correct) result: 1. 2. 2.
      program main
      implicit none
      integer*4 i1,i2,i3
      real*8 dmem
      common dmem(3)
      read (*,*) i1,i2,i3
      dmem(i1) = 1.0
      dmem(i2) = 2.0
      dmem(i3) = 3.0
      print *, dmem
      call buggy(i1,i2,i3)
      write (*,*) dmem(1),dmem(2),dmem(3)
      end

I let you close the PR as INVALID.

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #16 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
Dear Dominique,
I cannot agree with you. You are interpreting the code that may access the
array beyond declared bounds as invalid, which is simply not true.
As you pointed it out before, unnamed common block may be declared larger
elsewhere, so writing the dmem array beyond its first element may be a design
decision and therefore may be perfectly legal. The compiler has no clue about
real size of unnamed common while compiling buggy.f and bounds checking is
optional.
I would also like to point it out that interpreting things this way you do, you
exclude some older FORTRAN77 software (for example: quantum chemistry GAMESS),
in which the lack of dynamic memory allocation was overcome using the trick we
are discussing here (mixing with C was needed). BTW, change the size of dmem to
2 in buggy.f and things start to work correctly, although "out of bounds"
memory accesses still do happen. The problem occurs only if dmem is of size 1.
Of course you (developers) may decide to ignore this problem anyway, so if you
do so, feel free to close this bug. I'm not going to reopen it again, because
I'm out of arguments. I'm also not competent enough to tamper with the
compiler.

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #17 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> I cannot agree with you. You are interpreting the code that may access the
> array beyond declared bounds as invalid, which is simply not true.

6.5 Use of data objects

... The value of a subscript in an array element shall be within the bounds for
its dimension.

where the "shall" means (as elsewhere in the standard) that it is the coder
responsibility to honor the constraint at run time.

> As you pointed it out before, unnamed common block may be declared larger
> elsewhere, so writing the dmem array beyond its first element may be a design
> decision and therefore may be perfectly legal. 

As above this is a wrong assumption: the design decision must be standard
conforming. When you write "common dmem(1)" you tell the compiler that 'dmem'
has only one element.

> The compiler has no clue about real size of unnamed common while compiling 
> buggy.f and bounds checking is optional.

When you write "common dmem(1)" you tell the compiler that 'dmem' has only one
element.

> I would also like to point it out that interpreting things this way 
> you do, 

This is not my interpretation, it is what the Fortran standard says.

> you exclude some older FORTRAN77 software (for example: quantum chemistry
> GAMESS), in which the lack of dynamic memory allocation was overcome using 
> the trick we are discussing here (mixing with C was needed). 

Well, I have used safer tricks to overcome the limitation.

> BTW, change the size of dmem to 2 in buggy.f and things start to work 
> correctly, although "out of bounds" memory accesses still do happen. 
> The problem occurs only if dmem is of size 1.

Because only for size 1 the optimizer can infer that valid uses will provide
the (1,1,1) triplet.

> Of course you (developers) may decide to ignore this problem anyway, 
> so if you do so, feel free to close this bug. I'm not going to reopen 
> it again, because I'm out of arguments. I'm also not competent enough 
> to tamper with the compiler.

What you can do is to look at the GCC manual and try to find the optimization
pass than enable the optimization you don't like and disable it. Note that most
optimizations are not part of the gfortran front-end.

[Bug fortran/58270] Wrong code while accessing array elements in a global structure

[strasbur at chkw386 dot ch.pwr.wroc.pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #18 from Krzysztof Strasburger <strasbur at chkw386 dot ch.pwr.wroc.pl> ---
I have found that tree-fre, tree-pre and tree-dse have to be disabled in order
to generate correctly working code at all optimization levels (both C and
FORTRAN).
I'm happy with this workaround, so thank you for all suggestions.

[Bug middle-end/58270] Wrong code while accessing trailing array elements in a global common structure

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|i386-pc-linux               |
             Status|WAITING                     |NEW
                 CC|                            |rguenth at gcc dot gnu.org
          Component|fortran                     |middle-end
               Host|i386-pc-linux               |
            Summary|Wrong code while accessing  |Wrong code while accessing
                   |array elements in a global  |trailing array elements in
                   |structure                   |a global common structure
      Known to fail|                            |4.9.0

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
DECL_COMMONs could be special-cased in places that look at DECL_SIZE (I'll
declare this a QOI issue).  Or all decls that do not bind locally.  The
place that would "fix" tree-fre, tree-pre and tree-dse is in
get_ref_base_and_extent where it does

  if (DECL_P (exp))
    {
      /* If maxsize is unknown adjust it according to the size of the
         base decl.  */
      if (maxsize == -1
          && host_integerp (DECL_SIZE (exp), 1))
        maxsize = TREE_INT_CST_LOW (DECL_SIZE (exp)) - hbit_offset;
    }
  else if (CONSTANT_CLASS_P (exp))
    {
      /* If maxsize is unknown adjust it according to the size of the
         base type constant.  */
      if (maxsize == -1
          && host_integerp (TYPE_SIZE (TREE_TYPE (exp)), 1))
        maxsize = TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (exp))) - hbit_offset;
    }

In its general processing the function treats all trailing arrays as possibly
of undefined size.

Note that because -fcommon is still the default for all C dialects the
impact of changing the above for example in the simple && !DECL_COMMON (exp)
way is unknown.  -fcommon is a source of interesting bugs.

[Bug middle-end/58270] Wrong code while accessing trailing array elements in a global common structure

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #20 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
This looks like a duplicate of pr50463 (and may be more).

[Bug middle-end/58270] Wrong code while accessing trailing array elements in a global common structure

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58270

--- Comment #21 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Duplicate of pr53086 also.

[Bug middle-end/58270] Wrong code while accessing trailing array elements in a global common structure

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=53086
             Status|NEW                         |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #22 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Invalid as mentioned and a dup as mentioned.

*** This bug has been marked as a duplicate of bug 50463 ***