As of r160851, $ cat with_cl.cpp struct S { int a, b; }; class C { public: S foo(); }; S C::foo() { S ret; ret.a = 1; ret.b = 2; return ret; } ///////////////////// EOF $ cat with_clang.cpp struct S { int a, b; }; class C { public: S foo(); }; int main() { C c; S s = c.foo(); if (s.a != 1) return 1; if (s.b != 2) return 2; } ///////////////////// EOF $ clang -Xclang -cxx-abi -Xclang microsoft -c with_clang.cpp && cl -nologo -c with_cl.cpp && link -nologo with_cl.obj with_clang.o $ with_cl.exe Segmentation fault
Could you attach the assembly generated by MSVC?
(In reply to comment #1) > Could you attach the assembly generated by MSVC? Please don't attach output generated by the MSVC compiler to the public bug tracker unless you understand all of the implications of its EULA. I don't think we need it for Timur to continue working on this.
> I don't think we need it for Timur to continue working on this. FTR, I'm *not* working on this at the moment. This was found in SPEC 2006 tests on the povray benchmark. Currently I'm prioritized to get Clang working on Chromium and I don't think the pattern described in this bug is widespread there due to our strict code style. I might be wrong though as we have a lot of third_party/ there. So feel free to take over unless I explicitly take it myself :)
(In reply to comment #3) > > I don't think we need it for Timur to continue working on this. > FTR, > I'm *not* working on this at the moment. > This was found in SPEC 2006 tests on the povray benchmark. > > Currently I'm prioritized to get Clang working on Chromium and I don't think > the pattern described in this bug is widespread there due to our strict code > style. I'll wager you one beverage of your choice that you end up having to fix this for Chromium. ;] Anyways, prioritize as is convenient for you, I just was pointing out that whomever chooses to dig into this can likely do so w/o someone posting asm.
> I'll wager you one beverage of your choice that you end up having to fix > this for Chromium. ;] I was wise enough not to make a bet with you ;) Indeed, Chromium's FilePath::DirName returns a FilePath by value, it's a 32-byte structure => boom it crashes. -- Some more info. Actually it turns out the problem is not that the struct is "2 integers". At least I observe the same crash regardless of the "S" size (tested 4,8,12,32). A similar "returning a struct" scenario works fine for non-class functions or static class method. The crash also happens if I explicitly mark the method as __cdecl so it's likely not unique to thiscall (default method CC). Changing the subject accordingly.
Taking this. After some investigation, there are at least three different problems here: 1) LLVM: "sret" is currently mishandled on Windows. The current version of lib/Target/X86/X86CallingConv.td puts SRet into EAX though it should push the address on stack 2) Clang problem #1: Returning a struct with non-trivial default constructor should imply "sret"; Currently, only the presence of a copy constructor or destructor implies sret. 3) Clang problem #2: Handling of __cdecl instance methods - to be investigated later. I have small patches for (1) and (2), but their current versions likely break the Itanium ABI. I'll send a couple of e-mails to llvm-dev and cfe-dev for design suggestions soon; as well as adding a test for (2).
A fix for everything I know except for __cdecl methods has been committed as r179681. I'll file an issue for __cdecl soon.
(See also r178291, r178634)
Re: cdecl - see PR15768