New issue
Advanced search Search tips

Issue 637456 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 640254

Blocking:
issue 82385
issue 627216



Sign in to add a comment

clang-cl fails on code from VS 2015 Update 3's midl compiler

Project Member Reported by brucedaw...@chromium.org, Aug 12 2016

Issue description

While testing an upgrade to VS 2015 Update 3 (crrev.com/2106203002) the try bots failed on win_clang with these errors:

FAILED: obj/google_update/google_update/google_update_idl_i.obj 
E:\b\c\cipd\goma/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/google_update/google_update/google_update_idl_i.obj.rsp /c gen/google_update/google_update_idl_i.c /Foobj/google_update/google_update/google_update_idl_i.obj /Fd"obj/google_update/google_update_c.pdb"
gen/google_update/google_update_idl_i.c(70,23):  error: 'extern' variable has an initializer [-Werror,-Wextern-initializer]
MIDL_DEFINE_GUID(IID, IID_IGoogleUpdate3,0x6DB17455,0x4E85,0x46e7,0x9D,0x23,0xE5,0x55,0xE4,0xB0,0x05,0xAF);

The repro steps are to use these args.gn settings:

  is_component_build = false
  is_debug = false
  target_cpu = "x86"
  is_clang = true

and build with:

  ninja -C out\release_clang obj/google_update/google_update/google_update_idl_i.obj

The old google_update_idl_i.c file contains this:

#define MIDL_DEFINE_GUID(type,name,l,w1,w2,b1,b2,b3,b4,b5,b6,b7,b8) \
        const type name = {l,w1,w2,{b1,b2,b3,b4,b5,b6,b7,b8}}

while the new one contanis this:

#define MIDL_DEFINE_GUID(type,name,l,w1,w2,b1,b2,b3,b4,b5,b6,b7,b8) \
        EXTERN_C __declspec(selectany) const type name = {l,w1,w2,{b1,b2,b3,b4,b5,b6,b7,b8}}

I'm not sure what the best fix is. It might be possible to package the old midl compiler but I don't know for sure that that would work.

 

Comment 1 by r...@chromium.org, Aug 18 2016

Probably not hard to fix. I bet MSVC treats 'extern __declspec(selectany)' on data the same way they treat 'extern inline' on code.

Comment 2 by r...@chromium.org, Aug 18 2016

Cc: h...@chromium.org
Owner: r...@chromium.org
Fixed in r279116. Pasting the commit log.

-Wextern-initializer is a really weird warning, I had to do a lot of rationalization to explain what's going on here, and I'm not sure I got it 100% right.

commit eae018495f7508e12e3c90c37bcce866bce92f2a
Author: rnk <rnk@91177308-0d34-0410-b5e6-96231b3b80d8>
Date:   Thu Aug 18 18:45:07 2016 +0000

    [MS] Silence -Wextern-init on const selectany variables

    In C, 'extern' is typically used to avoid tentative definitions when
    declaring variables in headers, but adding an intializer makes it a
    defintion. This is somewhat confusing, so GCC and Clang both warn on it.
    In C++, 'extern' is often used to give implictly static 'const'
    variables external linkage, so don't warn in that case. If selectany is
    present, this might be header code intended for C and C++ inclusion, so
    apply the C++ rules.

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@279116 91177308-0d34-0410-b5e6-96231b3b80d8

Is this the only thing blocking Update 3? The Clang ToT bots are pretty green right now, so we can probably kick off another roll right away.
This is the only thing that I am aware of that is blocking Update 3. The try jobs are just finishing off running tests.

Comment 4 by r...@chromium.org, Aug 18 2016

Actually, it's probably better to temporarily add -Wno-extern-init to the MIDL target. We already have flags there:
https://cs.chromium.org/chromium/src/google_update/google_update.gyp?q=google_update_idl_i&sq=package:chromium&l=11
      'target_name': 'google_update',
      'type': 'static_library',
      'variables': {
        'clang_warning_flags': [
          # MIDL generates code like "#endif !_MIDL_USE_GUIDDEF_"
          '-Wno-extra-tokens',
        ],

Just reference this bug, and we can take it out on the next clang roll.

Comment 5 by r...@chromium.org, Aug 18 2016

I'm sorry, that's the gyp. I guess for gn you'd add it here:
https://cs.chromium.org/chromium/src/build/config/win/BUILD.gn?q=midl_warnings&sq=package:chromium&l=404&dr=C
Sounds good. I'll try that.
-Wno-extern-initializer works in crrev.com/2106203002 so I am now unblocked.

Comment 8 by thakis@chromium.org, Aug 22 2016

Blocking: 82385

Comment 9 by thakis@chromium.org, Aug 23 2016

Blockedon: 640254
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 24 2016

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

commit 109db8b6fcdb52a55e40c44128a9cf36c00a5649
Author: thakis <thakis@chromium.org>
Date: Wed Aug 24 15:57:13 2016

clang/win: Make builds work with MSVC2015 update 3.

The midl compiler now produces slightly questionable code that clang-cl warns
on.  The warning is no longer emitted in upstream clang-cl, but until we roll
that in, suppress it so that people who locally use update 3 are able to build
with clang.

BUG= 637456 ,  627216 ,  640000 
NOTRY=true

Review-Url: https://codereview.chromium.org/2270693008
Cr-Commit-Position: refs/heads/master@{#414083}

[modify] https://crrev.com/109db8b6fcdb52a55e40c44128a9cf36c00a5649/build/config/win/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 31 2016

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

commit a033f395bf2547cf4764f77cc9c86d08f3e22c23
Author: thakis <thakis@chromium.org>
Date: Wed Aug 31 05:16:14 2016

Roll clang 278861:280106.

* win: Members of base classes now should show up in debugger.
* win: Debugger shouldn't show funny highlights anymore due to
  debug info no longer including column information.  (we still
  force this on if sanitizers are used, mostly for clusterfuzz.
  maybe we want to make this toggleable independent of sanitizers
  at some point)
* win: -Wextern-initializer no longer warns on midl-generated code
* win: clang-cl now accepts /source-encoding:utf-8 and friends
  (utf-8 was the source enconding in clang-cl before already, but
  now we don't warn on an explicit flag requesting this)
* all platforms: Three plugin checks are now on-by-default,
  remove flags for these (see
    https://codereview.chromium.org/2267713003
    https://codereview.chromium.org/2268203002
    https://codereview.chromium.org/2265093002
  )
* win: clang-cl's /Brepro now does what it's supposed to do
* win: clang-cl now emits absolute paths in diagnostics, by
  popular request.

Ran `tools/clang/scripts/upload_revision.py 280106`.

BUG= 640254 , 637456 , 636109 , 636091 , 636099 

Review-Url: https://codereview.chromium.org/2292173002
Cr-Commit-Position: refs/heads/master@{#415563}

[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/clang/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/compiler/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/win/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/tools/clang/scripts/update.py

Status: Fixed (was: Untriaged)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 8 2016

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

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

Review-Url: https://codereview.chromium.org/2106203002
Cr-Commit-Position: refs/heads/master@{#417157}

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 8 2016

Labels: merge-merged-2854
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02343c661ee0482cb73ad0cf861999f2c57eeb11

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

Review-Url: https://codereview.chromium.org/2106203002
Cr-Commit-Position: refs/heads/master@{#417157}

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Sign in to add a comment