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

Issue 591920 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 0
Type: Bug-Regression



Sign in to add a comment

Compile Failure on instrumentation phase for Win - PGO builds 51.0.2667.2 (32 and 64 bit)

Project Member Reported by ranjitkan@chromium.org, Mar 4 2016

Issue description

Builder URL's:
==========
https://uberchromegw.corp.google.com/i/official.desktop/builders/win-pgo/builds/178  - (Win-PGO 32bit)
https://uberchromegw.corp.google.com/i/official.desktop/builders/win64-pgo/builds/32  - Win-PGO 64bit)

Log Url's:
=======
https://uberchromegw.corp.google.com/i/official.desktop/builders/win-pgo/builds/178/steps/Compile%3A%20Instrumentation%20phase./logs/stdio
https://uberchromegw.corp.google.com/i/official.desktop/builders/win64-pgo/builds/32/steps/Compile%3A%20Instrumentation%20phase./logs/stdio

Error from the Log:
==============
FAILED: ninja -t msvc -e environment.x64 -- "C:\b\depot_tools\win_toolchain\vs_files\b349b3cc596d5f7e13d649532ddd7e8db39db0cb\VC\bin\amd64\cl.exe" /nologo /showIncludes /FC @obj\third_party\libxml\src\libxml.buf.obj.rsp /c ..\..\third_party\libxml\src\buf.c /Foobj\third_party\libxml\src\libxml.buf.obj /Fdobj\third_party\libxml\libxml.c.pdb 
c:\b\depot_tools\win_toolchain\vs_files\b349b3cc596d5f7e13d649532ddd7e8db39db0cb\win_sdk\include\10.0.10586.0\ucrt\stdio.h(1925): warning C4005: 'snprintf': macro redefinition
c:\b\build\slave\win64-pgo\build\src\third_party\libxml\win32\config.h(99): note: see previous definition of 'snprintf'
c:\b\depot_tools\win_toolchain\vs_files\b349b3cc596d5f7e13d649532ddd7e8db39db0cb\win_sdk\include\10.0.10586.0\ucrt\stdio.h(1927): fatal error C1189: #error:  Macro definition of snprintf conflicts with Standard Library function declaration

Unable to find any possible suspect from the log, cc'ing chromium sheriffs and adding Infra troopers into the look. Request to please take a look.
 
Labels: Sheriff-Chromium
Labels: -Infra-Troopers
This looks possibly related to compiler versions. The error is referenced in this CL:
https://codereview.chromium.org/1072653003/

Removing from trooper queue since this doesn't look like a trooper issue.
Cc: brucedaw...@chromium.org
This is caused by the libxml roll: http://crrev.com/1752223002
Cc: gov...@chromium.org ligim...@chromium.org
The compile failures are seen continuously on latest Win 32 and Win 64 official desktop PGO biulds 51.0.2668.2 and 51.0.2669.2.
https://chromegw.corp.google.com/i/official.desktop/builders/win-pgo/builds/180
https://chromegw.corp.google.com/i/official.desktop/builders/win64-pgo/builds/34

Could someone look into this.
Cc: dimu@chromium.org
Owner: brucedaw...@chromium.org
Status: Started (was: Untriaged)
I'll take it.
Patch is pending review.
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 8 2016

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

commit f7bd300e8c52d6ebb7575a28d4cd9d3bdccaf439
Author: brucedawson <brucedawson@chromium.org>
Date: Tue Mar 08 05:13:41 2016

Avoid illegal definition of snprintf in VS 2015

VS 2015 supplies an implementation of snprintf which makes definining
it to _snprintf illegal - previously it was just a really bad idea.
This reinstates the compiler version check so that libxml will compile
in Chromium.

README.chromium was also updated to make it explicit that these checks
need to be added (for now) when config.h is copied.

A corresponding change will need to be made upstream.

BUG= 440500 , 591920 

Review URL: https://codereview.chromium.org/1770093002

Cr-Commit-Position: refs/heads/master@{#379755}

[modify] https://crrev.com/f7bd300e8c52d6ebb7575a28d4cd9d3bdccaf439/third_party/libxml/README.chromium
[modify] https://crrev.com/f7bd300e8c52d6ebb7575a28d4cd9d3bdccaf439/third_party/libxml/win32/config.h

Cc: -finnur@chromium.org
Had a chat with Bruce and he is working on it.
See https://bugs.chromium.org/p/chromium/issues/detail?id=440500#c305 (and comment 304) for the status of these failures.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 10 2016

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

commit 58552f9fcee335a8704f0c19472fde26ade8a7b9
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Mar 10 02:28:47 2016

Suppress new C4334 and C4595 warnings

These two warnings are new in VS 2015 Update 2 (C4334 isn't necessarily
brand new, but it fires much more frequently now). Therefore they are
preventing a switch to VS 2015, without actually indicating any new
issues. The possible issues pointed to by C4334, in particular, have
been benign in every case that I have investigated. Therefore the
pragmatic thing to do is to suppress them for now, and resolve the
warnings individually after VS 2015 is the default.

The 4595 warning suppression is only needed in third_party\icu but it is
placed globally temporarily.

These warnings are also blocking PGO builds - another reason why a
timely fix is important.

BUG= 440500 ,593448, 591920 

Review URL: https://codereview.chromium.org/1781593004

Cr-Commit-Position: refs/heads/master@{#380312}

[modify] https://crrev.com/58552f9fcee335a8704f0c19472fde26ade8a7b9/build/common.gypi
[modify] https://crrev.com/58552f9fcee335a8704f0c19472fde26ade8a7b9/build/config/compiler/BUILD.gn

Labels: -Sheriff-Chromium
Cc: mmohammad@chromium.org
Still seeing the Win64-PGO failing in the latest build.

Link to Build:
--------------
https://chromegw.corp.google.com/i/official.desktop/builders/win64-pgo/builds/39
The builder is now hitting a new break, covered by  crbug.com/593996 . Fix is incoming.
Labels: -ReleaseBlock-Dev
We don't plan to push pgo for dev this week, hence removing the label.
Status: Fixed (was: Started)
All VS 2015 compile errors have been fixed and the last three PGO builds went through cleanly.

Now that VS 2015 is the default compiler (71 hours and holding!) the PGO builds should be much less likely to break.
Status: Verified (was: Fixed)
Builds are green so marking as verified.Thank you !

Builds path : 
https://uberchromegw.corp.google.com/i/official.desktop/builders/win-pgo
https://uberchromegw.corp.google.com/i/official.desktop/builders/win64-pgo
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 28 2016

Labels: merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40ff9d7fda350ccb83c171c68b60290d6ff3b74d

commit 40ff9d7fda350ccb83c171c68b60290d6ff3b74d
Author: Dominic Cooney <dominicc@chromium.org>
Date: Mon Mar 28 12:46:39 2016

Avoid illegal definition of snprintf in VS 2015

VS 2015 supplies an implementation of snprintf which makes definining
it to _snprintf illegal - previously it was just a really bad idea.
This reinstates the compiler version check so that libxml will compile
in Chromium.

README.chromium was also updated to make it explicit that these checks
need to be added (for now) when config.h is copied.

A corresponding change will need to be made upstream.

BUG= 440500 , 591920 

Review URL: https://codereview.chromium.org/1770093002

Cr-Commit-Position: refs/heads/master@{#379755}
(cherry picked from commit f7bd300e8c52d6ebb7575a28d4cd9d3bdccaf439)

Review URL: https://codereview.chromium.org/1836853002 .

Cr-Commit-Position: refs/branch-heads/2661@{#401}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/40ff9d7fda350ccb83c171c68b60290d6ff3b74d/third_party/libxml/README.chromium
[modify] https://crrev.com/40ff9d7fda350ccb83c171c68b60290d6ff3b74d/third_party/libxml/win32/config.h

FYI: I merged https://bugs.chromium.org/p/chromium/issues/detail?id=587897#c14 to M50/2661. When that patch landed on trunk it broke VS2015 builders. I'm not 100% clear on what official builds are built with, so I have also merged the patch in comment 8 on this bug. That's what comment 20 is about.

Sign in to add a comment