llvm dev - Mar 2014 - [LLVMdev] Miscompile by copying a C union via an x87 FP load+store

I discovered a bug in which LLVM/Clang can miscompile C code that copies a
union.  This occurs on x86 when using x87 instructions for floating point
operations.

The following program's assertion fails when compiled with "-O1
-mno-sse"
(or when passing "-mcpu=blah" to llc).  do_copy() gets compiled using
x87
load+store instructions, which don't preserve exact byte values:

#include <assert.h>
#include <stdarg.h>
#include <stdio.h>

union U {
  double f;
  unsigned long long i;
};

__attribute__((noinline))
void do_copy(union U *dest, union U *src) {
  *dest = *src;
}

int main() {
  unsigned long long test_val = 0xfff0000000000001;
  union U val, dest;
  val.i = test_val;
  do_copy(&dest, &val);
  printf("%llx\n", dest.i);
  assert(dest.i == test_val);
  return 0;
}

$ clang copy_union_bug.c -o copy_union_bug -O1 -mno-sse -m32
$ ./copy_union_bug
fff8000000000001
copy_union_bug: copy_union_bug.c:22: int main(): Assertion `dest.i
=test_val' failed.


Initially I thought the bug was that Clang compiles the union type to
"%union.U = type { double }".

However, I think it's really the optimiser that's at fault.  InstCombine
converts do_copy()'s llvm.memcpy call to a load+store of a double.
 llvm.memcpy is meant to copy bytes faithfully.  llvm.memcpy takes i8*
arguments, and it shouldn't really assume anything about the data it's
copying based on the pointer types that its i8* arguments were bitcast from.

This would be a problem for Emscripten, where double loads/stores are
compiled to operations on a Javascript typed array, which aren't required
to preserve bit patterns of NaNs (
https://www.khronos.org/registry/typedarray/specs/latest/#4) and which
sometimes get compiled to x87 FP.  This could be a problem for PNaCl
depending on what guarantees PNaCl provides about double loads/stores (it's
currently not well-defined --
https://code.google.com/p/nativeclient/issues/detail?id=3822).

Should I change InstCombine to not convert the llvm.memcpy to a double
load+store?

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140326/0d25e9f8/attachment.html>