Add necessary trimmer files for not trimming COM types. #1
Conversation
|
We specifically want a ILLink.Descriptors.LibraryBuild.xml file - those are only passed during library build and don't make it into the SDK. They prevent trimming during library build, but won't root anything at the time of user's publish because they don't leave the repo. ILLink.Descriptors.Windows.xml would also get embedded into the DLL and root things when the user publishes as well. We don't want that. COM is turned off when trimming and enabling COM with trimming is 100% unsupported and we don't do anything to make it better. We also don't want to suppress the warning here - the method should be marked RequiresUnreferencedCode and the annotation propagated to any public API entrypoints. I'm not sure if RequiresUnreferencedCode will suppress the warning - I think it will. If it doesn't suppress, keep the suppression, but also add the RequiresUnreferencedCode. Alternatively, this could be rewritten to "manual COM". Then all the trimming concerns go away. |
| @@ -42,6 +42,7 @@ private void Initialize(SafeWaitHandle hEvent) | |||
| // TODO: Instantiate all the locks (critical sections) | |||
| } | |||
|
|
|||
| [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2050", Justification = "Leave me alone")] | |||
There was a problem hiding this comment.
While I understand the frustration... maybe we could come up with a slightly better justification 😉
There was a problem hiding this comment.
Indeed. My first round had a slightly more colorful justification. This PR is meant as merely a way to move forward and does need to be updated.
There was a problem hiding this comment.
Oh - you had even better one? Now I'm curious 😄
Anyway - it was just a jab, I'm sure we would never put this into the real source base.
| @@ -42,6 +42,7 @@ private void Initialize(SafeWaitHandle hEvent) | |||
| // TODO: Instantiate all the locks (critical sections) | |||
| } | |||
|
|
|||
| [System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("Trimming", "IL2050", Justification = "Leave me alone")] | |||
There was a problem hiding this comment.
As for the code itself - if the PInvoke is indeed OK (meaning no COM will be triggered at runtime - I don't know), the we typically do one other thing:
- Suppressing at the method level is very coarse - unfortunately it's the only currently available way (since attributes can't be applied at IL level), and thus suppressing on large methods is problematic for future code changes (the suppression could hide a problem introduced later on)
- For this we typically create a local function which does the one "dangerous" thing and suppress on it.
- It is sort of "tooling's inability forcing the code to be structured in a specific way" which is unfortunate, but so far it wasn't that bad.
/cc @jkurdek
It definitely should - if that's not the case I would like to take a look as it's likely a bug. Otherwise +1 on everything @MichalStrehovsky said. |
E.g., Update LSRA "Allocating Registers" table description. Dump nodes added during resolution, e.g.: ``` BB29 bottom (BB08->BB08): move V25 from STK to rdi (Critical) N001 ( 1, 1) [001174] ----------z t1174 = LCL_VAR int V25 cse4 rdi REG rdi ``` Dump more data in the LSRA block sequence data: ``` -BB03( 16 ) -BB04( 4 ) +BB03 ( 16 ) critical-in critical-out +BB04 ( 4 ) critical-out ``` When dumping various flow bitvectors, annotate the bitvectors better: ``` -BB25 in gen out -0000000000000000 -0000000000000003 CSE #1.c -0000000000000003 CSE #1.c +BB25 + in: 0000000000000000 +gen: 0000000000000003 CSE #1.c +out: 0000000000000003 CSE #1.c ``` Dump hoisting bitvectors using the sorting number: ``` - USEDEF (5)={V04 V00 V01 V02 V03} + USEDEF (5)={V00 V01 V02 V03 V04} ``` Also, fix various typos and formatting.
|
Thanks for the help and feedback everyone (especially @AaronRobinsonMSFT), submitted a new PR which incorporates this: dotnet#72051. |
* Fix the MacOS remote unwinder for VS4Mac The wrong module was being passed to the remote unwinder because the load bias for shared modules was being calculated incorrectly. Issue: dotnet#63309 * Fix native frame unwind in syscall on arm64 for VS4Mac crash report From PR in main: dotnet#63598 Add arm64 version of StepWithCompactNoEncoding for syscall leaf node wrappers that have compact encoding of 0. Fix ReadCompactEncodingRegister so it actually decrements the addr. Change StepWithCompactEncodingArm64 to match what MacOS libunwind does for framed and frameless stepping. arm64 can have frames with the same SP (but different IPs). Increment SP for this condition so createdump's unwind loop doesn't break out on the "SP not increasing" check and the frames are added to the thread frame list in the correct order. Add getting the unwind info for tail called functions like this: __ZL14PROCEndProcessPvji: 36630: f6 57 bd a9 stp x22, x21, [sp, #-48]! 36634: f4 4f 01 a9 stp x20, x19, [sp, dotnet#16] 36638: fd 7b 02 a9 stp x29, x30, [sp, dotnet#32] 3663c: fd 83 00 91 add x29, sp, dotnet#32 ... 367ac: e9 01 80 52 mov w9, dotnet#15 367b0: 7f 3e 02 71 cmp w19, dotnet#143 367b4: 20 01 88 1a csel w0, w9, w8, eq 367b8: 2e 00 00 94 bl _PROCAbort _TerminateProcess: -> 367bc: 22 00 80 52 mov w2, #1 367c0: 9c ff ff 17 b __ZL14PROCEndProcessPvji The IP (367bc) returns the (incorrect) frameless encoding with nothing on the stack (uses an incorrect LR to unwind). To fix this get the unwind info for PC -1 which points to PROCEndProcess with the correct unwind info. This matches how lldb unwinds this frame. Always address module segment to IP lookup list instead of checking the module regions. Strip pointer authentication bits on PC/LR.
* WIP: add gRPC tests * Fix AOT and trimming * WIP * Implement IncludeNetworkSecurityConfig * Use IncludeNetworkSecurityConfig * Fix gRPC test * Avoid git checkout * Remove unnecessary code * WIP: start working on CI configuration * Remove WinHttpHandler * Fix problem with SSL * Change server host * Setup CI (#1) * Get Docker container building & exported via test build * Changes * Add missing pfx certificate * changes * cleanup Co-authored-by: Simon Rozsival <[email protected]> * Use tls * Update yml * Revert changes to the mono Android sample app * Bump android image version * Bump image version * Enable TLS * Remove hardcoded package versions * Update package versions * Update package versions * Rename pipeline * Move interop tests website dependencies versions to Versions.props * Add cred scan supression for the interop test server private key * Fix licenses * Remove dependencies * Fix path to Versions.props * Remove unnecessary dependency version * Fix building docker image * Change pfx password Co-authored-by: Jo Shields <[email protected]>
* JIT: Introduce `LclVarDsc::lvIsMultiRegDest`
With recent work to expand returned promoted locals into `FIELD_LIST`
the only "whole references" of promoted locals we should see is when
stored from a multi-reg node. This is the only knowledge the backend
should need for correctness purposes, so introduce a bit to track this
property, and switch the backend to check this instead.
The existing `lvIsMultiRegRet` is essentially this + whether the local
is returned. We should be able to remove this, but it is currently used
for some heuristics in old promotion, so keep it around for now.
* JIT: Add some more constant folding in lowering
Add folding for shifts and certain binops that are now getting produced
late due to returned `FIELD_LIST` nodes.
win-arm64 example:
```csharp
[MethodImpl(MethodImplOptions.NoInlining)]
static ValueTask<byte> Foo()
{
return new ValueTask<byte>(123);
}
```
```diff
G_M17084_IG02: ;; offset=0x0008
mov x0, xzr
- mov w1, #1
- mov w2, wzr
- mov w3, dotnet#123
- orr w2, w2, w3, LSL dotnet#16
- orr w1, w2, w1, LSL dotnet#24
- ;; size=24 bbWeight=1 PerfScore 4.00
+ mov w1, #0x17B0000
+ ;; size=8 bbWeight=1 PerfScore 1.00
```
* Feedback
…otnet#114227) Presence of `.cctor` in `Thread` can cause circular dependency if Lock needs to block while Thread .cctor has not run yet. 1. Lock needs to wait on a WaitHandle 2. WaitHandle needs Thread.CurrentThread 3. if Thread's .cctor has not run yet, it needs to run. (it is unusual for this to be the first use of Thread, but the activation pattern in dotnet#113949 made it possible) 4. .cctor needs to take a Lock, so we go to `#1` Fixes: dotnet#113949
@roji The issue was that the trimmer was removing methods on the COM interfaces. It had originally looked like type confusion due to COM tear-offs being used, but I verified this wasn't the issue, rather the trimmer was augmenting COM interfaces and breaking vtable layouts.