chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 418 forks source link

Chapel performs invalid memory operations on complex numbers #6466

Open dmk42 opened 7 years ago

dmk42 commented 7 years ago

Summary of Problem

Chapel programs use type punning to attempt to get refs to the individual components of complex numbers. The method used is invalid in C; the behavior is undefined so that type-based alias analysis can be performed on complex numbers. Gcc 7.1 happens to react with an internal compiler error with optimization level -O2 or higher, but the Chapel generated code is invalid, triggering an unexpected case.

In modules/standard/Types.chpl the implementation of max() for complex numbers is as follows.

proc max(type t) where isComplexType(t) {
  param floatwidth = numBits(t) / 2;
  return (max(real(floatwidth)), max(real(floatwidth))): t;
}

The Chapel code translates to the following C code. This C code is greatly hand-simplified to make it easier to read and independent of header files, but it still triggers the undefined behavior leading to the gcc compiler error.

static inline float *complex64GetRealRef(float _Complex *cplx)
{
  return ((float *)cplx) + 0;  // invalid type punning
}

static inline float *complex64GetImagRef(float _Complex *cplx)
{
  return ((float *)cplx) + 1;  // invalid type punning
}

float _Complex max_chpl4()
{
  float _Complex c_chpl;

  // gcc 7.1 crashes on the following line
  *complex64GetRealRef(&c_chpl) = 3.40282346638528859811704183484516925e+38F;
  *complex64GetImagRef(&c_chpl) = 3.40282346638528859811704183484516925e+38F;

  return c_chpl;
}

The complex64GetRealRef() and complex64GetImagRef() functions are in runtime/include/chpltypes.h so this might seem like a bug in the runtime. However, the real problem is that the generated code needs these functions because it expects to get refs to the individual components of a complex number.

If the generated code could do something along these lines, it would accomplish the desired result without undefined behavior.

#include <complex.h>
#include <float.h>

float complex max_chpl4()
{
  return FLT_MAX + I*FLT_MAX;
}

Steps to Reproduce

Source Code:

Please see the example above. For a complete Chapel program that triggers the bug, see test/trivial/waynew/maxtest.chpl.

Compile command: chpl --fast test/trivial/wanew/maxtest.chpl

Execution command: When the backend compiler is gcc 7.1, execution is not possible because the compilation aborts.

Associated Future Test(s): N/A

Configuration Information

tmacd9 commented 7 years ago

The C14 Standard states in 6.2.5: each complex type has the same representation and alignment requirements as an array type containing exactly two elements of the corresponding real type; the first element is equal to the real part, and the second element to the imaginary part, of the complex number.

If the code were changed such that a "pointer to an array of two elements" were used, or the function parameter itself were declare as void * it seems like the punning is allowed?

dmk42 commented 7 years ago

Right, one way to avoid undefined behavior would be to have the compiler change the type it used to be an array of two floats, or a union containing a complex number and an array of two floats. The latter seems like it could be done pretty elegantly.

Another conforming method would be to use something like memcpy() to access the individual components.

Yet another way would be to use getter and setter functions for the individual components' values (instead of getting their addresses), and those functions could access through a union or via memcpy(). That would keep the compiler from having to change the types it uses. It depends on what is easiest given the architecture of the compiler.

I've tried a few different methods and some valid ones still crash gcc -- but in that situation the fault is entirely gcc's.

It would be a good idea to change to one of the blessed methods so that some future optimizer doesn't ruin our day. If that also allows gcc 7 to work, that's a bonus, but it isn't the long-term purpose.

mppf commented 7 years ago

tagging @daviditen on this issue since he's worked on Chapel's complex numbers most recently