Bug 224863 - clang 6.0.0 crashes building emulators/wine: Assertion failed: ((size_t)StackDisp < Context.ArgStoreVector.size() && "Function call has more parameters than the stack is adjusted for.")
Summary: clang 6.0.0 crashes building emulators/wine: Assertion failed: ((size_t)Stack...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Dimitry Andric
URL:
Keywords: toolchain
: 225352 (view as bug list)
Depends on:
Blocks: 224669
  Show dependency tree / graph
 
Reported: 2018-01-03 15:15 UTC by Jan Beich
Modified: 2018-04-02 18:52 UTC (History)
2 users (show)

See Also:

Attachments
dlls/oleacc/oleacc_classes_p.c (compressed, preprocessed, from wine-3.0-rc4) (292.14 KB, application/x-xz)
2018-01-03 15:15 UTC, Jan Beich 		no flags Details
command line args (for clang 6.0) (2.52 KB, text/plain)
2018-01-03 15:16 UTC, Jan Beich 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2018-01-03 15:15:50 UTC 
Created attachment 189364 [details]
dlls/oleacc/oleacc_classes_p.c (compressed, preprocessed, from wine-3.0-rc4)

gmake[2]: Entering directory '/wrkdirs/usr/ports/emulators/wine-devel/work/wine-3.0-rc4/dlls/oleacc'
cc -m64 -c -o oleacc_classes_p.o oleacc_classes_p.c -I. -I../../include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe \
  -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \
  -Wstrict-prototypes -Wtype-limits -Wvla -Wwrite-strings -Wpointer-arith -I/usr/local/include \
  -O2 -pipe  -fstack-protector -fno-strict-aliasing
[...]
Assertion failed: ((size_t)StackDisp < Context.ArgStoreVector.size() && "Function call has more parameters than the stack is adjusted for."), function collectCallInfo, file /poudriere/jails/projects/clang600-import-amd64/usr/src/contrib/llvm/lib/Target/X86/X86CallFrameOptimization.cpp, line 449.
cc: error: unable to execute command: Abort trap
cc: error: clang frontend command failed due to signal (use -v to see invocation)
FreeBSD clang version 6.0.0 (trunk 321545) (based on LLVM 6.0.0svn)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin

http://package18.nyi.freebsd.org/data/headamd64PR224669-default/2018-01-02_08h32m49s/logs/errors/wine-2.0.3,1.log
http://package18.nyi.freebsd.org/data/headamd64PR224669-default/2018-01-02_08h32m49s/logs/errors/wine-devel-3.0.r4,1.log
Comment 1 Jan Beich freebsd_committer freebsd_triage 2018-01-03 15:16:24 UTC 
Created attachment 189365 [details]
command line args (for clang 6.0)
Comment 2 Dimitry Andric freebsd_committer freebsd_triage 2018-01-03 23:45:15 UTC 
Reported upstream: https://bugs.llvm.org/show_bug.cgi?id=35814
Comment 3 Dimitry Andric freebsd_committer freebsd_triage 2018-01-21 15:09:36 UTC 
*** Bug 225352 has been marked as a duplicate of this bug. ***
Comment 4 Dimitry Andric freebsd_committer freebsd_triage 2018-01-21 15:17:49 UTC 
(In reply to Dimitry Andric from comment #2)
> Reported upstream: https://bugs.llvm.org/show_bug.cgi?id=35814

Unfortunately upstream has closed this bug as a duplicate, saying:

> As discussed on the patch, I think this is actually PR31362, which is that
> ms_abi just doesn't work for structs larger than 8 bytes. It uses the Linux
> calling convention logic, which is wrong.
> 
> We could keep open a separate issue for making byval not generate crashing
> code on win64, but I don't think it's worth it.
> 
> *** This bug has been marked as a duplicate of bug 31362 ***

and then, in a later comment:

> To work around it in wine itself, simply pass a pointer to the aggregate in
> question instead of passing it by value. That will actually be ABI
> compatible with code generated by other compilers that expect to receive the
> aggregate by value.
> 
> Consider:
> 
> struct TwoWords { __int64 x, y; };
> void f(int a, int b, int c, int d, struct TwoWords two) {
>   printf("%d, %d\n", two.x, two.y);
> }
> 
>   0000000000000000: 48 8B 54 24 28     mov         rdx,qword ptr [rsp+28h]
>   0000000000000005: 48 8D 0D 00 00 00  lea        
> rcx,[??_C@_07MHMABKGB@?$CFd?0?5?$CFd?6?$AA@]
>                     00
>   000000000000000C: 4C 8B 42 08        mov         r8,qword ptr [rdx+8]
>   0000000000000010: 48 8B 12           mov         rdx,qword ptr [rdx]
>   0000000000000013: E9 00 00 00 00     jmp         printf
> 
> Note how RDX contains the address of 'two', so only the address is passed in
> this calling convention.

See also https://bugs.llvm.org/show_bug.cgi?id=31362.

Help is appreciated, as I do not want to pretend to know too much about Windows calling conventions. :)
Comment 5 Gerald Pfeifer freebsd_committer freebsd_triage 2018-01-21 17:27:54 UTC 
A compiler crashing is a compiler bug in any case (regardless of any
potential problems with the source code).

Also, how comes this worked with older versions of clang?  Do we need
to USE_GCC=yes in emulators/wine and emulators/wine-devel on FreeBSD 12
and later for the time being?
Comment 6 Dimitry Andric freebsd_committer freebsd_triage 2018-01-21 17:33:41 UTC 
(In reply to Gerald Pfeifer from comment #5)
> A compiler crashing is a compiler bug in any case (regardless of any
> potential problems with the source code).
> 
> Also, how comes this worked with older versions of clang?

As I reported upstream, after bisecting it turned out that https://reviews.llvm.org/rL320749 seems to cause the regression, but this change is not directly related to MS ABI features.  It likely just exposed an underlying bug in the MS ABI handling, which was simply never noticed before.

Therefore, requesting to revert this change will likely not be accepted, nor might it completely fix the failure.


> Do we need
> to USE_GCC=yes in emulators/wine and emulators/wine-devel on FreeBSD 12
> and later for the time being?

That would be the big hammer method, if we can't find anything else then that would possibly do it.  Maybe it helps to lower the optimization level on the particular .c file that it is asserting on, I haven't tried.
Comment 7 Dimitry Andric freebsd_committer freebsd_triage 2018-01-21 17:37:17 UTC 
(In reply to Dimitry Andric from comment #6)
> As I reported upstream, after bisecting it turned out that
> https://reviews.llvm.org/rL320749 seems to cause the regression,

Eh, sorry, that was https://reviews.llvm.org/rL316416 instead.
Comment 8 commit-hook freebsd_committer freebsd_triage 2018-01-26 09:14:33 UTC 
A commit references this bug:

Author: gerald
Date: Fri Jan 26 09:14:22 UTC 2018
New revision: 459979
URL: https://svnweb.freebsd.org/changeset/ports/459979

Log:
  Use GCC instead of the system compiler on FreeBSD 12.x with clang 6.0.0
  since that compiler crashes.

  (There may be a way to work around this in Wine, but that is unlikely
  to be accepted for the stable release branch.)

  PR:		224863

Changes:
  head/emulators/wine/Makefile
Comment 9 Jan Beich freebsd_committer freebsd_triage 2018-01-26 09:18:47 UTC 
The crash only affects amd64. Why did you apply workaround for i386 as well?
Comment 11 Gerald Pfeifer freebsd_committer freebsd_triage 2018-01-26 10:43:28 UTC 
(In reply to Jan Beich from comment #9)
> The crash only affects amd64. Why did you apply workaround for i386 as well?

Simplicity and consistency (and robustness).  But I can limit this,
since you seem to prefer that.
Comment 12 commit-hook freebsd_committer freebsd_triage 2018-01-26 10:44:50 UTC 
A commit references this bug:

Author: gerald
Date: Fri Jan 26 10:43:50 UTC 2018
New revision: 459985
URL: https://svnweb.freebsd.org/changeset/ports/459985

Log:
  Limit the workaround for clang crashing to amd64.

  PR:		224863
  Reported by:	jbeich

Changes:
  head/emulators/wine/Makefile
Comment 13 commit-hook freebsd_committer freebsd_triage 2018-01-28 14:48:18 UTC 
A commit references this bug:

Author: gerald
Date: Sun Jan 28 14:47:36 UTC 2018
New revision: 460219
URL: https://svnweb.freebsd.org/changeset/ports/460219

Log:
  Use GCC instead of the system compiler on FreeBSD 12.x/AMD64 with
  clang 6.0.0 since that compiler crashes.

  PR:		224863

Changes:
  head/emulators/wine-devel/Makefile
Comment 14 commit-hook freebsd_committer freebsd_triage 2018-02-08 21:12:20 UTC 
A commit references this bug:

Author: dim
Date: Thu Feb  8 21:11:48 UTC 2018
New revision: 329033
URL: https://svnweb.freebsd.org/changeset/base/329033

Log:
  Pull in r324594 from upstream clang trunk (by Alexander Ivchenko):

    Fix for #31362 - ms_abi is implemented incorrectly for values >=16
    bytes.

    Summary:
    This patch is a fix for following issue:
    https://bugs.llvm.org/show_bug.cgi?id=31362 The problem was caused by
    front end lowering C calling conventions without taking into account
    calling conventions enforced by attribute. In this case win64cc was
    no correctly lowered on targets other than Windows.

    Reviewed By: rnk (Reid Kleckner)

    Differential Revision: https://reviews.llvm.org/D43016

    Author: belickim <mateusz.belicki@intel.com>

  This fixes clang 6.0.0 assertions when building the emulators/wine and
  emulators/wine-devel ports, and should also make it use the correct
  Windows calling conventions.  Bump __FreeBSD_version to make the fix
  easy to detect.

  PR:		224863
  MFC after:	3 months
  X-MFC-With:	r327952

Changes:
  head/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp
  head/sys/sys/param.h
Comment 15 Dimitry Andric freebsd_committer freebsd_triage 2018-02-08 21:33:40 UTC 
(In reply to commit-hook from comment #14)
> URL: https://svnweb.freebsd.org/changeset/base/329033
...
>   This fixes clang 6.0.0 assertions when building the emulators/wine and
>   emulators/wine-devel ports, and should also make it use the correct
>   Windows calling conventions.  Bump __FreeBSD_version to make the fix
>   easy to detect.

So please check for __FreeBSD_version >= 1200057, after which the wine ports should compile fine with clang 6.0.0.  I also did some light testing, and they seemed to run OK for me.
Comment 16 commit-hook freebsd_committer freebsd_triage 2018-02-08 21:54:55 UTC 
A commit references this bug:

Author: gerald
Date: Thu Feb  8 21:54:25 UTC 2018
New revision: 461272
URL: https://svnweb.freebsd.org/changeset/ports/461272

Log:
  The issue of clang 6.0.0 crashing when building Wine was just resolved
  with __FreeBSD_version 1200057, so reduce the window of versions we
  need a workaround for to 1200056.

  PR:		224863
  Thanks to:	dim

Changes:
  head/emulators/wine-devel/Makefile
Comment 17 Dimitry Andric freebsd_committer freebsd_triage 2018-02-08 22:57:18 UTC 
(In reply to commit-hook from comment #16)
> Changes:
>   head/emulators/wine-devel/Makefile

Hm, doesn't emulators/wine have the same workaround?
Comment 18 commit-hook freebsd_committer freebsd_triage 2018-02-18 20:46:04 UTC 
A commit references this bug:

Author: gerald
Date: Sun Feb 18 20:45:08 UTC 2018
New revision: 462264
URL: https://svnweb.freebsd.org/changeset/ports/462264

Log:
  The issue of clang 6.0.0 crashing when building Wine was resolved with
  __FreeBSD_version 1200057, so reduce the window of versions we need a
  workaround for to just 1200056.

  In the mid term I plan on removing this workaround, but let's keep it
  in place for a bit longer for the sake of users on an affected snapshot.

  PR:		224863
  Thanks to:	dim

Changes:
  head/emulators/wine/Makefile
Comment 19 Gerald Pfeifer freebsd_committer freebsd_triage 2018-02-18 21:16:37 UTC 
(In reply to Dimitry Andric from comment #17)
> Hm, doesn't emulators/wine have the same workaround?

Yes, you're right.  Sorry it took a couple of days (but it wasn't
broken, just had a dependency that wasn't needed any longer.)

I'll leave the workarounds in for a while and then will pull them
out later.
Comment 20 commit-hook freebsd_committer freebsd_triage 2018-04-02 14:15:52 UTC 
A commit references this bug:

Author: gerald
Date: Mon Apr  2 14:15:25 UTC 2018
New revision: 466230
URL: https://svnweb.freebsd.org/changeset/ports/466230

Log:
  Remove workaround (really: requiring GCC) for a clang 6.0.0 crash on
  FreeBSD revision 1200056.

  PR:		224863

Changes:
  head/emulators/wine-devel/Makefile
Comment 21 commit-hook freebsd_committer freebsd_triage 2018-04-02 18:52:40 UTC 
A commit references this bug:

Author: gerald
Date: Mon Apr  2 18:52:34 UTC 2018
New revision: 466248
URL: https://svnweb.freebsd.org/changeset/ports/466248

Log:
  Remove the workaround for a clang 6.0.0 crash on FreeBSD version 1200056
  (where we'd use GCC), now that version 1200057 which fixes that issue is
  eight weeks old.

  PR:		224863

Changes:
  head/emulators/wine/Makefile