Skip to content

Adding ARM64 support to the test console #1927

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.

Sign up for GitHub

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

Closed
dotMorten wants to merge 3 commits into from
Closed

Adding ARM64 support to the test console #1927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7efde93
Adds ARM64 support for test platform
dotMorten Feb 21, 2019
c01c544
Improve arm64 detection
dotMorten Feb 21, 2019
0236fc5
Update expected test message to fix unit test failures
dotMorten Feb 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Microsoft.TestPlatform.ObjectModel/Architecture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public enum Architecture
X86,
X64,
ARM,
AnyCPU
AnyCPU,
ARM64
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ public static RunConfiguration FromXml(XmlReader reader)
try
{
archType = (Architecture)Enum.Parse(typeof(Architecture), value, true);
if (archType != Architecture.X64 && archType != Architecture.X86 && archType != Architecture.ARM)
if (archType != Architecture.X64 && archType != Architecture.X86 && archType != Architecture.ARM && archType != Architecture.ARM64)
{
throw new SettingsException(
string.Format(
Expand Down
37 changes: 37 additions & 0 deletions src/Microsoft.TestPlatform.ObjectModel/Utilities/XmlRunSettingsUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,29 @@ public static ObjectModel.Architecture OSArchitecture
{
get
{
#if !NETSTANDARD1_4
Copy link
Contributor

@singhsarab singhsarab Feb 27, 2019

Choose a reason for hiding this comment

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

Can't this come from the PlatformEnvironment ?

Copy link
Contributor Author

@dotMorten dotMorten Feb 27, 2019

Choose a reason for hiding this comment

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

No due to backwards compat reasons this API normally lies about the OS architecture, so this is the way to do it.

try
{
// use the new IsWow64Process2 to detect ARM64 as well.
var processHandle = System.Diagnostics.Process.GetCurrentProcess().Handle;
if (IsWow64Process2(processHandle, out ImageFileMachine pProcessMachinem, out ImageFileMachine pNativeMachine))
Copy link
Member

@AntonLapounov AntonLapounov Jul 2, 2020

Choose a reason for hiding this comment

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

Is the m at the end of pProcessMachinem a typo?

Copy link
Contributor Author

@dotMorten dotMorten Jul 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I submitted this PR almost a year and a half ago, so honestly I have no clue but probably? 🤷‍♂️. To be honest, I don't even remember submitting this PR.
Feel free to take what I have and work with it, but with such a delay in response and in the mean time I found other means for testing on ARM64, I don't see myself picking this PR up.

Copy link
Member

@nohwnd nohwnd Jul 7, 2020

Choose a reason for hiding this comment

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

I will most likely pick this up, when we get ARM64 hardware.

{
switch (pNativeMachine)
{
case ImageFileMachine.AMD64:
case ImageFileMachine.IA64:
return Architecture.X64;
Comment on lines +39 to +40
Copy link
Member

@AntonLapounov AntonLapounov Jul 2, 2020

Choose a reason for hiding this comment

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

IA64 (Itanium) should not be mapped to X64.

case ImageFileMachine.X86:
return Architecture.X86;
case ImageFileMachine.ARM:
return Architecture.ARM;
case ImageFileMachine.ARM64:
return Architecture.ARM64;
}
}
}
catch { }
#endif
var arch = new PlatformEnvironment().Architecture;

switch (arch)
Expand All @@ -34,11 +57,25 @@ public static ObjectModel.Architecture OSArchitecture
return ObjectModel.Architecture.X64;
case PlatformArchitecture.X86:
return ObjectModel.Architecture.X86;
case PlatformArchitecture.ARM64:
return ObjectModel.Architecture.ARM64;
default:
return ObjectModel.Architecture.ARM;
}
}
}
#if !NETSTANDARD1_4
private enum ImageFileMachine : ushort
{
X86 = 0x014c,
ARM = 0x01c0,
IA64 = 0x0200,
AMD64 = 0x8664,
ARM64 = 0xAA64
}
[System.Runtime.InteropServices.DllImport("kernel32")]
private static extern bool IsWow64Process2([System.Runtime.InteropServices.In] IntPtr processHandle, [System.Runtime.InteropServices.Out] out ImageFileMachine pProcessMachinem, [System.Runtime.InteropServices.Out] out ImageFileMachine pNativeMachine);
#endif

/// <summary>
/// Gets the settings to be used while creating XmlReader for runsettings.
Expand Down
6 changes: 5 additions & 1 deletion src/Microsoft.TestPlatform.Utilities/InferRunSettingsHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,11 @@ private static void VerifyCompatibilityWithOSArchitecture(Architecture architect
{
var osArchitecture = XmlRunSettingsUtilities.OSArchitecture;

if (architecture == Architecture.X86 && osArchitecture == Architecture.X64)
if (architecture == Architecture.X86 && (osArchitecture == Architecture.X64 || osArchitecture == Architecture.ARM64))
{
return;
}
if ((architecture == Architecture.ARM || architecture == Architecture.X86) && osArchitecture == Architecture.ARM64)
Comment on lines +519 to +523
Copy link
Member

@AntonLapounov AntonLapounov Jul 2, 2020

Choose a reason for hiding this comment

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

The combination architecture == Architecture.X86 && osArchitecture == Architecture.ARM64 is checked twice. Using IsWow64GuestMachineSupported may be more appropriate here.

{
return;
}
Expand Down
5 changes: 4 additions & 1 deletion src/vstest.console/CommandLine/AssemblyMetadataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public Architecture GetArchitectureForSource(string imagePath)
const int IMAGE_FILE_MACHINE_ARM = 0x01c0; // ARM Little-Endian
const int IMAGE_FILE_MACHINE_THUMB = 0x01c2; // ARM Thumb/Thumb-2 Little-Endian
const int IMAGE_FILE_MACHINE_ARMNT = 0x01c4; // ARM Thumb-2 Little-Endian

const int IMAGE_FILE_MACHINE_ARM64 = 0xAA64; // ARM64 Little-Endian

try
{
Expand Down Expand Up @@ -235,6 +235,9 @@ public Architecture GetArchitectureForSource(string imagePath)
case IMAGE_FILE_MACHINE_ARMNT:
archType = Architecture.ARM;
break;
case IMAGE_FILE_MACHINE_ARM64:
archType = Architecture.ARM64;
break;
}
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/vstest.console/Processors/PlatformArgumentProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void Initialize(string argument)
var validPlatform = Enum.TryParse(argument, true, out platform);
if (validPlatform)
{
validPlatform = platform == Architecture.X86 || platform == Architecture.X64 || platform == Architecture.ARM;
validPlatform = platform == Architecture.X86 || platform == Architecture.X64 || platform == Architecture.ARM || platform == Architecture.ARM64;
}

if (validPlatform)
Expand Down
2 changes: 1 addition & 1 deletion src/vstest.console/Resources/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@
<value>The --ParentProcessId|/ParentProcessId argument requires the process id which is an integer. Specify the process id of the parent process that launched this process.</value>
</data>
<data name="InvalidPlatformType" xml:space="preserve">
<value>Invalid platform type:{0}. Valid platform types are x86, x64 and Arm.</value>
<value>Invalid platform type:{0}. Valid platform types are x86, x64, Arm and Arm64.</value>
Copy link
Member

@danmoseley danmoseley Jul 9, 2020

Choose a reason for hiding this comment

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

capitalize ARM and ARM64 in messages?

Copy link
Contributor Author

@dotMorten dotMorten May 19, 2022

Choose a reason for hiding this comment

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

Nope! This is the correct capitalization

Copy link
Member

@AntonLapounov AntonLapounov May 19, 2022

Choose a reason for hiding this comment

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

I would use lower case only as in C# compiler usage help. In any case the two lists below should be consistent, and they are not (@nohwnd):

vstest/src/Microsoft.TestPlatform.ObjectModel/Architecture.cs

Lines 11 to 16 in b999506

X86,
X64,
ARM,
AnyCPU,
ARM64,
S390x

vstest/src/vstest.console/Resources/Resources.resx

Line 452 in b999506

Valid values are x86, x64 and ARM.</value>

</data>
<data name="InvalidPortArgument" xml:space="preserve">
<value>The --Port|/Port argument requires the port number which is an integer. Specify the port for socket connection and receiving the event messages.</value>
Expand Down
4 changes: 2 additions & 2 deletions test/vstest.console.UnitTests/Processors/PlatformArgumentProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void InitializeShouldThrowIfArgumentIsNotAnArchitecture()
{
ExceptionUtilities.ThrowsException<CommandLineException>(
() => this.executor.Initialize("foo"),
"Invalid platform type:{0}. Valid platform types are x86, x64 and Arm.",
"Invalid platform type:{0}. Valid platform types are x86, x64, Arm and Arm64.",
"foo");
}

Expand All @@ -95,7 +95,7 @@ public void InitializeShouldThrowIfArgumentIsNotASupportedArchitecture()
{
ExceptionUtilities.ThrowsException<CommandLineException>(
() => this.executor.Initialize("AnyCPU"),
"Invalid platform type:{0}. Valid platform types are x86, x64 and Arm.",
"Invalid platform type:{0}. Valid platform types are x86, x64, Arm and Arm64.",
"AnyCPU");
}

Expand Down