Skip to content
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

Update the StubManager for better debugger stepping. #65947

Merged
merged 13 commits into from
Mar 4, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Feb 28, 2022

The StubManager is now aware of InstantiatingMethods in IL Stub and StubLink scenarios.

Created the concept of a "StepThroughStub". This handles cases where the stub should be treated as an implementation detail and forward to the appropriate target.

Updates to various logging statements.

Fixes #63199
Fixes #38777

/cc @noahfalk @jkotas @jkoritzinsky @hoyosjs

  and StubLink scenarios
Created the concept of a "StepThroughStub". This handles cases where
  the stub should be treated like as an implementation details and
  simply forward to the appropriate target.
Updates to various logging mechanisms.
@ghost
Copy link

ghost commented Feb 28, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

The StubManager is now aware of InstantiatingMethods in IL Stub and StubLink scenarios.

Created the concept of a "StepThroughStub". This handles cases where the stub should be treated like as an implementation detail and forward to the appropriate target.

Updates to various logging statements.

Fixes #63199
Fixes #38777

/cc @noahfalk @jkotas @jkoritzinsky @hoyosjs

Author: AaronRobinsonMSFT
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: 7.0.0

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Update the StubManager for better debugging stepping. Update the StubManager for better debugger stepping. Feb 28, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review February 28, 2022 19:49
Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Thanks a bunch for cleaning this all up 👍
I had a few suggestions and I am curious to see after with the enum change Jan was suggesting but I think it looks pretty good regardless.

src/coreclr/debug/ee/amd64/amd64walker.cpp Show resolved Hide resolved
src/coreclr/debug/ee/controller.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.h Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Show resolved Hide resolved
@noahfalk
Copy link
Member

noahfalk commented Mar 1, 2022

cc @mikem8361 @dotnet/dotnet-diag

src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.h Outdated Show resolved Hide resolved
src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved
src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved
src/coreclr/vm/stublink.h Outdated Show resolved Hide resolved
@AaronRobinsonMSFT
Copy link
Member Author

The ARM64 issue is related to writing to executable memory. I know the fix, but I'd like to make it make sense with the existing Stub and StubLinker API.

For others, this issue looks like the following from the macOS's Console. The failing instruction is attempting to write back a pointer into the Stub and upon stepping over that instruction the OS tears down the corerun process.

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGBUS)
Exception Codes:       KERN_PROTECTION_FAILURE at 0x00000002800940c0
Exception Codes:       0x0000000000000002, 0x00000002800940c0
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace SIGNAL, Code 10 Bus error: 10
Terminating Process:   exc handler [34828]

VM Region Info: 0x2800940c0 is in 0x280094000-0x280098000;  bytes after start: 192  bytes before end: 16191
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      VM_ALLOCATE                 280070000-280094000    [  144K] rw-/rwx SM=ZER  
--->  VM_ALLOCATE                 280094000-280098000    [   16K] rwx/rwx SM=ZER  
      VM_ALLOCATE                 280098000-2800a0000    [   32K] ---/rwx SM=ZER  

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libcoreclr.dylib              	       0x1047548ac Stub::SetInstantiatedMethodDesc(MethodDesc*) + 72 (stublink.h:569)
1   libcoreclr.dylib              	       0x104754888 Stub::SetInstantiatedMethodDesc(MethodDesc*) + 36 (stublink.h:568)
2   libcoreclr.dylib              	       0x104754648 MakeUnboxingStubWorker(MethodDesc*) + 700 (prestub.cpp:1682)

/cc @dotnet/interop-contrib as an FYI.

@AaronRobinsonMSFT
Copy link
Member Author

Failures are #66100

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM!

src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved
src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved
@AaronRobinsonMSFT
Copy link
Member Author

Failures are #66143

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit c8eeba7 into dotnet:main Mar 4, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the step_into_stubs branch March 4, 2022 01:10
@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants