Debugger attributes on methods still left after trimming #1355
Comments
|
For what I was able to see the linker does have the attribute in the cache, but the action for System.Private.CoreLib is CopyUsed. Therefore the custom attribute is being marked anyway due to #1344 which only removes the attributes if the action is Link. |
|
@tlakollo did you try to run it with link action? |
|
I didn't, but I just did. The attribute was removed |
|
@eerhardt could you try if passing the full path to ImmutableCollection assembly to linker with -r fixes the problem? |
|
Here's the full command line: You can see the only things with
Everything else says: |
@tlakollo - from your screenshot it appears you are using CoreCLR: https://github.com/dotnet/runtime/blob/6072e4d3a7a2a1493f514cdf4be75a3d56580e84/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs#L3943 On Mono, there is a different version of this method: https://github.com/dotnet/runtime/blob/2d9dbbb538322a7cc05397f5e2e15d445c7d3ff0/src/mono/netcore/System.Private.CoreLib/src/System/RuntimeType.Mono.cs#L1804. Were you using a Blazor app to test? |
|
I'm using a blazor application. Not sure if its the right one though, I use |
This does not use a wasm app. I've updated the top post of this issue with repro steps that you can use. Let me know if you have any questions. |
|
The attribute is being used by one of the descriptors files, https://github.com/dotnet/runtime/blob/0ed1588a0994494c44fa52d62915ddcc0dced753/src/mono/netcore/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml#L148 in #1332 we introduce descriptor files as a way to override the deletion of attributes. |
|
Nice find! I can send a PR to put those behind the feature switch. BTW - how would someone outside of the linker team go about diagnosing this? Would |
|
I think that's not the intended use for that flag, the dump dependencies flag creates an XML with dependencies for the analyzer. The app might not need the DebuggerStepThrough and therefore not dump it, and still due to ILLink.Descriptors keep it. |
Make sure you put Mono people on the PR - Mono loads these types with Mono in general seems to treat custom attributes differently than CoreCLR - in CoreCLR, we just tend to do a name match from the attribute metadata. Mono instead loads the attribute type, loads all attributes it's checking, and performs an assignability check between the data structures that represent loaded types. In Cecil terms, the difference would be "compare TypeReference.Name and TypeReference.Namespace for the custom attribute with a string" (for the CoreCLR way) and "get TypeDefinition of the custom attribute, call |
I would expect the dependencies to have an edge between the descriptor XML and the attribute type. There will probably be other edges to the attribute type (from all the instantiations), but those are effectively "conditional" on the one from the descriptor. I haven't tried this though.
I don't think we should warn here. The case described in this issue is "accidental", but I think that most cases where the same attribute will be "Removed" as well as present in descriptor are the cases we designed this for - where the SDK removes the attribute, but the app says "I want to keep it anyway". Warning in that case is just pure noise - I (the app developer) explicitly added the type into the descriptor and now I also have to go a suppress the warning for no reason. In theory it "might" be somewhat useful to warn if the removal and "keep it" are found in the same assembly (as embedded resources), but even that can be relatively tricky to get right. I would rather not warn in this case. This is not a functional problem, just a size problem. In general we don't have any diagnostic in the linker to warn developers about things which will increase size. If we ever get there, this might be part of it. |
eerhardt
added a commit
to eerhardt/runtime
that referenced
this issue
Jul 16, 2020
Changed debugger-agent to allow for the Debugger Attributes to not exist. Removed the Debugger Attributes entries from the Descriptors.xml file. Fix dotnet/linker#1355
monojenkins
pushed a commit
to monojenkins/mono
that referenced
this issue
Jul 20, 2020
* Changed debugger-agent to allow for the Debugger Attributes to not exist. * Removed the Debugger Attributes entries from the Descriptors.xml file. Fix dotnet/linker#1355
eerhardt
added a commit
to dotnet/runtime
that referenced
this issue
Jul 20, 2020
* Allow Debugger attributes to be trimmed in Mono. Changed debugger-agent to allow for the Debugger Attributes to not exist. Removed the Debugger Attributes entries from the Descriptors.xml file. Fix dotnet/linker#1355 * Use GENERATE_TRY_GET_CLASS_WITH_CACHE to initialize Debugger attribute pointers.
akoeplinger
pushed a commit
to mono/mono
that referenced
this issue
Jul 20, 2020
* Changed debugger-agent to allow for the Debugger Attributes to not exist. * Removed the Debugger Attributes entries from the Descriptors.xml file. Fix dotnet/linker#1355 Co-authored-by: eerhardt <[email protected]>
Jacksondr5
pushed a commit
to Jacksondr5/runtime
that referenced
this issue
Aug 10, 2020
* Allow Debugger attributes to be trimmed in Mono. Changed debugger-agent to allow for the Debugger Attributes to not exist. Removed the Debugger Attributes entries from the Descriptors.xml file. Fix dotnet/linker#1355 * Use GENERATE_TRY_GET_CLASS_WITH_CACHE to initialize Debugger attribute pointers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
After trimming a Blazor application with the
ILLink.LinkAttributes.xmldefined in dotnet/runtime#39237, I'm still seeing some Debugger attributes that are on methods, for exampleDebuggerStepThroughAttribute:But I have the following in the xml file:
It does appear to remove attributes on a class, like
DebuggerDisplayandDebuggerTypeProxy, so I don't know if it is just attributes on methods, or something else causing the attributes to be preserved.@vitek-karas @tlakollo
Steps to reproduce:
In the runtime repo:
.\build.cmd -s mono.corelib -rc Release -arch Wasm -os Browserto get the changes built locallyCreate a Blazor application:
dotnet new blazorwasmdotnet publish -c Release /bl/bland then get the linker command line from the_RunILLinktask in themsbuild.binlogSystem.Private.CoreLib.dllwith your locally built WASMSystem.Private.CoreLib.dll- it looks likeREPOROOT\artifacts\bin\mono\Browser.Wasm.Release\IL\System.Private.CoreLib.dllThe text was updated successfully, but these errors were encountered: