[PATCH] D76951: [llvm-rc] Allow -1 for menu item IDs

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: thakis, amccarth, rnk.
Herald added a project: LLVM.

This seems to be used in some resource files, e.g.
https://github.com/wxWidgets/wxWidgets/blob/f3217573d7240411e7817c9d76d965b2452987a2/include/wx/msw/wx.rc#L28.

MSVC rc.exe and GNU windres both allow any value here, and silently
just truncate to uint16_t range. This just explicitly allows the
-1 value and errors out on others - the same was done for control
IDs in dialogs in c1a67857ba0a6ba558818b589fe7c0fcc8f238ae.

I presume there's other cases where seemingly out of range values are used, where rc.exe and GNU windres just allow them through, while llvm-rc strictly checks the range of all values - but these two are the ones I've run into so far...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76951

Files:
  llvm/test/tools/llvm-rc/Inputs/tag-menu.rc
  llvm/test/tools/llvm-rc/tag-menu.test
  llvm/tools/llvm-rc/ResourceFileWriter.cpp


Index: llvm/tools/llvm-rc/ResourceFileWriter.cpp
===================================================================
--- llvm/tools/llvm-rc/ResourceFileWriter.cpp
+++ llvm/tools/llvm-rc/ResourceFileWriter.cpp
@@ -1180,8 +1180,10 @@
 
   if (auto *MenuItemPtr = dyn_cast<MenuItem>(DefPtr)) {
     writeInt<uint16_t>(Flags);
-    RETURN_IF_ERROR(
-        checkNumberFits<uint16_t>(MenuItemPtr->Id, "MENUITEM action ID"));
+    // Some resource files use -1, i.e. UINT32_MAX, for empty menu items.
+    if (MenuItemPtr->Id != static_cast<uint32_t>(-1))
+      RETURN_IF_ERROR(
+          checkNumberFits<uint16_t>(MenuItemPtr->Id, "MENUITEM action ID"));
     writeInt<uint16_t>(MenuItemPtr->Id);
     RETURN_IF_ERROR(writeCString(MenuItemPtr->Name));
     return Error::success();
Index: llvm/test/tools/llvm-rc/tag-menu.test
===================================================================
--- llvm/test/tools/llvm-rc/tag-menu.test
+++ llvm/test/tools/llvm-rc/tag-menu.test
@@ -69,7 +69,7 @@
 ; MENU-NEXT:   0060: 61002600 62006300 64006500 00000000  |a.&.b.c.d.e.....|
 ; MENU-NEXT:   0070: 03006100 62002600 63006400 65000000  |..a.b.&.c.d.e...|
 ; MENU-NEXT:   0080: 00000400 61006200 63002600 64006500  |....a.b.c.&.d.e.|
-; MENU-NEXT:   0090: 00008000 05006100 62006300 64002600  |......a.b.c.d.&.|
+; MENU-NEXT:   0090: 00008000 FFFF6100 62006300 64002600  |......a.b.c.d.&.|
 ; MENU-NEXT:   00A0: 65000000                             |e...|
 ; MENU-NEXT: )
 
Index: llvm/test/tools/llvm-rc/Inputs/tag-menu.rc
===================================================================
--- llvm/test/tools/llvm-rc/Inputs/tag-menu.rc
+++ llvm/test/tools/llvm-rc/Inputs/tag-menu.rc
@@ -54,7 +54,7 @@
     MENUITEM "a&bcde", 2
     MENUITEM "ab&cde", 3
     MENUITEM "abc&de", 4
-    MENUITEM "abcd&e", 5
+    MENUITEM "abcd&e", -1
   }
 END
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76951.253204.patch
Type: text/x-patch
Size: 1842 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200327/de1cc311/attachment.bin>