[browser][coreCLR] Fix IP resolution for interpreted methods on WASM#127370
[browser][coreCLR] Fix IP resolution for interpreted methods on WASM#127370pavelsavara wants to merge 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR fixes EventPipe/ETW instruction-pointer (IP) to managed-method resolution for interpreted methods on WASM, where CoreCLR uses FEATURE_PORTABLE_ENTRYPOINTS instead of interpreter precodes. This enables the sampling profiler to correctly attribute samples to interpreted managed methods.
Changes:
- Update
MethodDesc::JitCompileCodeLockedEventWrapperto derive the interpreted-method “native start address” fromPortableEntryPointinterpreter data whenFEATURE_PORTABLE_ENTRYPOINTSis enabled. - Update
MethodAndStartAddressToEECodeInfoPointerto fall back toMethodDesc::GetInterpreterCode()(returning theInterpByteCodeStartpointer) when no explicit start address is supplied on portable-entrypoint interpreter builds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/vm/prestub.cpp | Uses PortableEntryPoint::GetInterpreterData to compute the method start address for interpreted-method ETW events under portable entrypoints. |
| src/coreclr/vm/eventtrace.cpp | Adds a portable-entrypoints interpreter fallback so method events can resolve InterpByteCodeStart when no explicit start address is provided. |
| @@ -810,8 +810,12 @@ PCODE MethodDesc::JitCompileCodeLockedEventWrapper(PrepareCodeConfig* pConfig, J | |||
| if (isInterpreterCode) | |||
| { | |||
| // If this is interpreter code, we need to get the native code start address from the interpreter Precode | |||
There was a problem hiding this comment.
The comment mentions getting the native start address from the interpreter Precode, but in the FEATURE_PORTABLE_ENTRYPOINTS branch the data comes from PortableEntryPoint interpreter data. Update the comment to reflect both cases; optionally, you could avoid duplicating the extraction logic by using MethodDesc::GetInterpreterCode() (which JitCompileCodeLocked already sets) when isInterpreterCode is true.
| // If this is interpreter code, we need to get the native code start address from the interpreter Precode | |
| // If this is interpreter code, get the native code start address from the | |
| // interpreter entrypoint data: PortableEntryPoint interpreter data when | |
| // FEATURE_PORTABLE_ENTRYPOINTS is enabled, otherwise the interpreter Precode. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| PTR_InterpByteCodeStart pInterpCode = pMethodDesc->GetInterpreterCode(); | ||
| if (pInterpCode != NULL) | ||
| { | ||
| return dac_cast<TADDR>(pInterpCode); |
There was a problem hiding this comment.
In the portable-entrypoints + interpreter fallback, the returned address should be encoded consistently with other interpreter code addresses (e.g., JitCompileCodeLockedEventWrapper passes PINSTRToPCODE(dac_cast<TADDR>(interpreterCode)) to ETW). Returning dac_cast<TADDR>(pInterpCode) bypasses PINSTRToPCODE, which can cause mismatches on architectures where PCODE carries tagging (e.g., ARM THUMB bit) and may break comparisons/method event correlation. Consider returning PINSTRToPCODE(dac_cast<TADDR>(pInterpCode)) (or equivalent) instead of the raw pointer value.
| return dac_cast<TADDR>(pInterpCode); | |
| return PINSTRToPCODE(dac_cast<TADDR>(pInterpCode)); |
| } | ||
|
|
||
| return GetInterpreterCodeFromInterpreterPrecodeIfPresent(start); | ||
| start = GetInterpreterCodeFromInterpreterPrecodeIfPresent(start); |
There was a problem hiding this comment.
Should this be fixed in GetInterpreterCodeFromInterpreterPrecodeIfPresent instead?
Also, GetInterpreterCodeFromInterpreterPrecodeIfPresent may want to be renamed to GetInterpreterCodeFromEntryPointIfPresent to better represent what it does.
Motivation
The sampling profiler does not work on WASM because the EventPipe code paths that resolve instruction pointers (IPs) to method information assume the traditional
InterpreterPrecodemechanism. On WASM, CoreCLR uses portable entrypoints (FEATURE_PORTABLE_ENTRYPOINTS) instead of interpreter precodes, so the existing code silently returns wrong or zero addresses, causing the profiler to fail to attribute samples to managed methods.Split from #126324 for smaller reviews
Changes
src/coreclr/vm/prestub.cppWhen emitting the ETW
MethodJittedevent for interpreted methods, the code needs to extract theInterpByteCodeStartpointer to use as the native code start address. On non-WASM platforms this goes throughInterpreterPrecode::FromEntryPoint→ByteCodeAddr. On WASM (FEATURE_PORTABLE_ENTRYPOINTS), this path doesn't exist — interpreter data is stored viaPortableEntryPoint. The fix adds a conditional that usesPortableEntryPoint::GetInterpreterData(pCode)on WASM instead.src/coreclr/vm/eventtrace.cppMethodAndStartAddressToEECodeInfoPointerresolves aMethodDesc+ optional code address into a pointer suitable for ETW method events. The existing helperGetInterpreterCodeFromInterpreterPrecodeIfPresentis a no-op whenFEATURE_PORTABLE_ENTRYPOINTSis defined (it just returns its input unchanged). The fix adds a fallback: when no explicit native code start address was provided, it queriesMethodDesc::GetInterpreterCode()directly and returns theInterpByteCodeStartpointer. This allows the sampling profiler to correctly map sampled IPs back to interpreted methods on WASM.