Created attachment 310766 [details] Patch to change to use Visual Studio 2017 for building Switch the target compiler and build environment to Visual Studio 2017. N.B. The attached patch seems to cause MiniBrowser at startup with the following stack trace: WebKit\WebKitBuild\Debug\DerivedSources\WebCore\HTMLNames.cpp WTF.dll!WTFCrash() Line 292 C++ WebKit.dll!WTF::StringImpl::assertHashIsCorrect() Line 889 C++ > WebKit.dll!WebCore::HTMLNames::init() Line 1666 C++ WebKit.dll!WebCore::Frame::Frame(WebCore::Page & page, WebCore::HTMLFrameOwnerElement * ownerElement, WebCore::FrameLoaderClient & frameLoaderClient) Line 175 C++ WebKit.dll!WebCore::MainFrame::MainFrame(WebCore::Page & page, WebCore::PageConfiguration & configuration) Line 56 C++ WebKit.dll!WebCore::MainFrame::create(WebCore::Page & page, WebCore::PageConfiguration & configuration) Line 68 C++ WebKit.dll!WebCore::Page::Page(WebCore::PageConfiguration && pageConfiguration) Line 270 C++ WebKit.dll!WebView::initWithFrame(tagRECT frame, wchar_t * frameName, wchar_t * groupName) Line 3122 C++ MiniBrowserLib.dll!MiniBrowser::prepareViews(HWND__ * mainWnd, const tagRECT & clientRect, wchar_t * const & requestedURL, HWND__ * & viewHwnd) Line 100 C++ MiniBrowserLib.dll!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 160 C++ MiniBrowserLib.dll!dllLauncherEntryPoint(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 857 C++ MiniBrowser.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 249 C++
Your callstack looks similar with Bug 171800.
Yes, I thought so too. I've experimented with re-enabling static constructors by removing the line below from Source/WTF/wtf/StaticConstructors and it then works. #pragma init_seg(".unwantedstaticinits")
Also, I've managed to launch MiniBrowser in the debugger and it appears that the aData statics aren't initialized, unless that #pragma is removed from StaticConstructors.h
The init_seg trick doesn't work for VS 2017 as expected. How should we do? 1) Look for the solution or ask MS how to solve this 2) Stop using the init_seg trick for both VS 2015 and 2017 3) Use the init_seg trick only for VS 2015 Anyone have an idea?
Oh, no. Just removing pragma init_seg causes a crash when destructors are called at shutdown.
Created attachment 316752 [details] WIP patch (Moved #pragma init_seg) In the compilation units of generated *Names.cpp files, some global variable need to be constructed, but other doesn't. Put the global variable needing to be constructed in the front of one doesn't, and put #pragma init_seg just after the former.
For a long term solution, I think we should use LazyNeverDestroyed instead of DEFINE_GLOBAL hack.
Sounds very interesting. I'll try it in Bug 174979. Thank you.
The pragma init_seg issue will be solved in Bug 175209. This bug doesn't block Bug 174195 for now.
Created attachment 327998 [details] Require VS2017 for building This merges together pieces of the earlier patch and https://bug-174195-attachments.webkit.org/attachment.cgi?id=314698 and updates the webkitdirs pieces to deal with the vswhere changes. I ended up not giving a specific location relating to the location of VS2017 for vcvars.bat in some messages as both previous patches seemed to update the text to a location I didn't actually get with a VS2017 install and from my install it seemed like the location would be dependent on the particular VS2017 flavor (enterprise, professional, etc) you installed. Uploading first to throw at bots.
Attachment 327998 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 328010 [details] Require VS2017 for building
(In reply to Stephan Szabo from comment #12) > Created attachment 328010 [details] > Require VS2017 for building Does this patch need rebasing?
Created attachment 328508 [details] Require VS2017 for building Rebased to re-check on bot
Comment on attachment 328508 [details] Require VS2017 for building Looks good! R=me. Please wait until bots are green before committing.
Created attachment 328518 [details] Require VS2017 for building - reup
Created attachment 328539 [details] Require VS2017 for building - setting reviewer based on pre-reupload r=me
Comment on attachment 328539 [details] Require VS2017 for building - setting reviewer based on pre-reupload r=me Clearing flags on attachment: 328539 Committed r225563: <https://trac.webkit.org/changeset/225563>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35896610>
<rdar://problem/35896611>
*** Bug 174195 has been marked as a duplicate of this bug. ***