New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 24398 - [mingw64] clang 3.6.2 long double representation causes Segmentation fault
Summary: [mingw64] clang 3.6.2 long double representation causes Segmentation fault
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: C++14 (show other bugs)
Version: 3.6
Hardware: PC Windows NT
: P normal
Assignee: Martell Malone
URL:
Keywords: invalid-bug
Depends on:
Blocks:
 
Reported: 2015-08-08 02:58 PDT by Vladimir Petrigo
Modified: 2015-10-28 17:46 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s):


Attachments
clang/mingw64 + libstdc++ long double issue (6.73 KB, application/octet-stream)
2015-09-19 09:15 PDT, florian.ziesche+llvm
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Petrigo 2015-08-08 02:58:15 PDT
Hello,

I have met an issue with segmentation fault in program which was built by clang.
 Here is the code which represents the bug:

#include <iostream>                         
#include <string>                           

int main() {                                
    char *ptr;                              
    const char *s = "1.23";                         
    long double ld = std::strtold(s, &ptr);                                           
    std::cout << ld;

    return 0;                               
}                                                                       

With the current  g++ (5.2.0)  there is no problem, but clang has. 
 I use clang from mingw64 repo:
clang version 3.6.2 (tags/RELEASE_362/final)
Target: x86_64-w64-windows-gnu
Thread model: posix


I also tried to build the latest available clang , but it has the same effect:
clang version 3.8.0 (trunk)
Target: x86_64-w64-windows-gnu
Thread model: posix

And I also found one more case when clang++ gives  Segmentation fault  error:

#include <sstream>
#include <iostream>

long double foo(long double v) {
    std::stringstream ss;

    ss << v;

    ss.precision(6);

    ss >> v;

    return v;
}

int main() {
    long double a = 10.1238127638712638;

    std::cout << foo(a) << std::endl;

    return 0;
}
Comment 1 Martell Malone 2015-08-08 08:48:30 PDT
Thanks for moving this from msys2 to bugs.llvm so others can see :)

EDIT: It is now tagged as mingw-w64
The bug doesn't specify the target

I will do some digging and get some asm dumps of the test case and try to get to the bottom of this.

I suspect the cause to be that mingw-w64 has 'long double' support, but is
actually using 80-bit extended-mode x87 FPU.

There has been numerous bugs in this area with different projects.
Here is the general issue though

FPU (x87) control word setting as per manual:

fctrl default value 0x27f (double computation precision) is standard on BSD like systems and for MSVC.

fctrl default value 0x37f (extended computation precision) is standard on GNU/Linux as well as for mingw-w64.
Comment 2 Vladimir Petrigo 2015-08-09 13:36:14 PDT
But why then it works on Linux? And g++ works with the same stdlibc++ in the MSYS2 environment.
Comment 3 Martell Malone 2015-08-09 13:40:33 PDT
This is most likely because clang has not enabled handling of 80-bit extended-mode long double when building for the mingw target while gcc has.

I will get to it soon.
Its the next thing on my list after the linker
Comment 4 Martell Malone 2015-08-14 13:39:51 PDT
Proposed fix.

http://reviews.llvm.org/D12037
Comment 5 Martell Malone 2015-08-14 14:10:54 PDT
http://reviews.llvm.org/rL245084

landed
Comment 6 florian.ziesche+llvm 2015-09-19 05:13:18 PDT
this was accidentally reverted by http://llvm.org/viewvc/llvm-project?view=revision&revision=245620

please reapply the "LongDoubleFormat = &llvm::APFloat::x87DoubleExtended" part


not sure about the 32-bit part though (http://llvm.org/viewvc/llvm-project?view=revision&revision=245618), should this be x87DoubleExtended as well?
Comment 7 Yaron Keren 2015-09-19 06:41:17 PDT
It may be non obvious at a glance, but MinGWX86_64TargetInfo inherits from WindowsX86_64TargetInfo which inherits from WindowsTargetInfo<X86_64TargetInfo> that calls X86_64TargetInfo constructor which inherits from X86TargetInfo whose constructor sets LongDoubleFormat to &llvm::APFloat::x87DoubleExtended. 

Why set LongDoubleFormat to the same value it already has?
Do you have a test case where applying this patch changes behaviour?
Comment 8 florian.ziesche+llvm 2015-09-19 06:45:05 PDT
sorry, yes, didn't see that. but long double is still an issue on mingw64.

test code:
	long double val = 123.456;
	cout << "alignof: " << alignof(long double) << endl;
	cout << "sizeof: " << sizeof(long double) << endl;
	cout << "val: " << val << endl;

with gcc 5.2.0 + libstdc++ this produces:
alignof: 16
sizeof: 16
val: 123.456

with clang 3.7.0 + libstdc++ this produces:
alignof: 16
sizeof: 12
val: -1.68953e+4672


might be a different issue then? but I don't know what.
Comment 9 Yaron Keren 2015-09-19 07:15:11 PDT
With both prebuilt 3.7 binaries downloaded from llvm.org and current SVN I get sizeof: 16, not sizeof: 12.

Are you comparing mingw targetting 64-bit to clang targetting 32-bit?
To target 64-bit specify -target x86_64-windows-gnu on the command line.
Which mingw distribution do you use, exactly?
Comment 10 florian.ziesche+llvm 2015-09-19 09:15:30 PDT
Created attachment 14907 [details]
clang/mingw64 + libstdc++ long double issue
Comment 11 florian.ziesche+llvm 2015-09-19 09:16:05 PDT
(In reply to comment #9)
> With both prebuilt 3.7 binaries downloaded from llvm.org and current SVN I
> get sizeof: 16, not sizeof: 12.
> 
> Are you comparing mingw targetting 64-bit to clang targetting 32-bit?
> To target 64-bit specify -target x86_64-windows-gnu on the command line.
> Which mingw distribution do you use, exactly?

msys2, and yes, everything is 64-bit.
And I've found the culprit of 12 byte issue: https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-clang/0015-attempt-to-fix-long-double-on-mingw-w64.patch

After removing this patch, sizeof(long double) is 16 again, as it should be (so I'd assume that clang trunk and 3.7.0 are correct in that regard). But this still doesn't fix the original issue, cout will still print -1.68953e+4672 instead of 123.456.

Also, if it helps somehow, I've attached the .ll file.
Comment 12 Martell Malone 2015-09-19 09:26:33 PDT
0015-attempt-to-fix-long-double-on-mingw-w64.patch
Should not be applied to msys2 clang
We already went through the discussion that this doesn't work.

Alexey applied it when he was testing if it fixes it himself.
We already confirmed that this does not fix it.

Like you I never saw how X86TargetInfo set LongDoubleFormat = &llvm::APFloat::x87DoubleExtended;

So I done this bug more harm then good with my original fix.
Comment 13 florian.ziesche+llvm 2015-09-19 09:58:22 PDT
interestingly, just doing "cout << 123.456l << endl;" will print garbage (always a different value: 1.88947e-4944, 5.42507e-4944, 1.48336e-4944, ... although same exponent).

objdump/asm snippet for this:
0000000000401610 <main>:
  401610:	55                   	push   %rbp
  401611:	48 83 ec 60          	sub    $0x60,%rsp
  401615:	48 8d 6c 24 60       	lea    0x60(%rsp),%rbp
  40161a:	48 89 55 e8          	mov    %rdx,-0x18(%rbp)
  40161e:	89 4d e4             	mov    %ecx,-0x1c(%rbp)
  401621:	e8 0a 02 00 00       	callq  401830 <__main>
  401626:	c7 45 fc 00 00 00 00 	movl   $0x0,-0x4(%rbp)
  40162d:	8b 4d e4             	mov    -0x1c(%rbp),%ecx
  401630:	89 4d f8             	mov    %ecx,-0x8(%rbp)
  401633:	48 8b 55 e8          	mov    -0x18(%rbp),%rdx
  401637:	48 89 55 f0          	mov    %rdx,-0x10(%rbp)
  40163b:	48 89 e0             	mov    %rsp,%rax
  40163e:	db 2d ec 29 00 00    	fldt   0x29ec(%rip)        # 404030 <LCPI2_0>
  401644:	db 78 20             	fstpt  0x20(%rax)
  401647:	48 8d 0d 12 6c 00 00 	lea    0x6c12(%rip),%rcx        # 408260 <__imp__ZSt4cout>

000000000040164a <__fu0__ZSt4cout>:
  40164a:	12 6c 00 00          	adc    0x0(%rax,%rax,1),%ch
  40164e:	e8 55 00 00 00       	callq  4016a8 <_ZNSolsEe>
  401653:	48 8d 15 36 00 00 00 	lea    0x36(%rip),%rdx        # 401690 <_ZSt4endlIcSt11char_traitsIcEERSt13basic_ostreamIT_T0_ES6_>
  40165a:	48 89 c1             	mov    %rax,%rcx
  40165d:	e8 4e 00 00 00       	callq  4016b0 <_ZNSolsEPFRSoS_E>
  401662:	45 31 c0             	xor    %r8d,%r8d
  401665:	48 89 45 d8          	mov    %rax,-0x28(%rbp)
  401669:	44 89 c0             	mov    %r8d,%eax
  40166c:	48 83 c4 60          	add    $0x60,%rsp
  401670:	5d                   	pop    %rbp
  401671:	c3                   	retq   
  401672:	66 66 66 66 66 2e 0f 	data16 data16 data16 data16 nopw %cs:0x0(%rax,%rax,1)
  401679:	1f 84 00 00 00 00 00
Comment 14 Yaron Keren 2015-09-19 11:26:12 PDT
I confirm the garbage print with SVN clang in 64 bit but *not* in 32 bit.
Since the long double sizes do match, this may be wrong calling convention?
Comment 15 florian.ziesche+llvm 2015-09-19 11:33:32 PDT
(In reply to comment #14)
> Since the long double sizes do match, this may be wrong calling convention?

something along those lines.
this also affects other uses of long double in libstdc++ btw (unordered_map uses long double internally): https://llvm.org/bugs/show_bug.cgi?id=24408
Comment 16 Yaron Keren 2015-09-19 12:56:17 PDT
Reid or Rafael, any ideas about this?
Comment 17 Reid Kleckner 2015-10-08 12:22:20 PDT
(In reply to comment #16)
> Reid or Rafael, any ideas about this?

Can you resummarize the issue? It's not obvious from reading the comments.

It sounds like there is a calling convention problem in how we are passing long double on x64 mingw, i.e. using SSE when we should use x87 or the other way around. Is that accurate?
Comment 18 Yaron Keren 2015-10-08 12:38:14 PDT
Yes, compiling and running 

	long double val = 123.456;
	cout << "alignof: " << alignof(long double) << endl;
	cout << "sizeof: " << sizeof(long double) << endl;
	cout << "val: " << val << endl;

with mingw-w64 64bit prints correct values for the alignment and sizeof, both 16, yet the output of val printed out is garbage.

Comment 13 show disassembled code for the clang caller and the library callee.

A suitable copy of mingw-w64 64 bit can be found at:

https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/5.2.0/threads-posix/seh/

if the bin directory of mingw is on the PATH trunk clang built with mingw will use the toolchain automatically. If you build clang with MSVC you'll need ot set the triple or set it as default -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-pc-windows-gnu
Comment 19 Martell Malone 2015-10-21 15:45:43 PDT
With some great pointers to different commits and test cases to help debugging this from chh I have come to the conclusion that gcc uses SSE2 for long doubles under mingw-w64 while clang uses x87 (80-bit precision).

The reason it is not an issue on x86 is because x87(80-bit-precision) is what is used by both gcc and clang.
Comment 20 Martell Malone 2015-10-28 16:11:41 PDT
The previous message was supposed to say that clang seems uses SSE2 for long doubles under mingw-w64 while gcc uses x87 (80-bit precision).

The reason it is not an issue on x86 is because x87(80-bit-precision) is what is used by both gcc and clang.
Comment 21 Reid Kleckner 2015-10-28 16:27:32 PDT
(In reply to comment #20)
> The previous message was supposed to say that clang seems uses SSE2 for long
> doubles under mingw-w64 while gcc uses x87 (80-bit precision).
> 
> The reason it is not an issue on x86 is because x87(80-bit-precision) is
> what is used by both gcc and clang.

That's not consistent with what I see here:

$ cat t.c
long double f(long double x) { return x + 1.0; }

$ /c/Program\ Files/mingw-w64/x86_64-5.2.0-posix-seh-rt_v4-rev0/mingw64/bin/gcc t.c -S -o - -O1
        .file   "t.c"
        .text
        .globl  f
        .def    f;      .scl    2;      .type   32;     .endef
        .seh_proc       f
f:
        .seh_endprologue
        movq    %rcx, %rax
        fldt    (%rdx)
        fadds   .LC0(%rip)
        fstpt   (%rcx)
        ret
        .seh_endproc
        .section .rdata,"dr"
        .align 4
.LC0:
        .long   1065353216
        .ident  "GCC: (x86_64-posix-seh-rev0, Built by MinGW-W64 project) 5.2.0"

The way I read this is that GCC uses 80bit x87s for long doubles, which it passes indirectly by pointer.

Fixing this involves changes to lib/Basic/Targets.cpp and lib/CodeGen/TargetInfo.cpp to tweak the calling convention.
Comment 22 Martell Malone 2015-10-28 17:16:13 PDT
   The way I read this is that GCC uses 80bit x87s for long doubles, which it passes indirectly by pointer.

Yes I did say that it is using x87 80-bit precision
I also got kai from mingw-w64 to confirm this.
https://www.mail-archive.com/mingw-w64-public@lists.sourceforge.net/msg11851.html

Here is what clang produces for the same program

        .text
        .def     f;
        .scl    2;
        .type   32;
        .endef
        .globl  f
        .align  16, 0x90
f:                                      # @f
.Ltmp0:
.seh_proc f
# BB#0:
        pushq   %rax
.Ltmp1:
        .seh_stackalloc 8
.Ltmp2:
        .seh_endprologue
        fldt    48(%rsp)
        fld1
        faddp   %st(1)
        popq    %rax
        retq
.Ltmp3:
        .seh_endproc
Comment 23 Martell Malone 2015-10-28 17:26:24 PDT
After looking in TargetInfo.cpp I can see that this is beyond my current knowledge to be able to fix this. I will refer to chh for tips on this as he currently has a very large patch affecting x87 for Android
Comment 24 Reid Kleckner 2015-10-28 17:40:43 PDT
(In reply to comment #22)
>    The way I read this is that GCC uses 80bit x87s for long doubles, which
> it passes indirectly by pointer.
> 
> Yes I did say that it is using x87 80-bit precision

Oops, yeah, I was replying to the previous comment. Wires crossed. =/

Anyway, this should be fixed in r251567. I ran the iostream test case and it seems to work.
Comment 25 Martell Malone 2015-10-28 17:46:05 PDT
Yes I can confirm that this now works for me we can mark this as closed / fixed :)