Use metadata APIs in AssemblyChecker - #133622
Open
jkoritzinsky wants to merge 2 commits into
Open
jkoritzinsky wants to merge 2 commits into
jkoritzinsky wants to merge 2 commits into
Conversation
Inspect DebuggableAttribute metadata directly so AssemblyChecker does not need to resolve the inspected assembly's dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
--is-debug can throw on non-managed PEs or managed modules because IsDebug doesn’t check HasMetadata/IsAssembly before reading the assembly definition.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/coreclr/tools/AssemblyChecker/AssemblyInspector.cs — IsDebug calls peReader.GetMetadataReader() and reader.GetAssemblyDefinition() without first… |
What changed in this PR
Updates the AssemblyChecker tool’s debug-assembly detection to inspect DebuggableAttribute directly from PE metadata (via System.Reflection.Metadata) rather than loading the assembly into a MetadataLoadContext, and removes the System.Reflection.MetadataLoadContext package dependency.
Changes:
- Reworked
AssemblyInspector.IsDebugto read assembly-level custom attributes usingPEReader/MetadataReaderand decodeDebuggableAttributearguments directly. - Added explicit validation of supported
DebuggableAttributeconstructor signatures and argument shapes while decoding. - Removed the
System.Reflection.MetadataLoadContextpackage reference from the tool project.
| File | Description |
|---|---|
| src/coreclr/tools/AssemblyChecker/AssemblyInspector.cs | Switches debug detection to metadata-based custom attribute inspection and validates DebuggableAttribute signatures/arguments. |
| src/coreclr/tools/AssemblyChecker/AssemblyChecker.csproj | Removes System.Reflection.MetadataLoadContext package dependency. |
Add checks for metadata and assembly type in AssemblyInspector. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
steveisok
approved these changes
Sep 28, 2026
jkoritzinsky
enabled auto-merge (squash)
September 28, 2026 23:42
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DebuggableAttributedirectly withSystem.Reflection.Metadatainstead ofMetadataLoadContextDebuggableAttributeconstructor signaturesSystem.Reflection.MetadataLoadContextpackage dependencyTesting
./build.sh clr+libs+host./build.sh clr.tools--is-debugagainst Debug and Release assemblies after removing an assembly referenced by an unrelated assembly-level custom attributeFixes #134296
Note
This pull request description was generated by GitHub Copilot.