Created attachment 30717 [details] 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.
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.; }
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?
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.
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.
Created attachment 30719 [details] Second example, not working also with -fno-tree-dse -fno-dse
Created attachment 30720 [details] File containing main() for the second example
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).
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.
You are confusing how C/C++ commons work with how Fortran commons work. Your examples are simply invalid C/C++.
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.
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.
Created attachment 30740 [details] Example of failing FORTRAN code, with assembler output from gfortran 4.6.4
> 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. ... 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 dierent 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.
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?
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.
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.
> 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.
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.
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.
This looks like a duplicate of pr50463 (and may be more).
Duplicate of pr53086 also.
Invalid as mentioned and a dup as mentioned. *** This bug has been marked as a duplicate of bug 50463 ***