From 2a16a508a500bb92c0832cb6def5a2b1bf991ab7 Mon Sep 17 00:00:00 2001 From: Christian Semmler Date: Fri, 8 Dec 2023 06:37:44 -0500 Subject: [PATCH] (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 Co-authored-by: Anonymous Maarten --- .editorconfig | 2 +- .github/workflows/build.yml | 4 ++ .pylintrc | 4 +- CMakeLists.txt | 5 +- LEGO1/compat.h | 6 -- tools/patch_c2.py | 67 +++++++++++++++++++++ tools/reccmp/reccmp.py | 8 ++- tools/verexp/verexp.py | 116 ++++++++++++++++++++---------------- 8 files changed, 147 insertions(+), 65 deletions(-) create mode 100644 tools/patch_c2.py diff --git a/.editorconfig b/.editorconfig index 604cc306..c6a6539f 100644 --- a/.editorconfig +++ b/.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 diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8434d906..58cce4dc 100644 --- a/.github/workflows/build.yml +++ b/.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: | diff --git a/.pylintrc b/.pylintrc index 7afd8185..ab83fceb 100644 --- a/.pylintrc +++ b/.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] diff --git a/CMakeLists.txt b/CMakeLists.txt index 9efb9c99..4a60e3d4 100644 --- a/CMakeLists.txt +++ b/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. - - target_compile_options(isle PRIVATE "/ML$<$:d>") + if (ISLE_BUILD_APP) + target_compile_options(isle PRIVATE "/ML$<$:d>") + endif() target_compile_options(lego1 PRIVATE "/MT$<$:d>") set(CMAKE_CXX_FLAGS "/W3 /GX /D \"WIN32\" /D \"_WINDOWS\"") diff --git a/LEGO1/compat.h b/LEGO1/compat.h index d9cc2170..9616b254 100644 --- a/LEGO1/compat.h +++ b/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 diff --git a/tools/patch_c2.py b/tools/patch_c2.py new file mode 100644 index 00000000..58cc4760 --- /dev/null +++ b/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()) diff --git a/tools/reccmp/reccmp.py b/tools/reccmp/reccmp.py index d2a7f23a..98a8a4d9 100755 --- a/tools/reccmp/reccmp.py +++ b/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()) diff --git a/tools/verexp/verexp.py b/tools/verexp/verexp.py index 37e3e641..455791e1 100755 --- a/tools/verexp/verexp.py +++ b/tools/verexp/verexp.py @@ -8,61 +8,71 @@ 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( - "recompiled", metavar="recompiled-binary", help="The recompiled binary" -) -parser.add_argument( - "--no-color", "-n", action="store_true", help="Do not color the output" -) -args = parser.parse_args() +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( + "--no-color", "-n", action="store_true", help="Do not color the output" + ) -if not os.path.isfile(args.original): - parser.error(f"Original binary file {args.original} does not exist") + args = parser.parse_args() -if not os.path.isfile(args.recompiled): - parser.error(f"Recompiled binary {args.recompiled} does not exist") + 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): + parser.error(f"Recompiled binary {args.recompiled} does not exist") + + 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): + 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() + ) + + call.append(file) + + raw = subprocess.check_output(call).decode("utf-8").split("\r\n") + exports = [] + + start = False + + for line in raw: + if not start: + if line == " ordinal hint name": + start = True + else: + if line: + exports.append(line[27 : line.rindex(" (")]) + elif exports: + break + + return exports + + 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) + + return 1 if has_diff else 0 -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): - 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() - - call.append(file) - - raw = subprocess.check_output(call).decode("utf-8").split("\r\n") - exports = [] - - start = False - - for line in raw: - if not start: - if line == " ordinal hint name": - start = True - else: - if line: - exports.append(line[27 : line.rindex(" (")]) - elif exports: - break - - return exports - - -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) - -sys.exit(1 if has_diff else 0) +if __name__ == "__main__": + raise SystemExit(main())