Update stepping through ExternalMethodFixup under the Debugger #108942
Conversation
|
Tagging subscribers to this area: @mangod9 |
mikelle-rogers
requested review from
AaronRobinsonMSFT,
noahfalk,
a team and
jkotas
| @@ -1563,13 +1568,7 @@ BOOL RangeSectionStubManager::DoTraceStub(PCODE stubStartAddress, TraceDestinati | |||
| #ifdef DACCESS_COMPILE | |||
| DacNotImpl(); | |||
| #else | |||
| #if defined(TARGET_ARM64) && defined(__APPLE__) | |||
| // On ARM64 Mac, we cannot put a breakpoint inside of ExternalMethodFixupPatchLabel | |||
There was a problem hiding this comment.
Delete all occurrences of ExternalMethodFixupPatchLabel?
| @@ -4123,6 +4198,10 @@ void DebuggerController::TriggerMulticastDelegate(DELEGATEREF pDel, INT32 delega | |||
| { | |||
| _ASSERTE(!"This code should be unreachable. If your controller enables MulticastDelegateHelper events,it should also override this callback to do something useful when the event arrives."); | |||
| } | |||
| void DebuggerController::TriggerExternalMethodFixup(PCODE target) | |||
| { | |||
| _ASSERTE(!"This code should be unreachable. If your controller enables ExternalMethodFixup events,it should also override this callback to do something useful when the event arrives."); | |||
There was a problem hiding this comment.
Nit: the same missing space is in the message above too
| _ASSERTE(!"This code should be unreachable. If your controller enables ExternalMethodFixup events,it should also override this callback to do something useful when the event arrives."); | |
| _ASSERTE(!"This code should be unreachable. If your controller enables ExternalMethodFixup events, it should also override this callback to do something useful when the event arrives."); |
| @@ -3401,6 +3402,10 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl | |||
| pEMFrame->Pop(CURRENT_THREAD); // Pop the ExternalMethodFrame from the frame stack | |||
|
|
|||
| END_PRESERVE_LAST_ERROR; | |||
| if (g_externalMethodFixupTraceActiveCount > 0) | |||
There was a problem hiding this comment.
I'd suggest moving this code up to 3398. The INSTALL/UNINSTALL_xyz macros are creating try/catch regions to handle exceptions. I don't expect the debugger code to throw exceptions in any normal circumstance, but if there were some error (for example memory is corrupted causing an AV or stack space runs out) then its nice to treat those errors the same way as if they had happened anywhere else in the method body. Also the END_PRESERVE_LAST_ERROR macro is ensuring that the thread local GetLastError() value gets restored before this method returns. Its easier to put our debugger call above that point than to audit ensuring that nothing in the debugger call would ever cause it to change.
This PR updates stepping through the ExternalMethodFixup stub, used when calling other modules in a R2R app, under the debugger by introducing a callback rather than setting a breakpoint within the stub. This is needed as a result of the app crashing on MacOS arm64 if the breakpoint is set on the text section of the module.