-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix NativeAOT tracing tests using AppContext.BaseDirectory and Environment.ProcessPath #123553
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: mdh1418 <16830051+mdh1418@users.noreply.github.com>
Co-authored-by: mdh1418 <16830051+mdh1418@users.noreply.github.com>
Co-authored-by: mdh1418 <16830051+mdh1418@users.noreply.github.com>
…ling to actually tail call (#123513) call.tail in the interpreter isn't guaranteed to tail-call in some cases, so we need to put in a ret instruction after the tail-call to ensure that the runtime doesn't execute invalid code. The code for doing a ret is extracted into a helper routine, and now called from both the CEE_JMP and CEE_RET pathways. The only change made to the code was to unify where the ip adjustment was to not happen in the EmitRet logic and instead keep it all in the same place in CEE_RET handling case. This fixes these test cases on Windows Arm64 JIT/Directed/pinvoke/jump JIT/Directed/pinvoke/tail_pinvoke
These are all crashing: ``` 11:31:58.741 Running test: tracing/userevents/custommetadata/custommetadata/custommetadata.cmd Unhandled exception. System.ArgumentNullException: Value cannot be null. (Parameter 'path1') at System.ArgumentNullException.Throw(String) at System.IO.Path.Combine(String, String, String) at Tracing.UserEvents.Tests.Common.UserEventsTestRunner.ResolveRecordTracePath(String) at Tracing.UserEvents.Tests.Common.UserEventsTestRunner.RunOrchestrator(String, String, Func`2, Int32, Int32) at Tracing.UserEvents.Tests.CustomMetadata.CustomMetadata.Main(String[] args) ```
Test started running with #123112 Cc @dotnet/ilc-contrib
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@copilot on NativeAOT, it looks like the runtime tests layout is a bit different as they will embed an extra Instead of using Afterwards can you run the |
…OT compatibility Co-authored-by: mdh1418 <16830051+mdh1418@users.noreply.github.com>
Updated to use |
|
@copilot we only needed to revert one commit, 9f1fdb4. Can you clean up your commit history and have the first commit be reverting 9f1fdb4 and then apply the fix for the userevents tests for nativeAOT? Additionally nativeAOT tests aren't using corerun, compared to non-nativeAOT tests. |
NativeAOT tests run executables directly (not through corerun), so use Environment.ProcessPath instead of Process.GetCurrentProcess().MainModule.FileName. Co-authored-by: mdh1418 <16830051+mdh1418@users.noreply.github.com>
Updated to use
|
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Failing without NAOT: |
Description
NativeAOT tracing tests in
src/tests/tracing/userevents/*crash withArgumentNullExceptioninResolveRecordTracePathbecauseAssembly.Locationreturns empty string in NativeAOT builds. Additionally, NativeAOT has a different directory layout with executables placed in anative/subdirectory, and NativeAOT tests run executables directly without using corerun.This PR fixes the tests to work with both CoreCLR and NativeAOT by using
AppContext.BaseDirectoryfor path resolution andEnvironment.ProcessPathfor tracee execution, and reverts PR #123541 which had marked these tests as NativeAOT incompatible.Changes Made
AppContext.BaseDirectoryfor path resolution: Changed fromAssembly.LocationtoAppContext.BaseDirectorywhich works consistently for both CoreCLR and NativeAOT runtime layoutsEnvironment.ProcessPathfor tracee execution: Changed fromProcess.GetCurrentProcess().MainModule.FileNametoEnvironment.ProcessPathto support NativeAOT's direct executable execution (without corerun)traceeAssemblyPathparameter: Eliminated thetraceeAssemblyPathparameter fromUserEventsTestRunner.Run()method signature since it's no longer neededtypeof(<class>).Assembly.LocationparameterResolveRecordTracePath()sinceAppContext.BaseDirectoryis always validDirectory.Build.propsthat marked tests asNativeAotIncompatiblesince the fix makes them compatibleTechnical Notes
AppContext.BaseDirectoryconsistently points to.../tracing/userevents/<scenario>/<scenario>/for both:native/subdirectory, the base directory is still the parentTracee execution differs between runtimes:
corerun <path-to-scenario.dll> tracee)<native-executable> tracee)Using
Environment.ProcessPathhandles both scenarios correctly, returning the path to corerun for CoreCLR tests and the path to the native executable for NativeAOT tests.This allows both the scenario script (e.g.,
basic.script) and the record-trace executable (common/userevents_common/record-trace) to be resolved correctly using the same logic for both runtimes.TMPDIR is already configured correctly, but diagnostic port discovery will still fail for NativeAOT due to the perfmap dependency (see microsoft/one-collect#226).
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.