-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix runtime architecture detection logic in ANCM #63652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -411,7 +411,6 @@ HostFxrResolver::InvokeWhereToFindDotnet() | |
HandleWrapper<InvalidHandleTraits> hThread; | ||
CComBSTR pwzDotnetName = nullptr; | ||
DWORD dwFilePointer = 0; | ||
BOOL fIsCurrentProcess64Bit = FALSE; | ||
DWORD dwExitCode = 0; | ||
STRU struDotnetSubstring; | ||
STRU struDotnetLocationsString; | ||
|
@@ -426,6 +425,7 @@ HostFxrResolver::InvokeWhereToFindDotnet() | |
securityAttributes.bInheritHandle = TRUE; | ||
|
||
LOG_INFO(L"Invoking where.exe to find dotnet.exe"); | ||
auto currentProcessArch = Environment::GetCurrentProcessArchitecture(); | ||
|
||
// Create a read/write pipe that will be used for reading the result of where.exe | ||
FINISHED_LAST_ERROR_IF(!CreatePipe(&hStdOutReadPipe, &hStdOutWritePipe, &securityAttributes, 0)); | ||
|
@@ -499,13 +499,9 @@ HostFxrResolver::InvokeWhereToFindDotnet() | |
} | ||
|
||
FINISHED_IF_FAILED(struDotnetLocationsString.CopyA(pzFileContents, dwNumBytesRead)); | ||
|
||
LOG_INFOF(L"where.exe invocation returned: '%ls'", struDotnetLocationsString.QueryStr()); | ||
|
||
fIsCurrentProcess64Bit = Environment::IsRunning64BitProcess(); | ||
|
||
LOG_INFOF(L"Current process bitness type detected as isX64=%d", fIsCurrentProcess64Bit); | ||
|
||
// Look for a dotnet.exe that matches the current process architecture | ||
while (TRUE) | ||
{ | ||
index = struDotnetLocationsString.IndexOf(L"\r\n", prevIndex); | ||
|
@@ -518,37 +514,47 @@ HostFxrResolver::InvokeWhereToFindDotnet() | |
// \r\n is two wchars, so add 2 here. | ||
prevIndex = index + 2; | ||
|
||
LOG_INFOF(L"Processing entry '%ls'", struDotnetSubstring.QueryStr()); | ||
|
||
if (fIsCurrentProcess64Bit == IsX64(struDotnetSubstring.QueryStr())) | ||
ProcessorArchitecture dotnetArch = GetFileProcessorArchitecture(struDotnetSubstring.QueryStr()); | ||
if (dotnetArch == currentProcessArch) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if both if (dotnetArch == currentProcessArch)
{
if (dotnetArch == ProcessorArchitecture::Unknown)
{
LOG_INFOF(L"dotnet.exe and current process architectures are unknown. Behavior can be unexpected");
}
// continue
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
// The bitness of dotnet matched with the current worker process bitness. | ||
LOG_INFOF(L"Found dotnet.exe matching current process architecture (%ls) '%ls'", | ||
ProcessorArchitectureToString(dotnetArch), | ||
struDotnetSubstring.QueryStr()); | ||
|
||
return std::make_optional(struDotnetSubstring.QueryStr()); | ||
} | ||
else | ||
{ | ||
LOG_INFOF(L"Skipping dotnet.exe with non-matching architecture %ls (need %ls). '%ls'", | ||
ProcessorArchitectureToString(dotnetArch), | ||
ProcessorArchitectureToString(currentProcessArch), | ||
struDotnetSubstring.QueryStr()); | ||
} | ||
} | ||
|
||
Finished: | ||
return result; | ||
} | ||
|
||
BOOL HostFxrResolver::IsX64(const WCHAR* dotnetPath) | ||
// Reads the PE header of the binary to determine its architecture. | ||
ProcessorArchitecture HostFxrResolver::GetFileProcessorArchitecture(const WCHAR* binaryPath) | ||
{ | ||
// Errors while reading from the file shouldn't throw unless | ||
// file.exception(bits) is set | ||
std::ifstream file(dotnetPath, std::ios::binary); | ||
std::ifstream file(binaryPath, std::ios::binary); | ||
if (!file.is_open()) | ||
{ | ||
LOG_TRACEF(L"Failed to open file %ls", dotnetPath); | ||
return false; | ||
LOG_TRACEF(L"Failed to open file %ls", binaryPath); | ||
return ProcessorArchitecture::Unknown; | ||
} | ||
|
||
// Read the DOS header | ||
IMAGE_DOS_HEADER dosHeader{}; | ||
file.read(reinterpret_cast<char*>(&dosHeader), sizeof(dosHeader)); | ||
if (dosHeader.e_magic != IMAGE_DOS_SIGNATURE) // 'MZ' | ||
{ | ||
LOG_TRACEF(L"%ls is not a valid executable file (missing MZ header).", dotnetPath); | ||
return false; | ||
LOG_TRACEF(L"%ls is not a valid executable file (missing MZ header).", binaryPath); | ||
return ProcessorArchitecture::Unknown; | ||
} | ||
|
||
// Seek to the PE header | ||
|
@@ -559,32 +565,30 @@ BOOL HostFxrResolver::IsX64(const WCHAR* dotnetPath) | |
file.read(reinterpret_cast<char*>(&peSignature), sizeof(peSignature)); | ||
if (peSignature != IMAGE_NT_SIGNATURE) // 'PE\0\0' | ||
{ | ||
LOG_TRACEF(L"%ls is not a valid PE file (missing PE header).", dotnetPath); | ||
return false; | ||
LOG_TRACEF(L"%ls is not a valid PE file (missing PE header).", binaryPath); | ||
return ProcessorArchitecture::Unknown; | ||
} | ||
|
||
// Read the file header | ||
IMAGE_FILE_HEADER fileHeader{}; | ||
file.read(reinterpret_cast<char*>(&fileHeader), sizeof(fileHeader)); | ||
|
||
// Read the optional header magic field | ||
WORD magic{}; | ||
file.read(reinterpret_cast<char*>(&magic), sizeof(magic)); | ||
|
||
// Determine the architecture based on the magic value | ||
if (magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) | ||
// Determine the architecture based on the machine type | ||
switch (fileHeader.Machine) | ||
{ | ||
LOG_INFOF(L"%ls is 32-bit", dotnetPath); | ||
return false; | ||
} | ||
else if (magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) | ||
{ | ||
LOG_INFOF(L"%ls is 64-bit", dotnetPath); | ||
return true; | ||
case IMAGE_FILE_MACHINE_I386: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, if I understand docs correctly, then there are only 3 valid combinations -- assuming that's true, would it be worth asserting magic in these various cases, to record our understanding? given the confusion that we've already had here |
||
LOG_INFOF(L"%ls is x86 (32-bit)", binaryPath); | ||
return ProcessorArchitecture::x86; | ||
case IMAGE_FILE_MACHINE_AMD64: | ||
LOG_INFOF(L"%ls is AMD64 (x64)", binaryPath); | ||
return ProcessorArchitecture::AMD64; | ||
case IMAGE_FILE_MACHINE_ARM64: | ||
LOG_INFOF(L"%ls is ARM64", binaryPath); | ||
return ProcessorArchitecture::ARM64; | ||
default: | ||
LOG_INFOF(L"%ls has unknown architecture (machine type: 0x%X)", binaryPath, fileHeader.Machine); | ||
return ProcessorArchitecture::Unknown; | ||
} | ||
|
||
LOG_INFOF(L"%ls is unknown architecture %i", dotnetPath, fileHeader.Machine); | ||
return false; | ||
} | ||
|
||
std::optional<fs::path> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT License. See License.txt in the project root for license information. | ||
|
||
#pragma once | ||
|
||
enum class ProcessorArchitecture | ||
{ | ||
Unknown, | ||
x86, | ||
AMD64, | ||
ARM64 | ||
}; | ||
|
||
inline const wchar_t* ProcessorArchitectureToString(ProcessorArchitecture arch) | ||
{ | ||
switch (arch) | ||
{ | ||
case ProcessorArchitecture::x86: | ||
return L"x86"; | ||
case ProcessorArchitecture::AMD64: | ||
return L"AMD64"; | ||
case ProcessorArchitecture::ARM64: | ||
return L"ARM64"; | ||
case ProcessorArchitecture::Unknown: | ||
default: | ||
return L"Unknown"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace
static_assert(false, ...)
withstatic_assert(false, "Unknown target architecture")
or use a dependent false expression likestatic_assert(sizeof(void*) == 0, "Unknown target architecture")
to avoid potential compilation issues with newer compilers that may reject unconditional static_assert(false).Copilot uses AI. Check for mistakes.