New issue
Advanced search Search tips

Issue 678013 link

Starred by 1 user

Issue metadata

Status: Archived
Owner: ----
Closed: Jan 10
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Binary not relinking on Windows after warning suppression changes

Project Member Reported by scottmg@chromium.org, Jan 3 2017

Issue description

Copying from wez@'s post elsewhere for future reference.

MSVC's incremental linker won't re-link a binary if you change the warning suppressions you've supplied, even if you've specified that warnings should be treated as errors.

Linker warnings. We checked the ninja .rsp files and confirmed that the /ignore flags were listed there as expected.

If you:
1. Set up a component build.
2. Build the media_mojo_unittests target.
3. Go to media/mojo/BUILD.gn and comment out the suppressions in the media_mojo_unittests target.
4. Build the media_mojo_unittests target.

At step #4 you'll see GN regenerate ninja files, and ninja will re-run the link step, but the link step will trivially succeed, rather than failing due to the un-suppressed warnings being treated as errors.

 
I couldn't reproduce, but only because I don't actually get any warnings from the linker in building that target.

(I don't think we have a /WX in ldflags anywhere, I assumed you added that too?)
Scratch that, I can reproduce.

- Clean
- Add /WX to media_mojo_unittests ldflags, and remove /ignore:...
- Build media_mojo_unittests (fails)
- Add the /ignores
- Build media_mojo_unittests (succeeds)
- Remove the ignores again
- Build media_mojo_unittests (succeeds trivially, but shouldn't)

Bummer. Seems clearly like a toolchain bug. I'll see if I can make a small repro and file one against Microsoft.

Comment 3 by w...@chromium.org, Jan 3 2017

Re /WX: Yes, I landed a change recently to add /WX to the link flags for
most targets (see build/config/compiler/BUILD.gn in an up-to-date checkout).
Status: ExternalDependency (was: Unconfirmed)
Filed https://connect.microsoft.com/VisualStudio/feedback/details/3117886 .
Microsoft reports: "We have fixed this issue and will include the fix in VS 2017 Update 1."
Status: Archived (was: ExternalDependency)
Archiving P3s older than 1 year with no owner or component.

Sign in to add a comment