apparent union bug

[regehr]

Bugzilla Link	2680
Resolution	FIXED
Resolved on	Aug 16, 2008 23:27
Version	unspecified
OS	Linux

Extended Description

This is seen on r54780 on Ubuntu Feisty on ia32. Removing any field of cjt5 makes the problem go away.

[regehr@babel tmp8]$ llvm-gcc -O -Wall small.c -o small ; ./small
small: small.c:26: main: Assertion `cj41.cj35 == cj43.cj35' failed.
Aborted (core dumped)

[regehr@babel tmp8]$ cat small.c
#include <assert.h>

union cjt7
{
double *cj35;
struct cjt5 {
float cj27;
signed int cj29:7;
signed int cj30:7;
signed int cj31:5;
} cj37;
};

union cjt7 cj41 = {
(double *) 2140832546U,
};

union cjt7 callee_cj0f(void)
{
return cj41;
}

int main(void)
{
union cjt7 cj43 = callee_cj0f();
assert(cj41.cj35 == cj43.cj35);
return 0;
}

[llvmbot]

This works for me on x86-32 Darwin. It could be related to 54503. Is your llvm-gcc up to date as well as llvm? If it is, could you post the assembly code here?

[regehr]

x86 asm

[regehr]

llvm asm

[regehr]

Yes my llvm-gcc is from today. Files attached, hope this helps.

[llvmbot]

This is a cute one; I suppose the good news is that this has nothing to do with 54503. The code is copying 2140832546U == 0x7f9a8322 by
flds _cj41
fstps (%eax)
movl _cj41+4, %ecx
movl %ecx, 4(%eax)
(it's this way in the IR). Unfortunately 0x7f9a considered as single-precision is SNaN, and the hardware helpfully changes it to QNaN. Gotta love copy instructions that don't copy. Bottom line, 387 FP copies can't be relied on to do bitwise copies.

[llvmbot]

So is this PR invalid?

[llvmbot]

Not at all. The active element of the union at the time it's copied is a pointer, and we therefore can't use floating point operations to copy it (at least on x87). It is probably an easy fix in the llvm-gcc FE to do a bitwise copy (memcpy) instead in this case (returning a value). However, I'm concerned this might be a symptom of a more widespread problem. The IR represents this type as
%struct.cjt5 = type { float, i32 }
%struct.cjt7 = type { %struct.cjt5 }
so it's likely there's other code around that thinks it is OK to use floating point operations on it, IMO. The IR has lost the information about the overlapping pointer field. Perhaps we should add a was-union or must-be-copied-bitwise flag.

[llvmbot]

Thoughts: (1) if I bitcast a float* to an int*, then store to it,
the optimizers should not turn this into a float write unless they
can prove that the value corresponds to an ordinary float value.
Might be worth testing that the optimizers handle this right.
(2) I don't remember how C bitfields work, but doesn't struct cjt5
have holes in it? So how come it is being used to do the copy and
not the double*?
(3) In the case of
union U { float a; int b; }
what do the C language rules say about copying U? Is it allowed
to be copied by copying the float? [If so, that means the standard
allows undefined behaviour if U holds a funky float value].

[llvmbot]

proposed patch
John, could you see if this patch fixes it?

[llvmbot]

Re (2), the C struct cjt5 has holes in it; the LLVM IR type of that name does not. I don't know how the converter constructs a type to represent a union (except that it must be the same size and alignment as the real union).
Re (3), I don't think copying is supposed to change the value; using fld/fstp is wrong for that union.
But of course the as-if rule comes into play, and a QNaN will generally behave the same as an SNaN on targets that don't actually trap on SNaN (most of them). So using fld/fstp will generally be OK for struct fields, or when the active field of your union object is actually the FP object, not some overlapping int object. In general the compiler won't be able to tell that's the case, so it should not use fld/fstp.

[regehr]

John, could you see if this patch fixes it?

Yep, looks good. I'll do more extensive testing over the weekend.

[llvmbot]

Should be fixed here (somewhat more restrictive patch).
http://llvm.org/viewvc/llvm-project?view=rev&revision=54867