Is there anything unusual about your build environment? I have not seen this behavior.

In particular, make sure you have the latest version of the Windows 10 15063 SDK, 10.0.15063.468 IIRC. Having an older version of the SDK is the most likely explanation for why rc.exe is giving unexpected results.

Another known problem is that when running this command:

"C:\Program Files (x86)\Microsoft Visual Studio\Preview\Professional\VC\Auxiliary\Build\vcvarsall.bat" amd64

an incorrect version of rc.exe may appear first in the path. On my machine I see the 10.0.10011.16384 version appear first which is a confusing version number. This happens despite my having the 10.0.15063.0 SDK version installed - this version shows up later in the path. This was reported here:

https://developercommunity.visualstudio.com/content/problem/121726/vcvarsallbat-adds-old-sdk-to-path.html

I'm building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 and a local installation of Visual Studio 2017 15.4.0 and 10.0.15063.468 SDK. The PATH from my environment.x86 file is:

PATH=C:\Windows\Sysnative;C:\Windows\SysWOW64;c:\code\depot_tools\win_tools-2_7_6_bin\python\bin;c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.11.25503\bin\hostx64\x86;c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.11.25503\bin\hostx64\x64;c:\program files (x86)\microsoft visual studio\2017\professional\common7\ide\vc\vcpackages;c:\program files (x86)\microsoft sdks\typescript\2.3;c:\program files (x86)\microsoft sdks\typescript\2.3;c:\program files (x86)\microsoft visual studio\2017\professional\common7\ide\commonextensions\microsoft\testwindow;c:\program files (x86)\microsoft visual studio\2017\professional\common7\ide\commonextensions\microsoft\teamfoundation\team explorer;c:\program files (x86)\microsoft visual studio\2017\professional\msbuild\15.0\bin\roslyn;c:\program files (x86)\microsoft visual studio\2017\professional\team tools\performance tools;c:\program files (x86)\microsoft visual studio\shared\common\vsperfcollectiontools\;c:\program files (x86)\microsoft sdks\windows\v10.0a\bin\netfx 4.6.1 tools\x64\;c:\program files (x86)\windows kits\10\bin\x64;c:\program files (x86)\windows kits\10\bin\10.0.15063.0\x64;c:\program files (x86)\microsoft visual studio\2017\professional\\msbuild\15.0\bin;c:\windows\microsoft.net\framework64\v4.0.30319;c:\program files (x86)\microsoft visual studio\2017\professional\common7\ide\;c:\program files (x86)\microsoft visual studio\2017\professional\common7\tools\;c:\code\depot_tools\win_tools-2_7_6_bin\python\bin;c:\code\depot_tools\win_tools-2_7_6_bin\python\bin\scripts;c:\windows\system32;c:\windows;c:\windows\system32\wbem;c:\windows\system32\windowspowershell\v1.0\;c:\program files\microsoft sql server\110\tools\binn\;c:\program files (x86)\microsoft sdks\typescript\1.0\;c:\code\depot_tools;c:\program files (x86)\windows kits\10\windows performance toolkit\;;c:\code\depot_tools\

Setting that PATH in cmd.exe I get the following:

>which rc.exe
/c/program files (x86)/windows kits/10/bin/x64/rc.exe

>rc.exe /?

Microsoft (R) Windows (R) Resource Compiler Version 10.0.10011.16384

Also:

>where rc.exe
c:\program files (x86)\windows kits\10\bin\x64\rc.exe
c:\program files (x86)\windows kits\10\bin\10.0.15063.0\x64\rc.exe

So, it looks like the same issue you reported to Microsoft. Do you have any suggestions on a workaround?

For message_compiler.py we have version checking for mc.exe and we only compare the results if the versions are what we expect. See crrev.com/c/807205 for an example.

Can you attach cefsimple.rc?

oh nvm it's at https://bitbucket.org/chromiumembedded/cef/src/4c795f51884cc1255f009a601a40237532b72414/tests/cefsimple/cefsimple.rc?at=master&fileviewer=file-view-default in comment 0

Smaller repro:

100 ICON "cefsimple.ico"


With cefsimple.ico from here: https://bytebucket.org/chromiumembedded/cef/raw/4c795f51884cc1255f009a601a40237532b72414/tests/cefsimple/res/cefsimple.ico

What's different is the "bpp" header in the RT_GROUP_ICON. rc.cc writes the value from the .ico file, which is 0. Looks like it's supposed to sanitize the 0 in the input file to 4.

Which program did you use to create that .ico file?

This seems to do the trick:

diff --git a/res/rc.cc b/res/rc.cc
index e92e4e5..b9f7ad7 100644
--- a/res/rc.cc
+++ b/res/rc.cc
@@ -2986,14 +2986,37 @@ bool SerializationVisitor::WriteIconOrCursorGroup(const FileResource* r,
   write_little_short(out_, dir.type);
   write_little_short(out_, dir.count);
   for (IconEntry& entry : entries) {
+    // For cursors, entry.num_planes and entry.bpp store the hotspot
+    // coordinates.  For icons, these two fields do exist, but it seems rc.exe
+    // ignores their contents and grabs the values from the BITMAPINFOHEADER
+    // (if present) there too.  So always do this.
+    uint16_t num_planes;
+    uint16_t bpp;
+    fseek(f, entry.data_offset, SEEK_SET);
+    uint32_t bitmapinfoheader_size = read_little_long(f);
+    if (bitmapinfoheader_size == 0x28) {
+      // foo_new.ico just contains raw png data at the data offset, without
+      // BITMAPINFOHEADER.  The 0x28 check prevents reading that.
+      (void)read_little_long(f);  // width
+      (void)read_little_long(f);  // height
+      num_planes = read_little_short(f);
+      bpp = read_little_short(f);
+    } else {
+      // A PNG .ico file.
+      // https://blogs.msdn.microsoft.com/oldnewthing/20101022-00/?p=12473
+      // "The image must be in 32bpp"
+      num_planes = 1;
+      bpp = 32;
+    }
+
     if (type == kIcon) {
       fputc(entry.width, out_);
       fputc(entry.height, out_);
       fputc(entry.num_colors, out_);
       fputc(entry.reserved, out_);
       // Some files have num_planes set to 0, but rc.exe still writes 1 here.
-      write_little_short(out_, 1);
-      write_little_short(out_, entry.bpp);
+      write_little_short(out_, num_planes);
+      write_little_short(out_, bpp);
     } else {
       // .cur files are weird, they store width and height as shorts instead
       // of bytes here, multiply height by 2 apparently, and then store
@@ -3004,8 +3027,6 @@ bool SerializationVisitor::WriteIconOrCursorGroup(const FileResource* r,
       write_little_short(out_, 2*entry.height);
       // Grab actual non-hotspot num_planes, bpp from BITMAPINFOHEADER.
       fseek(f, entry.data_offset + 12, SEEK_SET);
-      uint16_t num_planes = read_little_short(f);
-      uint16_t bpp = read_little_short(f);
       write_little_short(out_, num_planes);
       write_little_short(out_, bpp);
     }


What's the license on cefsimple.ico? Can you put it in the public domain so I can add it to my test suite?

https://github.com/nico/hack/commit/8269c34211b9653305619606caacdb89cabb6d26 should do it; I changed my existing .ico file in a hex editor to test.

Make that https://github.com/nico/hack/commit/ae6757d3b49149df890d03319d30c67d0480b24e

Thanks for the quick fix :)

> What's the license on cefsimple.ico? Can you put it in the public domain so I can add it to my test suite?

It came from the webkit test_shell application originally, so it's already in the public domain:
https://chromium.googlesource.com/chromium/src/+/10.0.601.0/webkit/tools/test_shell/resources/test_shell.ico

Unfortunately I don't know what program was used to create it.

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

commit ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7
Author: Nico Weber <thakis@chromium.org>
Date: Thu Dec 07 18:37:10 2017

Update prebuilt rc binary.

CL created by running build/toolchain/win/rc/upload_rc_binaries.sh

Bug:  792594 , 792576 
Change-Id: I86fbfa2bbafed0670c0a7e2f43518b7a86d2c6b1
Reviewed-on: https://chromium-review.googlesource.com/813416
Reviewed-by: Marshall Greenblatt <marshall@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522484}
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/linux64/rc.sha1
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/mac/rc.sha1
[modify] https://crrev.com/ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7/build/toolchain/win/rc/win/rc.exe.sha1

Can we also merge this fix to M64? Thanks.

Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Cherry-pick PR here: https://chromium-review.googlesource.com/c/chromium/src/+/818094

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

commit 40814bb06aba3d09b6a7556ce475e62693b905a4
Author: Nico Weber <thakis@chromium.org>
Date: Fri Dec 08 20:02:43 2017

Update prebuilt rc binary.

CL created by running build/toolchain/win/rc/upload_rc_binaries.sh

Bug:  792594 , 792576 
Change-Id: I86fbfa2bbafed0670c0a7e2f43518b7a86d2c6b1
Reviewed-on: https://chromium-review.googlesource.com/813416
Reviewed-by: Marshall Greenblatt <marshall@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522484}(cherry picked from commit ae2a67ea2b14bfe6d9ee07986e950d3d36839ac7)
Reviewed-on: https://chromium-review.googlesource.com/818094
Cr-Commit-Position: refs/branch-heads/3282@{#102}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/linux64/rc.sha1
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/mac/rc.sha1
[modify] https://crrev.com/40814bb06aba3d09b6a7556ce475e62693b905a4/build/toolchain/win/rc/win/rc.exe.sha1