Skip to content

Conversation

DeagleGross
Copy link
Member

@DeagleGross DeagleGross commented Sep 17, 2025

Description

Backport of #63652 to net9.0

A change (#61894) improved algorithm to determine the architecture of a binary by switching away from GetBinaryTypeW (which seems to load the exe into executable space which might trigger custom windows policies). It was backported to net9.0: #62038.

However, we spotted that sometimes function incorrectly chooses a binary of an incompatible architecture, resulting in crashing site load.

Current change specifically checks for a full compatibility of bitness and architecture.

Fixes #63650

Customer Impact

For example users hosting x86 app on x64 machine / x64 app on arm64 machine are impacted unless current change is merged and used.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Impacts users who are using different process architecture then what where.exe outputs trying to find dotnet binary. It takes OS / process / dotnet arch installed combination to affect the behavior.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@Copilot Copilot AI review requested due to automatic review settings September 17, 2025 18:26
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0.x milestone Sep 17, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR backports a fix for runtime architecture detection logic in the ASP.NET Core Module (ANCM) to .NET 9.0. The changes replace the previous boolean-based x64 detection with a more comprehensive processor architecture system that supports x86, AMD64, and ARM64 architectures.

  • Introduces a new ProcessorArchitecture enum to handle multiple architecture types instead of just x64/x86
  • Replaces runtime PE header magic number detection with compile-time architecture detection
  • Updates architecture matching logic to use the new enum-based system for finding compatible dotnet.exe instances

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ProcessorArchitecture.h New header defining ProcessorArchitecture enum and string conversion utility
HostFxrResolver.h Updates method signature from IsX64 to GetFileProcessorArchitecture
HostFxrResolver.cpp Refactors architecture detection logic and improves PE header parsing
Environment.h Adds GetCurrentProcessArchitecture method declaration
Environment.cpp Implements compile-time architecture detection using preprocessor macros

#elif defined(_M_IX86)
return ProcessorArchitecture::x86;
#else
static_assert(false, "Unknown target architecture");
Copy link
Preview

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use static_assert(false) with a dependent type to avoid immediate compilation failure. Consider using static_assert(sizeof(void*) == 0, ...) or a template-dependent expression to ensure this only triggers when the code path is actually instantiated.

Suggested change
static_assert(false, "Unknown target architecture");
static_assert(sizeof(void*) == 0, "Unknown target architecture");

Copilot uses AI. Check for mistakes.

@danmoseley danmoseley added the Servicing-consider Shiproom approval is required for the issue label Sep 17, 2025
@danmoseley
Copy link
Member

Please send the usual mail to tactics.

@danmoseley danmoseley added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 19, 2025
@joperezr joperezr merged commit e820f58 into release/9.0 Sep 19, 2025
25 checks passed
@joperezr joperezr deleted the dmkorolev/9.0/acnm_arch branch September 19, 2025 21:55
@dotnet-policy-service dotnet-policy-service bot modified the milestones: 9.0.x, 9.0.10 Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants