New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 773476 link

Starred by 13 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment

Chrome builds should support an SDK later than Spring Creators Update SDK

Project Member Reported by brucedaw...@chromium.org, Oct 10 2017

Issue description

VS 2017 Update 4 ships with this SDK and it appears to have a few problems when building Chrome. We should address these.

 
I have found two problems so far. One appears to be a component-build only issue but that is little consolation:

https://developercommunity.visualstudio.com/content/problem/128961/idwritetextanalysissource-error-in-vs-2017-update.html

The other is a straightforward conflict in crashpad which I will land a fix for.

Cc: msw@chromium.org
msw@ - any thoughts on the text_analysis_source.cc errors triggered by the new SDK? There is clearly a bug in the new SDK but I'm wondering if you can think of any practical mitigations.

Comment 3 by msw@chromium.org, Oct 11 2017

Cc: kulshin@chromium.org jsc...@chromium.org ananta@chromium.org scottmg@chromium.org
I don't see FillArrayWithIid in ui/gfx/win/text_analysis_source.cc?
Also, I haven't built on windows in a while, and don't have any good ideas offhand :(
(maybe we could polyfill stuff missing from the interface? I haven't looked close)
kulshin@ may have some ideas... CC'ing others on some reviews there too.
Labels: -Pri-3 Pri-2
kulshin@ has left the building...

The build error happens on component builds because we declspec(dllexport) the class which forces the compiler to instantiate everything in the class, thus exposing problems in the templates that might not otherwise matter.

I suspect that we can avoid the problem by exporting an interface rather than exporting all of IDWriteTextAnalysisSource.

Comment 5 by mark@chromium.org, Oct 11 2017

Cc: mark@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65e7acb2c57731ba7d22f2cda10308cc0b5bcba8

commit 65e7acb2c57731ba7d22f2cda10308cc0b5bcba8
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Wed Oct 11 02:39:44 2017

Update CLIENT_SDK declaration for 10.0.16299.0 SDK

The Fall Creators Update SDK (10.0.16299.0) adds a declaration for
CLIENT_SDK that is slightly different from the forward declaration used
by crashpad. This updates the crashpad forward declaration so that it
doesn't conflict.

Bug:  773476 
Change-Id: I0e3f55ad6021f2cd51aef9a8d684f0664a158dc8
Reviewed-on: https://chromium-review.googlesource.com/710522
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507871}
[modify] https://crrev.com/65e7acb2c57731ba7d22f2cda10308cc0b5bcba8/third_party/crashpad/README.chromium
[modify] https://crrev.com/65e7acb2c57731ba7d22f2cda10308cc0b5bcba8/third_party/crashpad/crashpad/util/win/nt_internals.cc

Cc: thakis@chromium.org
One more problem - the python based message compiler is giving different results. Error output attached.

I'll have a packaged toolchain tomorrow for easier experimentation. It looks like the only big issue is the one I filed a bug for in comment #2.

messaging_errors.txt
6.6 KB View Download
Feedback from Microsoft is that dllexport of their WRL templates is unlikely to be fixed, so please don't do that. I'll see about creating a workaround.

Meanwhile another issue has popped up - clang doesn't like the new SDK. Some initial errors include:

FAILED: obj/base/base/win_util.obj 
E:\b\c\goma_client/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes  @obj/base/base/win_util.obj.rsp /c ../../base/win/win_util.cc /Foobj/base/base/win_util.obj /Fd"obj/base/base_cc.pdb"
In file included from ../../base/win/win_util.cc:29:
In file included from e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.UI.ViewManagement.h:253:
In file included from e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.Devices.Enumeration.h:253:
In file included from e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.ApplicationModel.Background.h:253:
In file included from e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.ApplicationModel.Activation.h:256:
e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.ApplicationModel.Contacts.h(7088,46):  error: missing ',' between enumerators
                    ContactFieldType_Location
                                             ^
e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.ApplicationModel.Contacts.h(7090,41):  error: expected '= constant-expression' or end of enumerator definition
                    DEPRECATEDENUMERATOR("Location  may be altered or unavailable for releases after Windows 8.1. Instead, use Address.")
                                        ^
e:\b\c\win_toolchain\vs_files\419b2ec6150201287022cd13808d9855db6c753c\win_sdk\bin\..\..\win_sdk\include\10.0.16299.0\winrt/Windows.ApplicationModel.Contacts.h(7097,52):  error: missing ',' between enumerators
                    ContactFieldType_InstantMessage
                                                   ^


Project Member

Comment 9 by bugdroid1@chromium.org, Oct 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/crashpad/crashpad.git/+/4c4e67952c93c846210740d061fe3bac2bdf5816

commit 4c4e67952c93c846210740d061fe3bac2bdf5816
Author: Mark Mentovai <mark@chromium.org>
Date: Wed Oct 11 22:39:00 2017

win: 10.0.16299.0 SDK compatibility

This corresponds to Windows 10 version 1709 (Fall Creators Update,
“Redstone 3”).

While compiling util/win/nt_internals.cc:

…\crashpad\crashpad\util\win\nt_internals.cc(22): error C2371: 'CLIENT_ID': redefinition; different basic types
c:\program files (x86)\windows kits\10\include\10.0.16299.0\um\winternl.h(83): note: see declaration of 'CLIENT_ID'

The CLIENT_ID structure, which should have been part of the SDK to begin
with, has been added. Provide a compatible definition in <winternl.h>.

Bug:  chromium:773476 
Change-Id: Iafc77f8cffd06d1194fc909bad587f1ffd1687a2
Reviewed-on: https://chromium-review.googlesource.com/711415
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>

[modify] https://crrev.com/4c4e67952c93c846210740d061fe3bac2bdf5816/compat/compat.gyp
[add] https://crrev.com/4c4e67952c93c846210740d061fe3bac2bdf5816/compat/win/winternl.h
[modify] https://crrev.com/4c4e67952c93c846210740d061fe3bac2bdf5816/util/win/nt_internals.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98392ed2d255753c2c8ca5b2f31c333f75b579a8

commit 98392ed2d255753c2c8ca5b2f31c333f75b579a8
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Oct 12 16:14:56 2017

Force building with *spring* Creators Update SDK

The Fall Creators Update SDK has various bugs/incompatibilities that
cause errors when building Chromium. This will not affect Google
employees who use a packaged toolchain but good affect anybody who set
DEPOT_TOOLS_WIN_TOOLCHAIN=0 and is therefore using the system install
of VS 2017. This change forces vcvarsall.bat to use the 15063 SDK
instead of the latest-installed.

This should be a temporary measure that can be undone as we land
workarounds for the incompatibilities or get fixed SDK versions.

Bug:  773476 
Change-Id: If5b89186d5053c878f5c771a489fe2082d9b2d68
Reviewed-on: https://chromium-review.googlesource.com/714366
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508362}
[modify] https://crrev.com/98392ed2d255753c2c8ca5b2f31c333f75b579a8/build/toolchain/win/setup_toolchain.py

There are at least two clang related errors. One is from:

#if _MSC_VER >= 1900
#define DEPRECATED(x) [[deprecated(x)]]
#define DEPRECATEDENUMERATOR(x) [[deprecated(x)]]

This is easily avoided with DISABLE_WINRT_DEPRECATION.

The other error is less obvious and is shown here:

>ninja -C out\clang obj/third_party/angle/libANGLE/driver_utils.obj
ninja.exe -C out\clang obj/third_party/angle/libANGLE/driver_utils.obj -j 960 -l 48
ninja: Entering directory `out\clang'
[1 processes, 1/1 @ 0.4/s : 2.492s ] CXX obj/third_party/angle/libANGLE/driver_utils.obj
FAILED: obj/third_party/angle/libANGLE/driver_utils.obj
C:\goma\goma-win64/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes  @obj/third_party/angle/libANGLE/driver_utils.obj.rsp /c ../../third_party/angle/src/libANGLE/renderer/driver_utils.cpp /Foobj/third_party/angle/libANGLE/driver_utils.obj /Fd"obj/third_party/angle/libANGLE_cc.pdb"
In file included from ../../third_party/angle/src/libANGLE/renderer/driver_utils.cpp:11:
In file included from ../../third_party/angle/src\libANGLE/renderer/driver_utils.h:12:
In file included from ../../third_party/angle/src\libANGLE/angletypes.h:12:
In file included from ../../third_party/angle/src\common/bitset_utils.h:17:
In file included from ../../third_party/angle/src\common/angleutils.h:12:
In file included from ../../third_party/angle/src\common/platform.h:71:
In file included from c:\program files (x86)\windows kits\10\include\10.0.16299.0\winrt\wrl.h:16:
c:\program files (x86)\windows kits\10\include\10.0.16299.0\winrt\wrl\implements.h(944,16):  error: ambiguous conversion from derived class 'Microsoft::WRL::Details::ImplementsHelper<Microsoft::WRL::RuntimeClassFlags<3>, true, Microsoft::WRL::CloakedIid<IMarshal> >' to base class 'Microsoft::WRL::Details::ImplementsHelper<Microsoft::WRL::RuntimeClassFlags<3>, true>':
    struct Microsoft::WRL::Details::ImplementsHelper<struct Microsoft::WRL::RuntimeClassFlags<3>, true, struct Microsoft::WRL::CloakedIid<struct IMarshal> > -> ImplementsHelper<struct Microsoft::WRL::RuntimeClassFlags<3>, true, struct IMarshal> -> ImplementsHelper<struct Microsoft::WRL::RuntimeClassFlags<3>, true>
    struct Microsoft::WRL::Details::ImplementsHelper<struct Microsoft::WRL::RuntimeClassFlags<3>, true, struct Microsoft::WRL::CloakedIid<struct IMarshal> > -> ImplementsHelper<struct Microsoft::WRL::RuntimeClassFlags<3>, true>
        return ImplementsHelper<RuntimeClassFlagsT, true, TInterfaces...>::CanCastTo(riid, ppv, pRefDelegated);
               ^~~~~~~~~~~~~~~~
c:\program files (x86)\windows kits\10\include\10.0.16299.0\winrt\wrl\implements.h(1236,95):  note: in instantiation of member function 'Microsoft::WRL::Details::ImplementsHelper<Microsoft::WRL::RuntimeClassFlags<3>, true, Microsoft::WRL::CloakedIid<IMarshal> >::CanCastTo' requested here
        return Details::ImplementsHelper<RuntimeClassFlags<flags>, true, I0, TInterfaces...>::CanCastTo(riid, ppv);
                                                                                              ^
c:\program files (x86)\windows kits\10\include\10.0.16299.0\winrt\wrl\implements.h(1279,23):  note: in instantiation of member function 'Microsoft::WRL::Implements<Microsoft::WRL::RuntimeClassFlags<3>, Microsoft::WRL::CloakedIid<IMarshal> >::CanCastTo' requested here
        return Super::CanCastTo(riid, ppv);
                      ^
1 error generated.
ninja: build stopped: subcommand failed.

Re comment 7: I can't reproduce the message_compiler problem. From the diff, it looks like line endings. Are you synced past https://chromium-review.googlesource.com/697425 locally? I could imagine that might help.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f48c21352588b61d857897d0d1c7d695edcbcdf

commit 8f48c21352588b61d857897d0d1c7d695edcbcdf
Author: Nico Weber <thakis@chromium.org>
Date: Fri Oct 13 17:47:03 2017

Add bat files to rerun midl and mc rules.

Bug:  773476 , 756607 
Change-Id: Ibb5ceb0aa96a993e76369802e53e0184b2dfab40
Reviewed-on: https://chromium-review.googlesource.com/719157
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508749}
[add] https://crrev.com/8f48c21352588b61d857897d0d1c7d695edcbcdf/third_party/win_build_output/remc.bat
[add] https://crrev.com/8f48c21352588b61d857897d0d1c7d695edcbcdf/third_party/win_build_output/remidl.bat

Re the deprecated thing, here's what's happening. The SDK headers try to check for C++14 availability and if so use the standardized deprecated attribute like so:

#if !defined(DISABLE_WINRT_DEPRECATION)
#if defined(__cplusplus)
#if __cplusplus >= 201402
#define DEPRECATED(x) [[deprecated(x)]]
#elif defined(_MSC_VER)
#if _MSC_VER >= 1900
#define DEPRECATED(x) [[deprecated(x)]]
#define DEPRECATEDENUMERATOR(x) [[deprecated(x)]]
#else
#define DEPRECATED(x) __declspec(deprecated(x))
#define DEPRECATEDENUMERATOR(x)
#endif // _MSC_VER >= 1900
#else // Not Standard C++ or MSVC, ignore the construct.
#define DEPRECATED(x)
#define DEPRECATEDENUMERATOR(x)
#endif  // C++ deprecation
#else // C - disable deprecation
#define DEPRECATED(x)
#define DEPRECATEDENUMERATOR(x)
#endif
#else // Deprecation is disabled
...


Which is cool! Except they only define DEPRECATED if __cplusplus >= 201402, and fail to define DEPRECATEDENUMERATOR (which they define in every other branch). cl.exe doesn't set __cplusplus to 201402 even though they support 14 without opt-in flag. clang-cl sets it correctly. The following program builds fine with cl but not with clang-cl:

#if __cplusplus >= 201402
#error foo
#endif



This suggests nobody ever tried to test that first branch, and they're shipping really really broken headers. It'd very much prefer if we could get the headers fixed upstream.
One possible workaround would be "if in ms mode and in system headers, pretend that __cplusplus evaluates to something older, since ms system headers are syntactically incorrect with __cplusplus = 201402".
D'oh! I'll file a bug against the SDK. Meanwhile I've updated crrev.com/c/717190 with an alternate fix - forcibly defining the macro that Microsoft forgot to.
I filed this bug for the failure to define DEPRECATEDENUMERATOR. Meanwhile, it should be easy to workaround.

https://developercommunity.visualstudio.com/content/problem/131391/154-fails-to-define-deprecatedenumerator-2.html
Blockedon: 774669
Re comment 12: I figured out why the message_compiler problem is intermittent.  crbug.com/774669  has details.
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00de4b5174c190b4d9f6120ef550a310ec740459

commit 00de4b5174c190b4d9f6120ef550a310ec740459
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Oct 13 23:21:23 2017

Force define DEPRECATEDENUMERATOR for clang

The Fall Creators Update SDK uses DEPRECATEDENUMERATOR to mark some
enums as being deprecated but in the __cplusplus >= 201402 case it
fails to define it! This change forcibly defines it to work around
this header-file bug. This change avoids dozens of errors in
Windows.ApplicationModel.Contacts.h.

VS bug is filed here:
https://developercommunity.visualstudio.com/content/problem/131391/154-fails-to-define-deprecatedenumerator-2.html

Bug:  773476 
Change-Id: I604228cac02f676e2c79e9a8e533fd332221b0cd
Reviewed-on: https://chromium-review.googlesource.com/717190
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508862}
[modify] https://crrev.com/00de4b5174c190b4d9f6120ef550a310ec740459/build/config/win/BUILD.gn

Comment 21 by mark@chromium.org, Oct 14 2017

Wow. I’m shocked that they define __cplusplus as 199711L.
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1690d39c1e6875377f4685b53a5423cb69e2947

commit c1690d39c1e6875377f4685b53a5423cb69e2947
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Mon Oct 16 20:02:46 2017

Updates to build with Fall Creators Update SDK

Some Microsoft::WRL classes in the Fall Creators Update SDK reference an
undefined function called FillArrayWithIidHelper. This is normally never
instantiated but __declspec(dllexport) of a class that derives from one
of these classes leads to compile errors, as reported here:

https://developercommunity.visualstudio.com/content/problem/128961/idwritetextanalysissource-error-in-vs-2017-update.html

The response so far from Microsoft has been "don't do that" which is not
totally unreasonable, especially given the compile cost of generating
and exporting all those unneeded functions. It appears that a small
number of factory functions can avoid the need to export these classes,
so that's what this change does.

Bug:  773476 
Change-Id: I7f201e56e64f880fc71d369a48ffdf6e07e2c5e3
Reviewed-on: https://chromium-review.googlesource.com/717480
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509146}
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/dwrite_font_proxy_init_impl_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/dwrite_font_proxy_win_unittest.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/font_fallback_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/font_fallback_win.h
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/content/child/dwrite_font_proxy/font_fallback_win_unittest.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/base/dragdrop/drag_source_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/base/dragdrop/drag_source_win.h
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/gfx/font_fallback_win.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/gfx/win/text_analysis_source.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/gfx/win/text_analysis_source.h
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/gfx/win/text_analysis_source_unittest.cc
[modify] https://crrev.com/c1690d39c1e6875377f4685b53a5423cb69e2947/ui/views/widget/desktop_aura/desktop_drag_drop_client_win.cc

Blockedon: 775174
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70155edf7d3f4ffa931adc3077968ee99b9462ac

commit 70155edf7d3f4ffa931adc3077968ee99b9462ac
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Oct 19 21:00:20 2017

VS 2017 15.4 with 10.0.15063.468 SDK

This change switches the VS 2017 package to use VS 2017 Update 4 while
still using the 10.0.15063.468 SDK. Update 4 fixes at least one code-gen
bug ( crbug.com/759402 ) but the 10.0.16299.0 SDK is still incompatible
with Chrome ( crbug.com/773476 ).

Packaging was done on a Windows Server 2016 VM, cleanly created for this
purpose.

Compiler was packaged up by downloading VS 2017 Update 4, from
https://www.visualstudio.com/vs/, and then passing these parameters to
the installer:

    --add Microsoft.VisualStudio.Workload.NativeDesktop
    --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
    --passive

Then the Windows 10.0.15063.468 SDK installer was run and everything was
installed except for the Windows Performance Toolkit, .Net Framework,
and the arm SDKs.

Then the packaging script was run like this:

    python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.15063.0

Bug:  773476 , 759402 
Change-Id: Ie2176b5ff765d9e5497f51a7b00c02fad04fb973
Reviewed-on: https://chromium-review.googlesource.com/727523
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510207}
[modify] https://crrev.com/70155edf7d3f4ffa931adc3077968ee99b9462ac/build/vs_toolchain.py

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/breakpad/breakpad/+/73d2773f9f455944e2cec1c0ad3ea9f5e6705361

commit 73d2773f9f455944e2cec1c0ad3ea9f5e6705361
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Oct 26 22:25:57 2017

Avoid skipping an initializer with a goto

C++ doesn't allow skipping initialization with a goto. This means that
this code is illegal:

  void func(bool b) {
    if(b) goto END;
    int value = 0; //error C2362 with /permissive-
    //... value used here
  END:
    return;
  }

Adding an extra scope makes the code legal. This problem is only
detected with /permissive- but now that compiling with this
switch is practical we might as well stay /permissive- clean:
https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/

Note that compiling /permissive- clean only works with the 10.0.16299.0
SDK which currently has other issues...

Bug:  773476 
Change-Id: I54e64aaef46d70a817cf7da272f76d9ae5f6a6f7
Reviewed-on: https://chromium-review.googlesource.com/740287
Reviewed-by: Mark Mentovai <mark@chromium.org>

[modify] https://crrev.com/73d2773f9f455944e2cec1c0ad3ea9f5e6705361/src/common/windows/pdb_source_line_writer.cc

Blockedon: 780056
Blockedon: 780323
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db45606398cf4389bf332b0cdcffd04e7de4a4f6

commit db45606398cf4389bf332b0cdcffd04e7de4a4f6
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Nov 03 23:48:27 2017

Revert "VS 2017 15.4 with 10.0.15063.468 SDK"

This reverts commit 70155edf7d3f4ffa931adc3077968ee99b9462ac.

Reason for revert: the 15.4 toolchain breaks incremental linking for
Chrome ( crbug.com/780161 )

Original change's description:
> VS 2017 15.4 with 10.0.15063.468 SDK
> 
> This change switches the VS 2017 package to use VS 2017 Update 4 while
> still using the 10.0.15063.468 SDK. Update 4 fixes at least one code-gen
> bug ( crbug.com/759402 ) but the 10.0.16299.0 SDK is still incompatible
> with Chrome ( crbug.com/773476 ).
> 
> Packaging was done on a Windows Server 2016 VM, cleanly created for this
> purpose.
> 
> Compiler was packaged up by downloading VS 2017 Update 4, from
> https://www.visualstudio.com/vs/, and then passing these parameters to
> the installer:
> 
>     --add Microsoft.VisualStudio.Workload.NativeDesktop
>     --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
>     --passive
> 
> Then the Windows 10.0.15063.468 SDK installer was run and everything was
> installed except for the Windows Performance Toolkit, .Net Framework,
> and the arm SDKs.
> 
> Then the packaging script was run like this:
> 
>     python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.15063.0
> 
> Bug:  773476 , 759402 
> Change-Id: Ie2176b5ff765d9e5497f51a7b00c02fad04fb973
> Reviewed-on: https://chromium-review.googlesource.com/727523
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510207}

TBR=thakis@chromium.org,brucedawson@chromium.org,scottmg@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  773476 ,  759402 
Change-Id: Ica63e9740c954499b85ee891fb3bec0d48dd70fb
Reviewed-on: https://chromium-review.googlesource.com/753422
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513965}
[modify] https://crrev.com/db45606398cf4389bf332b0cdcffd04e7de4a4f6/build/vs_toolchain.py

Cc: -jsc...@chromium.org
Project Member

Comment 30 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecef9073a76e802f1989dfc87749f4da0f2722ed

commit ecef9073a76e802f1989dfc87749f4da0f2722ed
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Dec 15 22:06:00 2017

Repackaged VS 2017 with updated Debuggers

Due to bugs in the Fall Creators Update SDK we can't build Chromium with
clang with that SDK. However the debuggers package (windbg and friends)
in the "spring" Creators Update SDK can't handle fastlink PDBs reliably.

So, this repackages the previous toolchain with the debuggers package
from the Fall Creators Update SDK.

This makes no difference to the builds, although it does mean that an
updated copy of dbghelp.dll will be copied to the build directories.

I created this package by manually copying over the new debuggers
directory and then running:

    python package_from_installed.py --repackage=...

Bug:  773476 
Change-Id: I05b37d518e3700e125eee5bbcb2cbf885d01afc5
Reviewed-on: https://chromium-review.googlesource.com/827629
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524481}
[modify] https://crrev.com/ecef9073a76e802f1989dfc87749f4da0f2722ed/build/vs_toolchain.py

Status: WontFix (was: Started)
Microsoft has no plans to update the FCU SDK with a fix for the MIDL_CONST_ID bug:

https://developercommunity.visualstudio.com/content/problem/141665/midl-const-id-is-defined-incorrectly-leading-to-li.html

Therefore the most we can hope for is that the SDK that comes with the next major revision of Windows will work better. I am waiting to hear when a preview version with the MIDL_CONST_ID fix will ship and then I will test it in hopes of getting ahead of these errors.

But, I'm closing this bug as Won't Fix.

Project Member

Comment 32 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21a82c61057f6ef171caa85a6df424f1e5eb6a39

commit 21a82c61057f6ef171caa85a6df424f1e5eb6a39
Author: Johann <johannkoenig@google.com>
Date: Wed May 09 14:38:58 2018

visual studio setup: require older SDK

Support for the latest SDK is incomplete. Mention the required version
when prompting the developer.

R=brucedawson@chromium.org

Bug:  773476 
Change-Id: I01010f0db251e11617fb879845940fa2aee19f7b
Reviewed-on: https://chromium-review.googlesource.com/1050691
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557178}
[modify] https://crrev.com/21a82c61057f6ef171caa85a6df424f1e5eb6a39/build/vs_toolchain.py

Status: Assigned (was: WontFix)
Summary: Chrome builds should support an SDK later than Spring Creators Update SDK (was: Chrome builds should support Fall Creators Update SDK)
The Fall Creators Update SDK never worked with Chrome with clang-cl, and never will, so I'm repurposing this bug to track trying to get the April 2018 working. It ships with VS 2017 15.7. A few breaks have been hit but I don't know whether they are trivial fixes or impossible.
Blockedon: 842111
This update will by default include a new linker. We'll have to check to make sure this doesn't break incremental linking ( crbug.com/780161 ). It shouldn't but we should be sure.

The switch to lld will make this irrelevant, but we're not quite there yet.
Actually, we're at a point where we can at least try the lld switch again and see what breaks this time. I made a reland cl here: https://chromium-review.googlesource.com/c/chromium/src/+/1054521
I've confirmed that the new VS 15.7 linker handles incremental linking in Chrome correctly. But, it looks like lld has landed so this no longer matters.
Note that message_compiler.py contains this checking:

    if version != '10.0.15063':
      return

Once the switch has landed we'll need to check and see whether the format has changed (almost certainly) and then update the expectations.

Project Member

Comment 39 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda

commit 5a7f3c442684be5eeb244b904bbfc8e6edaf6fda
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Wed May 16 10:55:30 2018

Switch to VS 2017 15.7.1 with 10.0.17134.0 SDK

This change switches the VS 2017 package to use VS 2017 Update 7.1 while
using the 10.0.17134.12 SDK. The new SDK is needed to support new HDR
features, and to stop forcing external developers to install an old
([Spring] Creators Update) SDK. This change will also bring in a new
linker and other build tools, but the version of clang-cl will be
unchanged.

Packaging was done on a Windows Server 2016 VM, cleanly created for this
purpose.

Compiler was packaged up by downloading VS 2017 Update 7.1, from
https://www.visualstudio.com/vs/, and then passing these parameters to
the installer:

    --add Microsoft.VisualStudio.Workload.NativeDesktop
    --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
    --passive

Then Add or Remove Programs was used to modify the 10.0.17134.0 SDK to add
the Debuggers package.

Then the packaging script was run like this:

  python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.17134.0

Bug:  773476 
Change-Id: Ic819f3ae79d7e869227bf33fbb8d202e2f57039b
Reviewed-on: https://chromium-review.googlesource.com/1054027
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559033}
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/base/win/windows_version.cc
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/build/toolchain/win/setup_toolchain.py
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/build/vs_toolchain.py
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/docs/windows_build_instructions.md
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/tools/gn/visual_studio_writer.cc
[modify] https://crrev.com/5a7f3c442684be5eeb244b904bbfc8e6edaf6fda/tools/gn/visual_studio_writer_unittest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ad7e759dd35a114375a671f02669c9a0283bea1

commit 7ad7e759dd35a114375a671f02669c9a0283bea1
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu May 17 09:35:36 2018

Revert "Switch to VS 2017 15.7.1 with 10.0.17134.0 SDK"

This reverts commit 5a7f3c442684be5eeb244b904bbfc8e6edaf6fda.

Reason for revert: this breaks v8's VC++ goma builds, reverting until
goma is updated to support this compiler

Original change's description:
> Switch to VS 2017 15.7.1 with 10.0.17134.0 SDK
> 
> This change switches the VS 2017 package to use VS 2017 Update 7.1 while
> using the 10.0.17134.12 SDK. The new SDK is needed to support new HDR
> features, and to stop forcing external developers to install an old
> ([Spring] Creators Update) SDK. This change will also bring in a new
> linker and other build tools, but the version of clang-cl will be
> unchanged.
> 
> Packaging was done on a Windows Server 2016 VM, cleanly created for this
> purpose.
> 
> Compiler was packaged up by downloading VS 2017 Update 7.1, from
> https://www.visualstudio.com/vs/, and then passing these parameters to
> the installer:
> 
>     --add Microsoft.VisualStudio.Workload.NativeDesktop
>     --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
>     --passive
> 
> Then Add or Remove Programs was used to modify the 10.0.17134.0 SDK to add
> the Debuggers package.
> 
> Then the packaging script was run like this:
> 
>   python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.17134.0
> 
> Bug:  773476 
> Change-Id: Ic819f3ae79d7e869227bf33fbb8d202e2f57039b
> Reviewed-on: https://chromium-review.googlesource.com/1054027
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#559033}

TBR=thakis@chromium.org,dpranke@chromium.org,hubbe@chromium.org,brucedawson@chromium.org

Change-Id: Ib8e84084a9717dafa6616f132b4824d93cbf39bf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  773476 
Reviewed-on: https://chromium-review.googlesource.com/1063810
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559471}
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/base/win/windows_version.cc
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/build/toolchain/win/setup_toolchain.py
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/build/vs_toolchain.py
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/docs/windows_build_instructions.md
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/tools/gn/visual_studio_writer.cc
[modify] https://crrev.com/7ad7e759dd35a114375a671f02669c9a0283bea1/tools/gn/visual_studio_writer_unittest.cc

Project Member

Comment 41 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82a5f004e0e52897a6e8bac10490a14fc2845625

commit 82a5f004e0e52897a6e8bac10490a14fc2845625
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri May 18 20:05:49 2018

Reland "Switch to VS 2017 15.7.1 with 10.0.17134.0 SDK"

This is a reland of 5a7f3c442684be5eeb244b904bbfc8e6edaf6fda

goma now supports this compiler and the new warnings were dealt with.

Original change's description:
> Switch to VS 2017 15.7.1 with 10.0.17134.0 SDK
>
> This change switches the VS 2017 package to use VS 2017 Update 7.1 while
> using the 10.0.17134.12 SDK. The new SDK is needed to support new HDR
> features, and to stop forcing external developers to install an old
> ([Spring] Creators Update) SDK. This change will also bring in a new
> linker and other build tools, but the version of clang-cl will be
> unchanged.
>
> Packaging was done on a Windows Server 2016 VM, cleanly created for this
> purpose.
>
> Compiler was packaged up by downloading VS 2017 Update 7.1, from
> https://www.visualstudio.com/vs/, and then passing these parameters to
> the installer:
>
>     --add Microsoft.VisualStudio.Workload.NativeDesktop
>     --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
>     --passive
>
> Then Add or Remove Programs was used to modify the 10.0.17134.0 SDK to add
> the Debuggers package.
>
> Then the packaging script was run like this:
>
>   python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.17134.0
>
> Bug:  773476 
> Change-Id: Ic819f3ae79d7e869227bf33fbb8d202e2f57039b
> Reviewed-on: https://chromium-review.googlesource.com/1054027
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#559033}

Bug:  773476 ,834213
Change-Id: I903158f9dfa604f250010a7047496509f51782e7
Reviewed-on: https://chromium-review.googlesource.com/1066130
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560002}
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/base/win/windows_version.cc
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/build/toolchain/win/setup_toolchain.py
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/build/vs_toolchain.py
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/docs/windows_build_instructions.md
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/tools/gn/visual_studio_writer.cc
[modify] https://crrev.com/82a5f004e0e52897a6e8bac10490a14fc2845625/tools/gn/visual_studio_writer_unittest.cc

Comment 42 by kbr@chromium.org, May 25 2018

Blockedon: 846313
Project Member

Comment 43 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eac0ee57871b39ff11daaa73730e29b58e171898

commit eac0ee57871b39ff11daaa73730e29b58e171898
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Wed May 30 12:35:14 2018

Undefine DEPRECATEDENUMERATOR macro for clang

The issue was fixed by VS 2017 15.7.1 with 10.0.17134.0 SDK
https://developercommunity.visualstudio.com/content/problem/131391/154-fails-to-define-deprecatedenumerator-2.html

Bug:  773476 
Change-Id: Ib90e99e9b3d69907aaaa994df92f7debd65105b3
Reviewed-on: https://chromium-review.googlesource.com/1078213
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562802}
[modify] https://crrev.com/eac0ee57871b39ff11daaa73730e29b58e171898/build/config/win/BUILD.gn

Project Member

Comment 44 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac88874511d02aaf42ebbefaf078d8c8f8c2af8a

commit ac88874511d02aaf42ebbefaf078d8c8f8c2af8a
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Wed Jun 06 23:52:33 2018

Update toolchain package to fix dbghelp.dll bug

This change updates the VS 2017 package to fix a bad version of
dbghelp.dll in the Debuggers package, caused by a now-obsolete
workaround in the packaging script, removed in crrev.com/c/1086990.

The package still uses VS 2017 Update 7.1 and the 10.0.17134.12 SDK.

Packaging was done on a Windows Server 2016 VM, cleanly created for this
purpose.

Compiler was packaged up by downloading VS 2017 Update 7.1, from
https://www.visualstudio.com/vs/, and then passing these parameters to
the installer:

    --add Microsoft.VisualStudio.Workload.NativeDesktop
    --add Microsoft.VisualStudio.Component.VC.ATLMFC --includeRecommended
    --passive

Then Add or Remove Programs was used to modify the 10.0.17134.0 SDK to add
the Debuggers package.

Then the packaging script was run like this:

  python depot_tools\win_toolchain\package_from_installed.py 2017 -w 10.0.17134.0

The results were compared to make sure that there were no unintended changes. One
quirk is that the new package is missing the arm/arm64 directories in Debuggers,
which is correct, whereas the previous package was *not* missing them, for some
reason. Other than that there are no unintended changes.

Bug:  773476 ,  846313 
Change-Id: I43db2ea95999fb7b2aeb02ba078e70298b62ffad
Reviewed-on: https://chromium-review.googlesource.com/1086992
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565106}
[modify] https://crrev.com/ac88874511d02aaf42ebbefaf078d8c8f8c2af8a/build/vs_toolchain.py

Status: Fixed (was: Assigned)
This is fully fixed now, after resolving the cdb.exe glitch.

I also confirmed with an external developer that they were able to package VS/SDK using my instructions and get the same results. Note that they had to use the offline installer as the online installer gave them the latest version.

Sign in to add a comment