Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C struct mapping with a single double field corrupts data on win64 #28325

Closed
barche opened this issue Jul 28, 2018 · 5 comments
Closed

C struct mapping with a single double field corrupts data on win64 #28325

barche opened this issue Jul 28, 2018 · 5 comments
Labels
kind:upstream The issue is with an upstream dependency, e.g. LLVM system:windows Affects only Windows

Comments

@barche
Copy link
Contributor

barche commented Jul 28, 2018

Using the following C code:

#include <stdio.h>

struct OneWrappedInt
{
  double x;
};

struct TwoWrappedInts
{
  double x;
  double y;
};

#ifdef _WIN32
      #define DLL_EXPORT __declspec(dllexport)
#else
   #define DLL_EXPORT
#endif

DLL_EXPORT void print_one(struct OneWrappedInt x)
{
  printf("%f\n", x.x);
}

DLL_EXPORT void print_two(struct TwoWrappedInts x)
{
  printf("%f %f\n", x.x, x.y);
}

and the following Julia code:

struct OneWrappedInt
  x::Float64
end

struct TwoWrappedInts
  x::Float64
  y::Float64
end

println("This should print \"1\" :")
ccall((:print_one,"structs"), Cvoid, (OneWrappedInt,), OneWrappedInt(1.0))
println("This should print \"1 2\" :")
ccall((:print_two,"structs"), Cvoid, (TwoWrappedInts,), TwoWrappedInts(1.0,2.0))

This yields the wrong result for the OneWrappedInt case (0 instead of 1) on Win64:
https://ci.appveyor.com/project/barche/julia-c-struct-test#L16

I previously encountered this using mingw on Windows, but now it happens using the BinaryBuilder cross-compilation system, and CxxWrap is affected.

All other platforms, including Win64 using MSVC, seem to work.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 28, 2018

Do you have clang available? It would be helpful to compare @code_llvm against clang -o - -S -emit-llvm for the print_one function signature, then we can figure out if gcc or
https://github.com/JuliaLang/julia/blob/master/src/abi_win64.cpp is more likely at fault here

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 28, 2018

Actually, I'm on the machine where I have windows installed, so I can do that test:

julia> struct OneWrappedInt
         x::Float64
         end

julia> f(x) = ccall(x, Cvoid, (OneWrappedInt,), OneWrappedInt(1.0))
...
  %2 = inttoptr i64 %0 to void (i64)*
  call void %2(i64 4607182418800017408) ; f64 1.0e0
...
~$ clang --target=x86_64-w64-mingw32-pc -emit-llvm -S -o - -x c -
struct OneWrappedInt
{
  double x;
};

void print_one(struct OneWrappedInt x)
{
}
; ModuleID = '-'
source_filename = "-"
target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-gnu"

%struct.OneWrappedInt = type { double }

; Function Attrs: noinline nounwind uwtable
define void @print_one(i64) #0 {
  %2 = alloca %struct.OneWrappedInt, align 8
  %3 = getelementptr inbounds %struct.OneWrappedInt, %struct.OneWrappedInt* %2, i32 0, i32 0
  %4 = bitcast double* %3 to i64*
  store i64 %0, i64* %4, align 8
  ret void
}

attributes #0 = { noinline nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }

!llvm.module.flags = !{!0}
!llvm.ident = !{!1}

!0 = !{i32 1, !"PIC Level", i32 2}
!1 = !{!"Apple LLVM version 9.0.0 (clang-900.0.39.2)"}

It appears we're emitting correct code, so it's probably gcc that's broken. It's much harder to see, but we can confirm that this is a GCC bug (since we believe MSVC also give correct answers) by looking at its assembly output:

$ gcc -S -o - -x c -
struct OneWrappedInt
{
  double x;
};

void print_one(struct OneWrappedInt x)
{
}
        .file   ""
        .text
        .globl  print_one
        .def    print_one;      .scl    2;      .type   32;     .endef
        .seh_proc       print_one
print_one:
        pushq   %rbp
        .seh_pushreg    %rbp
        movq    %rsp, %rbp
        .seh_setframe   %rbp, 0
        .seh_endprologue
        movsd   %xmm0, 16(%rbp)
        nop
        popq    %rbp
        ret
        .seh_endproc
        .ident  "GCC: (GNU) 6.4.0"

For comparison, with clang, that result is:

~$ clang --target=x86_64-w64-mingw32-pc -S -o - -x c -
struct OneWrappedInt
{
  double x;
};

void print_one(struct OneWrappedInt x)
{
}
^D      .text
	.def	 print_one;
	.scl	2;
	.type	32;
	.endef
	.globl	print_one
	.p2align	4, 0x90
print_one:                              # @print_one
.Lcfi0:
.seh_proc print_one
# BB#0:
	pushq	%rax
.Lcfi1:
	.seh_stackalloc 8
.Lcfi2:
	.seh_endprologue
	movq	%rcx, (%rsp)
	popq	%rax
	retq
	.seh_handlerdata
	.text
.Lcfi3:
	.seh_endproc

Observe that clang correctly uses %rcx, while gcc incorrectly references %xmm0

@vtjnash vtjnash added kind:upstream The issue is with an upstream dependency, e.g. LLVM system:windows Affects only Windows labels Jul 28, 2018
@barche
Copy link
Contributor Author

barche commented Jul 28, 2018

OK, thanks, I just remembered how handy Compiler Explorer is for this kind of thing, and it shows that MSVC indeed also uses %rcx. I'll attempt to report this as a bug with gcc.

@barche
Copy link
Contributor Author

barche commented Jul 29, 2018

For reference, here is my report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86727

@simonbyrne
Copy link
Contributor

Sounds like this is not a Julia issue. Probably worth opening an issue at BinaryProvider.jl?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:upstream The issue is with an upstream dependency, e.g. LLVM system:windows Affects only Windows
Projects
None yet
Development

No branches or pull requests

3 participants