Skip to content

Move IsDebuggerPresent to minipal, convert to QCall #108837

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

Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Oct 14, 2024

  • Consolidate minipal_is_native_debugger_present and clean up the code across the coreclr, mono, and nativeaot implementations.
  • Convert the few remaining debugger-related methods to QCalls.
  • Remove the associated kernel32 call from the libraries' interop layer.

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 14, 2024
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch from 41174d8 to c7cf37d Compare October 14, 2024 12:03
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 14, 2024
@am11 am11 added area-PAL-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 14, 2024
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch 12 times, most recently from 7c113c7 to 2084bcc Compare October 14, 2024 15:44
@am11 am11 marked this pull request as ready for review October 14, 2024 17:18
@@ -14,6 +14,7 @@ set(CLR_CMAKE_KEEP_NATIVE_SYMBOLS TRUE)
add_executable_clr(corerun
corerun.cpp
dotenv.cpp
${CLR_SRC_NATIVE_DIR}/minipal/is_native_debugger_present.c
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't there some effort to make the minipal and static lib?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes whichever PR is merged first, the other one will need to resolve conflicts as we usually do. :)

@jkotas jkotas requested a review from noahfalk October 15, 2024 07:54
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch from 3198cea to 167799b Compare October 15, 2024 07:54
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch 2 times, most recently from c3e6998 to 4f18d67 Compare October 15, 2024 08:54
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch from 4f18d67 to 669a148 Compare October 15, 2024 12:05
@am11 am11 force-pushed the feature/minipal_is_native_debugger_present branch from 669a148 to e8a3879 Compare October 15, 2024 12:22
@am11
Copy link
Member Author

am11 commented Oct 15, 2024

Wasm builds timed out on Azure DevOps (GitHub keeps showing them as running). Wasm errors are either known or timeout related according to /ba.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

Other than the minor Nit and outstanding question to Jan, I'm signed off. I'll approve once I get his feedback on my question.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 44b7ba3 into dotnet:main Oct 15, 2024
174 of 179 checks passed
@am11 am11 deleted the feature/minipal_is_native_debugger_present branch October 16, 2024 06:40
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants