(Proposal) Use alternative C4786 warning suppression (#312)

* Use alternative warning suppression

* Remove newline

* Fix script

* Patch C2.EXE to disable C4786 warning

* Delete compile.cmd

* py-fixes

* Update tools/patch_c2.py

* Update tools/patch_c2.py

---------

Co-authored-by: Anonymous Maarten <anonymous.maarten@gmail.com>
Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>

.editorconfig

@@ -2,7 +2,7 @@ root = true
 
 [*.{py,txt,editorconfig}]
 indent_style = space
-indent_size = 2
+indent_size = 4
 insert_final_newline = true
 trim_trailing_whitespace = true
 

.github/workflows/build.yml

@@ -20,6 +20,10 @@ jobs:
         # Use minimum supported version
         cmake-version: '3.13.x'
 
+    - name: Patch MSVC 4.2
+      run: |
+        python tools/patch_c2.py msvc420/bin/C2.EXE
+
     - name: Build
       shell: cmd
       run: |

.pylintrc

@@ -165,7 +165,7 @@ class-naming-style=PascalCase
 #class-rgx=
 
 # Naming style matching correct constant names.
-const-naming-style=snake_case
+const-naming-style=UPPER_CASE
 
 # Regular expression matching correct constant names. Overrides const-naming-
 # style. If left empty, constant names will be checked with the set naming
@@ -511,7 +511,7 @@ ignore-imports=yes
 ignore-signatures=yes
 
 # Minimum lines number of a similarity.
-min-similarity-lines=4
+min-similarity-lines=16
 
 
 [SPELLING]

CMakeLists.txt

@@ -289,8 +289,9 @@ if (MSVC)
   # These flags have been taken from the defaults for a Visual C++ 4.20 project (the compiler the
   # game was originally built with) and tweaked slightly to produce more debugging info for reccmp.
   # They ensure a recompilation that can be byte/instruction accurate to the original binaries.
-
+  if (ISLE_BUILD_APP)
     target_compile_options(isle PRIVATE "/ML$<$<CONFIG:Debug>:d>")
+  endif()
   target_compile_options(lego1 PRIVATE "/MT$<$<CONFIG:Debug>:d>")
 
   set(CMAKE_CXX_FLAGS "/W3 /GX /D \"WIN32\" /D \"_WINDOWS\"")

LEGO1/compat.h

@@ -18,12 +18,6 @@
 // Impossible to avoid this if using STL map or set.
 // This removes most (but not all) occurrences of the warning.
 #pragma warning(disable : 4786)
-// To really remove *all* of the warnings, we have to employ the following,
-// obscure workaround from https://www.earthli.com/news/view_article.php?id=376
-static class msVC6_4786WorkAround {
-public:
-	msVC6_4786WorkAround() {}
-} msVC6_4786WorkAround;
 
 #define MSVC420_VERSION 1020
 

tools/patch_c2.py

@@ -0,0 +1,67 @@
+#!/usr/bin/env python
+
+import argparse
+import hashlib
+import pathlib
+import shutil
+
+ORIGINAL_C2_MD5 = "dcd69f1dd28b02dd03dd7ed02984299a"  # original C2.EXE
+
+C2_MD5 = (
+    ORIGINAL_C2_MD5,
+    "e70acde41802ddec06c4263bb357ac30",  # patched C2.EXE
+)
+
+C2_SIZE = 549888
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        allow_abbrev=False,
+        description="Path to C2.EXE of Microsoft Visual Studio 4.2.0 to disable C4786 warning",
+    )
+    parser.add_argument("path", type=pathlib.Path, help="Path of C2.EXE")
+    parser.add_argument(
+        "-f", dest="force", default=False, action="store_true", help="force"
+    )
+    args = parser.parse_args()
+
+    if not args.path.is_file():
+        parser.error("Input is not a file")
+
+    binary = bytearray(args.path.open("rb").read())
+    md5 = hashlib.md5(binary).hexdigest()
+    print(md5, C2_MD5)
+
+    msg_cb = parser.error if not args.force else print
+    if len(binary) != C2_SIZE:
+        msg_cb("file size is not correct")
+    if md5 not in C2_MD5:
+        msg_cb("md5 checksum does not match")
+
+    if md5 == ORIGINAL_C2_MD5:
+        backup = f"{args.path}.BAK"
+        print(f'Creating backup "{backup}"')
+        shutil.copyfile(args.path, backup)
+
+    def nop_patch(start, count, expected=None):
+        replacement = [0x90] * count
+        if expected:
+            current = list(binary[start : start + count])
+            assert len(expected) == count
+            assert current in (expected, replacement)
+        print(f"Nopping {count} bytes at 0x{start:08x}")
+        binary[start : start + count] = replacement
+
+    print(
+        "Disable C4786 warning: '%Fs' : identifier was truncated to '%d' characters in the debug information"
+    )
+    nop_patch(0x52F07, 5, [0xE8, 0x4F, 0xB3, 0xFE, 0xFF])  # 0x00453b07
+    nop_patch(0x74832, 5, [0xE8, 0x24, 0x9A, 0xFC, 0xFF])  # 0x00475432
+
+    args.path.open("wb").write(binary)
+    print("done")
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tools/reccmp/reccmp.py

@@ -23,6 +23,7 @@
 import colorama
 from pystache import Renderer
 
+
 REGISTER_LIST = set(
     [
         "ax",
@@ -200,7 +201,8 @@ def gen_svg(svg_file, name_svg, icon, svg_implemented_funcs, total_funcs, raw_ac
 
 
 # Do the actual work
-if __name__ == "__main__":
+def main():
+    # pylint: disable=too-many-locals, too-many-nested-blocks, too-many-branches, too-many-statements
     parser = argparse.ArgumentParser(
         allow_abbrev=False,
         description="Recompilation Compare: compare an original EXE with a recompiled EXE + PDB.",
@@ -483,3 +485,7 @@ def gen_svg(svg_file, name_svg, icon, svg_implemented_funcs, total_funcs, raw_ac
                         function_count,
                         total_effective_accuracy,
                     )
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tools/verexp/verexp.py

@@ -8,36 +8,43 @@
 
 from isledecomp.utils import print_diff
 
-parser = argparse.ArgumentParser(
-    allow_abbrev=False, description="Verify Exports: Compare the exports of two DLLs."
-)
-parser.add_argument("original", metavar="original-binary", help="The original binary")
-parser.add_argument(
+
+def main():
+    parser = argparse.ArgumentParser(
+        allow_abbrev=False,
+        description="Verify Exports: Compare the exports of two DLLs.",
+    )
+    parser.add_argument(
+        "original", metavar="original-binary", help="The original binary"
+    )
+    parser.add_argument(
         "recompiled", metavar="recompiled-binary", help="The recompiled binary"
-)
-parser.add_argument(
+    )
+    parser.add_argument(
         "--no-color", "-n", action="store_true", help="Do not color the output"
-)
+    )
 
-args = parser.parse_args()
+    args = parser.parse_args()
 
-if not os.path.isfile(args.original):
+    if not os.path.isfile(args.original):
         parser.error(f"Original binary file {args.original} does not exist")
 
-if not os.path.isfile(args.recompiled):
+    if not os.path.isfile(args.recompiled):
         parser.error(f"Recompiled binary {args.recompiled} does not exist")
 
-
-def get_file_in_script_dir(fn):
+    def get_file_in_script_dir(fn):
         return os.path.join(os.path.dirname(os.path.abspath(sys.argv[0])), fn)
 
-
-def get_exports(file):
+    def get_exports(file):
         call = [get_file_in_script_dir("DUMPBIN.EXE"), "/EXPORTS"]
 
         if os.name != "nt":
             call.insert(0, "wine")
-        file = subprocess.check_output(["winepath", "-w", file]).decode("utf-8").strip()
+            file = (
+                subprocess.check_output(["winepath", "-w", file])
+                .decode("utf-8")
+                .strip()
+            )
 
         call.append(file)
 
@@ -58,11 +65,14 @@ def get_exports(file):
 
         return exports
 
+    og_exp = get_exports(args.original)
+    re_exp = get_exports(args.recompiled)
 
-og_exp = get_exports(args.original)
-re_exp = get_exports(args.recompiled)
+    udiff = difflib.unified_diff(og_exp, re_exp)
+    has_diff = print_diff(udiff, args.no_color)
 
-udiff = difflib.unified_diff(og_exp, re_exp)
-has_diff = print_diff(udiff, args.no_color)
+    return 1 if has_diff else 0
 
-sys.exit(1 if has_diff else 0)
+
+if __name__ == "__main__":
+    raise SystemExit(main())