From 62f3f667f439304899fad818f4285e38008be544 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Tue, 26 Jan 2021 22:28:11 -0800 Subject: [PATCH] =?UTF-8?q?=EF=BB=BFEnable=20scripting=20of=20SuperPMI=20c?= =?UTF-8?q?ollection=20of=20crossgen2=20compilations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `superpmi.py collect` has a new `--crossgen2` option that specifies to run crossgen2 on all the assemblies specified with the `-assemblies` option. For example: ``` py c:\rt\src\coreclr\scripts\superpmi.py collect --crossgen2 -output_mch_path c:\win-x64-cg2.mch -assemblies c:\rt\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\System.Private.CoreLib.dll ``` There are currently quite a few failures during the replay "cleaning" phase with a System.Private.CoreLib collection: 5654 compilation failures out of 14705 functions, including 100 JIT asserts. In contrast, the crossgen(1) collection has 3 failures out of 20165 functions. So, there is some work to be done to make SuperPMI collection of crossgen2 compilations work well. The change in spmirecordhelper.h is to work around what appears to be a crossgen2 bug: when the JIT calls `getCallInfo`, sometimes crossgen2 sets sig.sigInst.methInstCount in the result CORINFO_CALL_INFO to non-zero, but it sets sig.sigInst.methInst to `nullptr`, which should be illegal: the count indicates how many elements are in the array, so you can?t have a null array with a non-zero count. Maybe the JIT doesn?t look at these fields in this scenario for some reason, but SPMI tries to serialize/de-serialize the array, and was crashing. Fixes #45909 --- .../superpmi-shared/spmirecordhelper.h | 11 +- src/coreclr/scripts/superpmi.py | 251 +++++++++++++++--- 2 files changed, 226 insertions(+), 36 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h index a57681c3138c87..932e192186c364 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/spmirecordhelper.h @@ -232,7 +232,9 @@ inline void SpmiRecordsHelper::StoreAgnostic_CORINFO_SIG_INST_HandleArray( { unsigned handleInstIndex; - if (handleInstCount > 0) + // We shouldn't need to check (handleInstArray != nullptr), but often, crossgen2 sets (leaves?) + // handleInstCount > 0 and handleInstArray == nullptr. + if ((handleInstCount > 0) && (handleInstArray != nullptr)) { if (handleMap == nullptr) handleMap = new DenseLightWeightMap(); // this updates the caller @@ -254,6 +256,13 @@ inline void SpmiRecordsHelper::StoreAgnostic_CORINFO_SIG_INST_HandleArray( } else { + if (handleInstCount > 0) + { + // This is really a VM/crossgen bug. + LogVerbose("(handleInstCount > 0) but (handleInstArray == nullptr)!"); + handleInstCount = 0; + } + handleInstIndex = (DWORD)-1; } diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 634018d2e77582..5ea16dd1f73efd 100755 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -243,7 +243,8 @@ collect_parser.add_argument("--pmi", action="store_true", help="Run PMI on a set of directories or assemblies.") collect_parser.add_argument("--crossgen", action="store_true", help="Run crossgen on a set of directories or assemblies.") -collect_parser.add_argument("-assemblies", dest="assemblies", nargs="+", default=[], help="Pass a sequence of managed dlls or directories to recursively use while collecting with PMI or crossgen. Required if --pmi or --crossgen is specified.") +collect_parser.add_argument("--crossgen2", action="store_true", help="Run crossgen2 on a set of directories or assemblies.") +collect_parser.add_argument("-assemblies", dest="assemblies", nargs="+", default=[], help="Pass a sequence of managed dlls or directories to recursively use while collecting with PMI, crossgen, or crossgen2. Required if --pmi, --crossgen, or --crossgen2 is specified.") collect_parser.add_argument("-pmi_location", help="Path to pmi.dll to use during PMI run. Optional; pmi.dll will be downloaded from Azure Storage if necessary.") collect_parser.add_argument("-output_mch_path", help="Location to place the final MCH file.") collect_parser.add_argument("--merge_mch_files", action="store_true", help="Merge multiple MCH files. Use the -mch_files flag to pass a list of MCH files to merge.") @@ -591,8 +592,8 @@ async def __run_to_completion__(self, async_callback, *extra_args): """ # Create a queue with one entry for each of the threads we're - # going to allow. By default, this will be one entry per cpu. - # using subproc_count_queue.get() will block when we're running + # going to allow. By default, this will be one entry per CPU. + # Using subproc_count_queue.get() will block when we're running # a task on every CPU. chunk_size = self.subproc_count self.subproc_count_queue = asyncio.Queue(chunk_size) @@ -681,7 +682,7 @@ def __init__(self, coreclr_args): if coreclr_args.crossgen: self.crossgen_tool = os.path.join(self.core_root, self.crossgen_tool_name) - if coreclr_args.pmi or coreclr_args.crossgen: + if coreclr_args.pmi or coreclr_args.crossgen or coreclr_args.crossgen2: self.assemblies = coreclr_args.assemblies self.coreclr_args = coreclr_args @@ -785,31 +786,51 @@ def __collect_mc_files__(self): if not self.coreclr_args.skip_collect_mc_files: assert os.path.isdir(self.temp_location) - # Set environment variables. + # Set environment variables. For crossgen2, we need to pass the COMPlus variables as arguments to the JIT using + # the `-codegenopt` argument. + env_copy = os.environ.copy() - env_copy["SuperPMIShimLogPath"] = self.temp_location - env_copy["SuperPMIShimPath"] = self.jit_path - env_copy["COMPlus_JitName"] = self.collection_shim_name - env_copy["COMPlus_EnableExtraSuperPmiQueries"] = "1" - env_copy["COMPlus_TieredCompilation"] = "0" + + root_env = {} + root_env["SuperPMIShimLogPath"] = self.temp_location + root_env["SuperPMIShimPath"] = self.jit_path + + complus_env = {} + complus_env["EnableExtraSuperPmiQueries"] = "1" + complus_env["TieredCompilation"] = "0" if self.coreclr_args.use_zapdisable: - env_copy["COMPlus_ZapDisable"] = "1" - env_copy["COMPlus_ReadyToRun"] = "0" + complus_env["ZapDisable"] = "1" + complus_env["ReadyToRun"] = "0" logging.debug("Starting collection.") logging.debug("") - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "SuperPMIShimLogPath", self.temp_location) - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "SuperPMIShimPath", self.jit_path) - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "COMPlus_JitName", self.collection_shim_name) - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "COMPlus_EnableExtraSuperPmiQueries", "1") - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "COMPlus_TieredCompilation", "0") - if self.coreclr_args.use_zapdisable: - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "COMPlus_ZapDisable", "1") - print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, "COMPlus_ReadyToRun", "0") - logging.debug("") + def set_and_report_env(env, root_env, complus_env = None): + for var, value in root_env.items(): + env[var] = value + print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, var, value) + if complus_env is not None: + for var, value in complus_env.items(): + complus_var = "COMPlus_" + var + env[complus_var] = value + print_platform_specific_environment_vars(logging.DEBUG, self.coreclr_args, complus_var, value) + + # If we need them, collect all the assemblies we're going to use for the collection(s) + if self.coreclr_args.pmi or self.coreclr_args.crossgen or self.coreclr_args.crossgen2: + assemblies = [] + for item in self.assemblies: + assemblies += get_files_from_path(item, match_func=lambda file: any(file.endswith(extension) for extension in [".dll", ".exe"])) + + ################################################################################################ Do collection using given collection command (e.g., script) if self.collection_command is not None: + logging.debug("Starting collection using command") + + collection_command_env = env_copy.copy() + collection_complus_env = complus_env.copy() + collection_complus_env["JitName"] = self.collection_shim_name + set_and_report_env(collection_command_env, root_env, collection_complus_env) + logging.info("Collecting using command:") logging.info(" %s %s", self.collection_command, " ".join(self.collection_args)) @@ -817,17 +838,16 @@ def __collect_mc_files__(self): assert isinstance(self.collection_args, list) command = [self.collection_command, ] + self.collection_args - proc = subprocess.Popen(command, env=env_copy, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + proc = subprocess.Popen(command, env=collection_command_env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) stdout_output, _ = proc.communicate() for line in stdout_output.decode('utf-8').splitlines(): # There won't be any stderr output since it was piped to stdout logging.debug(line) + ################################################################################################ end of "self.collection_command is not None" - if self.coreclr_args.pmi is True or self.coreclr_args.crossgen is True: - assemblies = [] - for item in self.assemblies: - assemblies += get_files_from_path(item, match_func=lambda file: any(file.endswith(extension) for extension in [".dll", ".exe"])) - + ################################################################################################ Do collection using PMI if self.coreclr_args.pmi is True: + logging.debug("Starting collection using PMI") + async def run_pmi(print_prefix, assembly, self): """ Run pmi over all dlls """ @@ -867,16 +887,25 @@ async def run_pmi(print_prefix, assembly, self): raise ose # Set environment variables. + pmi_command_env = env_copy.copy() + pmi_complus_env = complus_env.copy() + pmi_complus_env["JitName"] = self.collection_shim_name + set_and_report_env(pmi_command_env, root_env, pmi_complus_env) + old_env = os.environ.copy() - os.environ.update(env_copy) + os.environ.update(pmi_command_env) helper = AsyncSubprocessHelper(assemblies, verbose=True) helper.run_to_completion(run_pmi, self) + # Review: does this delete the items that weren't there before we updated with the PMI variables? os.environ.update(old_env) ################################################################################################ end of "self.coreclr_args.pmi is True" + ################################################################################################ Do collection using crossgen if self.coreclr_args.crossgen is True: + logging.debug("Starting collection using crossgen") + async def run_crossgen(print_prefix, assembly, self): """ Run crossgen over all dlls """ @@ -889,7 +918,7 @@ async def run_crossgen(print_prefix, assembly, self): except OSError as ose: if "[WinError 32] The process cannot access the file because it is being used by another " \ "process:" in format(ose): - logging.warning("Skipping file %s. Got error: %s".format(root_output_filename, format(ose))) + logging.warning("Skipping file %s. Got error: %s".format(crossgen_output_assembly_filename, format(ose))) return else: raise ose @@ -929,15 +958,140 @@ async def run_crossgen(print_prefix, assembly, self): raise ose # Set environment variables. + crossgen_command_env = env_copy.copy() + crossgen_complus_env = complus_env.copy() + crossgen_complus_env["JitName"] = self.collection_shim_name + set_and_report_env(crossgen_command_env, root_env, crossgen_complus_env) + old_env = os.environ.copy() - os.environ.update(env_copy) + os.environ.update(crossgen_command_env) helper = AsyncSubprocessHelper(assemblies, verbose=True) helper.run_to_completion(run_crossgen, self) + # Review: does this delete the items that weren't there before we updated with the crossgen variables? os.environ.update(old_env) ################################################################################################ end of "self.coreclr_args.crossgen is True" + ################################################################################################ Do collection using crossgen2 + if self.coreclr_args.crossgen2 is True: + logging.debug("Starting collection using crossgen2") + + async def run_crossgen2(print_prefix, assembly, self): + """ Run crossgen2 over all dlls + """ + + root_crossgen2_output_filename = make_safe_filename("crossgen2_" + assembly) + ".out.dll" + crossgen2_output_assembly_filename = os.path.join(self.temp_location, root_crossgen2_output_filename) + try: + if os.path.exists(crossgen2_output_assembly_filename): + os.remove(crossgen2_output_assembly_filename) + except OSError as ose: + if "[WinError 32] The process cannot access the file because it is being used by another " \ + "process:" in format(ose): + logging.warning("Skipping file %s. Got error: %s".format(crossgen2_output_assembly_filename, format(ose))) + return + else: + raise ose + + root_output_filename = make_safe_filename("crossgen2_" + assembly + "_") + + # Create a temporary response file to put all the arguments to crossgen2 (otherwise the path length limit could be exceeded): + # + # + # -o: + # -r:\System.*.dll + # -r:\Microsoft.*.dll + # -r:\mscorlib.dll + # -r:\netstandard.dll + # --jitpath: + # --codegenopt: