[MSVC] CMake: Prepare for LLVM 5.0 by kinke · Pull Request #2279 · ldc-developers/ldc

kinke · 2017-08-17T21:46:11Z

Some LLVM libs were missing from the linker flags for current LLVM 5.0. It was only during troubleshooting that I noticed the MSVC special case in FindLLVM.cmake. Getting rid of it entirely made the issues disappear. I checked llvm-config.exe, and it's returning appropriate switches for the MS toolchain, no need for special processing.

This apparently also leads to LLVM_CXXFLAGS being used for the first time when building with MSVC. These include /W4, a level further up from CMake's default /W3, so I disabled a few more frequent warnings.

I also had to link in a lib from Microsoft's Debug Info Access SDK manually. As the VS command prompt unfortunately doesn't even include the DIA SDK dir in the LIBPATH environment variable, I even had to specify a full path, relying on the VSINSTALLDIR environment variable.

Some LLVM libs were missing from the linker flags for current LLVM 5.0. It was only during troubleshooting that I noticed the MSVC special case in FindLLVM.cmake. Getting rid of it entirely made the issues disappear. I checked llvm-config.exe, and it's returning appropriate switches for the MS toolchain, no need for special processing. This apparently also leads to LLVM_CXXFLAGS being used for the first time when building with MSVC. These include /W4, a level further up from CMake's default /W3, so I disabled a few more frequent warnings. I also had to link in a lib from Microsoft's Debug Info Access SDK manually. As the VS command prompt unfortunately doesn't even include the DIA SDK dir in the LIBPATH environment variable, I even had to specify a full path, relying on the `VSINSTALLDIR` environment variable.

kinke · 2017-08-17T22:09:13Z

AppVeyor: The command line is too long. ...

One advantage is that the D modules aren't recompiled if only .cpp files have been modified. Another advantage is that the 2 separate command lines are shorter than the single combined one, preventing issues on Windows wrt. max command line length.

rainers · 2017-08-18T06:05:05Z

I also had to link in a lib from Microsoft's Debug Info Access SDK manually.

I never had trouble with this when building with LLVM master. I guess that's because I'm not compiling with LLD integration. Does it have debug info support now?

rainers · 2017-08-18T06:09:25Z

cmake/Modules/FindLLVM.cmake

-        if(${LLVM_VERSION_STRING} MATCHES "^3\\.[0-9][\\.0-9A-Za-z]*")
-            # Versions below 4.0 do not support component debuginfomsf
-            list(REMOVE_ITEM LLVM_FIND_COMPONENTS "debuginfomsf" index)
-        endif()


Removing this list is very much appreciated. I had to add the binaryformat library recently, too (probably caused by the selected targets).

kinke · 2017-08-18T08:27:28Z

Yep binaryformat was missing for me too. I disabled the LLD integration (needs #2148 otherwise to work) but still needed diaguids.lib due to a few class and interface GUIDs.

kinke · 2017-08-18T08:29:52Z

The remaining AppVeyor issue is unrelated btw, realizing that cost me some hours yesterday - they probably changed something in their image (and once again didn't mention anything in their blog/Twitter feed/...). I built this PR yesterday successfully with both LLVM 5.0 and 4.0.1.

kinke · 2017-08-18T18:11:25Z

AppVeyor upgraded to the latest VS + WinSDK update and since then master has been red. Upgrading myself right now...

kinke · 2017-08-18T19:56:24Z

I have the same issue now, even after rebuilding LLVM. LDC segfaults during a GC collection triggered by a druntime module ctor, in gc.impl.conservative.gc.Gcx.mark() called by rt.tlsgc.scan()...

… x86

kinke · 2017-08-18T23:58:43Z

Might have something to do with the .tls section disappearing from ldc2.exe when linking with VS2017 v15.3.1 (but the magic _tls_start/end symbols are still there and bracket a plausibly small range). @rainers: You're the expert for that kind of thing, right? ;)

rainers · 2017-08-19T06:13:40Z

Might have something to do with the .tls section disappearing from ldc2.exe when linking with VS2017 v15.3.1 (but the magic _tls_start/end symbols are still there and bracket a plausibly small range).

Oh, boy. Sounds like this will break every program linked against/with v15.3.1 ;-(

@rainers: You're the expert for that kind of thing, right? ;)

I'll have a look...

rainers · 2017-08-19T09:30:59Z

The problem seems to be that .tls is no longer an image section, but only a segment in the .data section. TLS access seems to be ok, but _tls_start and _tls_end are now evaluated relative to the .data section:

int x = 7;

extern(C) extern
{
	int _tls_start;
	int _tls_end;
}

void[] initTLSRanges() nothrow @nogc
{
	auto pbeg = cast(void*)&_tls_start;
	auto pend = cast(void*)&_tls_end;
	return pbeg[0 .. pend - pbeg];
}

void main()
{
	printf("%p: %d\n", &x, x);
	
	auto r = initTLSRanges();
	printf("%p: %d\n", r.ptr, r.length);
}

prints

000001B0638D7470: 7
000001B0638E60E0: 233

i.e. the variable is not in the TLS range (but it's the range that is wrong here). Happens with dmd, too, if using V2017 v15.3.1. This seems to be a linker issue as exchanging the vc library with the one from VS2015 doesn't help.

kinke · 2017-08-19T12:12:33Z

Thanks for the analysis. So if we were to work around that, the host compiler's druntime would have to be patched first, thanks MS, very much appreciated.
I can't even downgrade to 15.2. Needed to uninstall VS2017 completely and download the 15.0 installer (no installers for the previous updates). I can only upgrade directly to 15.3.1 later on. And I thought the whole reason for the new VS2017 system with less registry keys was to enable different parallel installations...

rainers · 2017-08-19T13:18:45Z

So if we were to work around that, the host compiler's druntime would have to be patched first

Depends on the workaround. Maybe we can enforce moving the segment back into the .tls section.

I can't even downgrade to 15.2.

I guess my VS2017 is broken now, too. (I mostly use 2013 and 2015, though).

They also changed the vcvars64.bat to always switch to %HOME%\source. That breaks a good deal of my batches, probably also some builds by Visual D.

kinke · 2017-08-25T19:36:12Z

Does it have debug info support now?

From https://llvm.org/docs/CMake.html#llvm-specific-variables:

LLVM_ENABLE_DIA_SDK:BOOL
    Enable building with MSVC DIA SDK for PDB debugging support. Available only with MSVC. Defaults to ON.

Don't know what that means exactly and don't have time to dig into it right now, but I take it as a good sign. ;)

kinke · 2017-08-26T16:27:35Z

Btw, Microsoft released 15.3.2 4 days later. I don't dare testing that yet though.

They also changed the vcvars64.bat to always switch to %HOME%\source. That breaks a good deal of my batches

Yep, mine too. Additionally, the $(Platform) MSBuild variable apparently isn't defined anymore...

rainers · 2017-08-26T20:49:13Z

Btw, Microsoft released 15.3.2 4 days later. I don't dare testing that yet though.

I tried it, no improvement for our issues. cv2pdb also doesn't work anymore...

rainers · 2017-08-27T16:21:09Z

So if we were to work around that, the host compiler's druntime would have to be patched first, thanks MS, very much appreciated.

I have not found any work around, so here is the runtime change: dlang/druntime#1910

rainers · 2017-09-01T11:06:03Z

I have not found any work around, so here is the runtime change: dlang/druntime#1910

I stumbled over a workaround: there is a hidden linker switch /noopttls that restores the old behaveour. Seems to exist starting with VS2015.

kinke · 2017-09-01T12:58:07Z

Oh wow, how did you find out about that one? No single Google hit... ;)

rainers · 2017-09-01T16:32:24Z

Oh wow, how did you find out about that one? No single Google hit... ;)

Staring at strings in link.exe. You can find a quite some more undocumented switches this way.

kinke force-pushed the cmake2 branch from 4ac4fb1 to acfa2df Compare August 17, 2017 21:49

kinke force-pushed the cmake2 branch from 4b335ab to 25987cc Compare August 18, 2017 00:04

rainers reviewed Aug 18, 2017

View reviewed changes

AppVeyor: Switch to previous VS2017 image for x64 and test VS2015 for…

c60ba68

… x86

kinke merged commit f4038aa into ldc-developers:master Aug 19, 2017

kinke deleted the cmake2 branch August 19, 2017 19:58

rainers mentioned this pull request Aug 28, 2017

fix issue 17788: MSCOFF: TLS broken with VS2017 15.3.1 dlang/druntime#1910

Merged

Uh oh!

Conversation

kinke commented Aug 17, 2017

Uh oh!

kinke commented Aug 17, 2017

Uh oh!

rainers commented Aug 18, 2017

Uh oh!

rainers Aug 18, 2017

Choose a reason for hiding this comment

Uh oh!

kinke commented Aug 18, 2017

Uh oh!

kinke commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinke commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinke commented Aug 18, 2017

Uh oh!

kinke commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rainers commented Aug 19, 2017

Uh oh!

rainers commented Aug 19, 2017

Uh oh!

kinke commented Aug 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rainers commented Aug 19, 2017

Uh oh!

kinke commented Aug 25, 2017

Uh oh!

kinke commented Aug 26, 2017

Uh oh!

rainers commented Aug 26, 2017

Uh oh!

rainers commented Aug 27, 2017

Uh oh!

rainers commented Sep 1, 2017

Uh oh!

kinke commented Sep 1, 2017

Uh oh!

rainers commented Sep 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kinke commented Aug 18, 2017 •

edited

Loading

kinke commented Aug 18, 2017 •

edited

Loading

kinke commented Aug 18, 2017 •

edited

Loading

kinke commented Aug 19, 2017 •

edited

Loading