Skip to content

Commit 81ce4f6

Browse files
committed
Add trim and NativeAOT safety attributes
This gets vs-threading to self-identify as a NativeAOT-ready assembly, and adds the attributes on the specific methods that are *not* safe. Incidentally, all the trim-unsafe methods are strictly for runtime diagnostics rather than critical functionality.
1 parent 02ba9f2 commit 81ce4f6

15 files changed

+328
-261
lines changed

Microsoft.VisualStudio.Threading.slnx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,6 @@
4040
<Project Path="test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Microsoft.VisualStudio.Threading.Analyzers.Tests.csproj" />
4141
<Project Path="test/Microsoft.VisualStudio.Threading.Tests.Win7RegistryWatcher/Microsoft.VisualStudio.Threading.Tests.Win7RegistryWatcher.csproj" />
4242
<Project Path="test/Microsoft.VisualStudio.Threading.Tests/Microsoft.VisualStudio.Threading.Tests.csproj" />
43+
<Project Path="test/NativeAOTCompatibility/NativeAOTCompatibility.csproj" />
4344
</Folder>
4445
</Solution>

azure-pipelines/build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ jobs:
197197
Is1ESPT: ${{ parameters.Is1ESPT }}
198198
RunTests: ${{ parameters.RunTests }}
199199
IsOptProf: ${{ parameters.IsOptProf }}
200+
osRID: win
200201

201202
- ${{ if and(parameters.EnableDotNetFormatCheck, not(parameters.EnableLinuxBuild)) }}:
202203
- script: dotnet format --verify-no-changes
@@ -241,6 +242,7 @@ jobs:
241242
parameters:
242243
Is1ESPT: ${{ parameters.Is1ESPT }}
243244
RunTests: ${{ parameters.RunTests }}
245+
osRID: linux
244246
- ${{ if parameters.EnableDotNetFormatCheck }}:
245247
- script: dotnet format --verify-no-changes
246248
displayName: 💅 Verify formatted code
@@ -276,6 +278,7 @@ jobs:
276278
parameters:
277279
Is1ESPT: ${{ parameters.Is1ESPT }}
278280
RunTests: ${{ parameters.RunTests }}
281+
osRID: osx
279282

280283
- job: WrapUp
281284
dependsOn:

azure-pipelines/dotnet.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ parameters:
55
default: false
66
- name: Is1ESPT
77
type: boolean
8+
- name: osRID
9+
type: string
810

911
steps:
1012

@@ -16,6 +18,15 @@ steps:
1618
displayName: 🧪 dotnet test
1719
condition: and(succeeded(), ${{ parameters.RunTests }})
1820

21+
- powershell: |
22+
dotnet publish -f net9.0 -c Release -r ${{ parameters.osRID }}-x64 -warnaserror
23+
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
24+
if (!($IsMacOS -or $IsLinux)) {
25+
dotnet publish -f net9.0-windows -c Release -r ${{ parameters.osRID }}-x64 -warnaserror
26+
}
27+
displayName: 🧪 NativeAOT test
28+
workingDirectory: test/NativeAOTCompatibility
29+
1930
- ${{ if parameters.IsOptProf }}:
2031
- script: dotnet pack src\VSInsertionMetadata -c $(BuildConfiguration) -warnaserror /bl:"$(Build.ArtifactStagingDirectory)/build_logs/VSInsertion-Pack.binlog"
2132
displayName: 🔧 dotnet pack VSInsertionMetadata

src/Microsoft.VisualStudio.Threading/AsyncReaderWriterLock+HangReportContributor.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Globalization;
78
using System.Linq;
89
using System.Text;
910
using System.Threading;
10-
using System.Threading.Tasks;
1111
using System.Xml.Linq;
1212

1313
namespace Microsoft.VisualStudio.Threading
@@ -46,6 +46,7 @@ protected internal virtual SynchronizationContext NoMessagePumpSynchronizationCo
4646
/// Contributes data for a hang report.
4747
/// </summary>
4848
/// <returns>The hang report contribution. Null values should be ignored.</returns>
49+
[RequiresUnreferencedCode(Reasons.DiagnosticAnalysisOnly)]
4950
HangReportContribution IHangReportContributor.GetHangReport()
5051
{
5152
return this.GetHangReport();
@@ -55,7 +56,8 @@ HangReportContribution IHangReportContributor.GetHangReport()
5556
/// Contributes data for a hang report.
5657
/// </summary>
5758
/// <returns>The hang report contribution. Null values should be ignored.</returns>
58-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity"), System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity")]
59+
[SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity"), SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity")]
60+
[RequiresUnreferencedCode(Reasons.DiagnosticAnalysisOnly)]
5961
protected virtual HangReportContribution GetHangReport()
6062
{
6163
using (this.NoMessagePumpSynchronizationContext.Apply())
@@ -125,6 +127,7 @@ private static XDocument CreateDgml(out XElement nodes, out XElement links)
125127
/// <summary>
126128
/// Appends details of a given collection of awaiters to the hang report.
127129
/// </summary>
130+
[RequiresUnreferencedCode(Reasons.DiagnosticAnalysisOnly)]
128131
private static XElement CreateAwaiterNode(Awaiter awaiter)
129132
{
130133
Requires.NotNull(awaiter, nameof(awaiter));
@@ -211,9 +214,13 @@ public IEnumerable<string> Categories
211214
{
212215
get
213216
{
217+
#if NET
218+
foreach (AwaiterCollection value in Enum.GetValues<AwaiterCollection>())
219+
#else
214220
#pragma warning disable CS8605 // Unboxing a possibly null value.
215221
foreach (AwaiterCollection value in Enum.GetValues(typeof(AwaiterCollection)))
216222
#pragma warning restore CS8605 // Unboxing a possibly null value.
223+
#endif
217224
{
218225
if (this.Membership.HasFlag(value))
219226
{
Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
5-
using System.Collections.Generic;
6-
using System.Linq;
7-
using System.Text;
8-
using System.Threading.Tasks;
4+
using System.Diagnostics.CodeAnalysis;
95

106
namespace Microsoft.VisualStudio.Threading;
117

@@ -18,5 +14,6 @@ public interface IHangReportContributor
1814
/// Contributes data for a hang report.
1915
/// </summary>
2016
/// <returns>The hang report contribution. Null values should be ignored.</returns>
17+
[RequiresUnreferencedCode(Reasons.DiagnosticAnalysisOnly)]
2118
HangReportContribution GetHangReport();
2219
}
Lines changed: 0 additions & 241 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
5-
using System.Collections;
64
using System.Collections.Generic;
7-
using System.Globalization;
8-
using System.Linq;
9-
using System.Reflection;
10-
using System.Runtime.CompilerServices;
11-
using System.Threading.Tasks;
125

136
namespace Microsoft.VisualStudio.Threading;
147

@@ -17,15 +10,6 @@ namespace Microsoft.VisualStudio.Threading;
1710
/// </summary>
1811
internal static class InternalUtilities
1912
{
20-
/// <summary>
21-
/// The substring that should be inserted before each async return stack frame.
22-
/// </summary>
23-
/// <remarks>
24-
/// When printing synchronous callstacks, .NET begins each frame with " at ".
25-
/// When printing async return stack, we use this to indicate continuations.
26-
/// </remarks>
27-
private const string AsyncReturnStackPrefix = " -> ";
28-
2913
/// <summary>
3014
/// Removes an element from the middle of a queue without disrupting the other elements.
3115
/// </summary>
@@ -60,229 +44,4 @@ internal static bool RemoveMidQueue<T>(this Queue<T> queue, T valueToRemove)
6044

6145
return found;
6246
}
63-
64-
/// <summary>
65-
/// Walk the continuation objects inside "async state machines" to generate the return call stack.
66-
/// FOR DIAGNOSTIC PURPOSES ONLY.
67-
/// </summary>
68-
/// <param name="continuationDelegate">The delegate that represents the head of an async continuation chain.</param>
69-
internal static IEnumerable<string> GetAsyncReturnStackFrames(this Delegate continuationDelegate)
70-
{
71-
IAsyncStateMachine? stateMachine = FindAsyncStateMachine(continuationDelegate);
72-
if (stateMachine is null)
73-
{
74-
// Did not find the async state machine, so returns the method name as top frame and stop walking.
75-
yield return GetDelegateLabel(continuationDelegate);
76-
yield break;
77-
}
78-
79-
do
80-
{
81-
object? state = GetStateMachineFieldValueOnSuffix(stateMachine, "__state");
82-
yield return string.Format(
83-
CultureInfo.CurrentCulture,
84-
"{2}{0} (state: {1}, address: 0x{3:X8})",
85-
stateMachine.GetType().FullName,
86-
state,
87-
AsyncReturnStackPrefix,
88-
(long)GetAddress(stateMachine)); // the long cast allows hex formatting
89-
90-
Delegate[]? continuationDelegates = FindContinuationDelegates(stateMachine).ToArray();
91-
if (continuationDelegates.Length == 0)
92-
{
93-
break;
94-
}
95-
96-
// Consider: It's possible but uncommon scenario to have multiple "async methods" being awaiting for one "async method".
97-
// Here we just choose the first awaiting "async method" as that should be good enough for postmortem.
98-
// In future we might want to revisit this to cover the other awaiting "async methods".
99-
stateMachine = continuationDelegates.Select((d) => FindAsyncStateMachine(d))
100-
.FirstOrDefault((s) => s is object);
101-
if (stateMachine is null)
102-
{
103-
yield return GetDelegateLabel(continuationDelegates.First());
104-
}
105-
}
106-
while (stateMachine is object);
107-
}
108-
109-
/// <summary>
110-
/// A helper method to get the label of the given delegate.
111-
/// </summary>
112-
private static string GetDelegateLabel(Delegate invokeDelegate)
113-
{
114-
Requires.NotNull(invokeDelegate, nameof(invokeDelegate));
115-
116-
MethodInfo? method = invokeDelegate.GetMethodInfo();
117-
if (invokeDelegate.Target is object)
118-
{
119-
string instanceType = string.Empty;
120-
if (!(method?.DeclaringType?.Equals(invokeDelegate.Target.GetType()) ?? false))
121-
{
122-
instanceType = " (" + invokeDelegate.Target.GetType().FullName + ")";
123-
}
124-
125-
return string.Format(
126-
CultureInfo.CurrentCulture,
127-
"{3}{0}.{1}{2} (target address: 0x{4:X" + (IntPtr.Size * 2) + "})",
128-
method?.DeclaringType?.FullName,
129-
method?.Name,
130-
instanceType,
131-
AsyncReturnStackPrefix,
132-
GetAddress(invokeDelegate.Target).ToInt64()); // the cast allows hex formatting
133-
}
134-
135-
return string.Format(
136-
CultureInfo.CurrentCulture,
137-
"{2}{0}.{1}",
138-
method?.DeclaringType?.FullName,
139-
method?.Name,
140-
AsyncReturnStackPrefix);
141-
}
142-
143-
/// <summary>
144-
/// Gets the memory address of a given object.
145-
/// </summary>
146-
/// <param name="value">The object to get the address for.</param>
147-
/// <returns>The memory address.</returns>
148-
/// <remarks>
149-
/// This method works when GCHandle will refuse because the type of object is a non-blittable type.
150-
/// However, this method provides no guarantees that the address will remain valid for the caller,
151-
/// so it is only useful for diagnostics and when we don't expect addresses to be changing much any more.
152-
/// </remarks>
153-
private static unsafe IntPtr GetAddress(object value) => new IntPtr(Unsafe.AsPointer(ref value));
154-
155-
/// <summary>
156-
/// A helper method to find the async state machine from the given delegate.
157-
/// </summary>
158-
private static IAsyncStateMachine? FindAsyncStateMachine(Delegate invokeDelegate)
159-
{
160-
Requires.NotNull(invokeDelegate, nameof(invokeDelegate));
161-
162-
if (invokeDelegate.Target is object)
163-
{
164-
// Some delegates are wrapped with a ContinuationWrapper object. We have to unwrap that in those cases.
165-
// In testing, this m_continuation field jump is only required when the debugger is attached -- weird.
166-
// I suspect however that it's a natural behavior of the async state machine (when there are >1 continuations perhaps).
167-
// So we check for the case in all cases.
168-
if (GetFieldValue(invokeDelegate.Target, "m_continuation") is Action continuation)
169-
{
170-
invokeDelegate = continuation;
171-
if (invokeDelegate.Target is null)
172-
{
173-
return null;
174-
}
175-
}
176-
177-
var stateMachine = GetFieldValue(invokeDelegate.Target, "m_stateMachine") as IAsyncStateMachine;
178-
return stateMachine;
179-
}
180-
181-
return null;
182-
}
183-
184-
/// <summary>
185-
/// This is the core to find the continuation delegate(s) inside the given async state machine.
186-
/// The chain of objects is like this: async state machine -> async method builder -> task -> continuation object -> action.
187-
/// </summary>
188-
/// <remarks>
189-
/// There are 3 types of "async method builder": AsyncVoidMethodBuilder, AsyncTaskMethodBuilder, AsyncTaskMethodBuilder&lt;T&gt;.
190-
/// We don't cover AsyncVoidMethodBuilder as it is used rarely and it can't be awaited either;
191-
/// AsyncTaskMethodBuilder is a wrapper on top of AsyncTaskMethodBuilder&lt;VoidTaskResult&gt;.
192-
/// </remarks>
193-
private static IEnumerable<Delegate> FindContinuationDelegates(IAsyncStateMachine stateMachine)
194-
{
195-
Requires.NotNull(stateMachine, nameof(stateMachine));
196-
197-
object? builder = GetStateMachineFieldValueOnSuffix(stateMachine, "__builder");
198-
if (builder is null)
199-
{
200-
yield break;
201-
}
202-
203-
object? task = GetFieldValue(builder, "m_task");
204-
if (task is null)
205-
{
206-
// Probably this builder is an instance of "AsyncTaskMethodBuilder", so we need to get its inner "AsyncTaskMethodBuilder<VoidTaskResult>"
207-
builder = GetFieldValue(builder, "m_builder");
208-
if (builder is object)
209-
{
210-
task = GetFieldValue(builder, "m_task");
211-
}
212-
}
213-
214-
if (task is null)
215-
{
216-
yield break;
217-
}
218-
219-
// "task" might be an instance of the type deriving from "Task", but "m_continuationObject" is a private field in "Task",
220-
// so we need to use "typeof(Task)" to access "m_continuationObject".
221-
FieldInfo? continuationField = typeof(Task).GetTypeInfo().GetDeclaredField("m_continuationObject");
222-
if (continuationField is null)
223-
{
224-
yield break;
225-
}
226-
227-
object? continuationObject = continuationField.GetValue(task);
228-
if (continuationObject is null)
229-
{
230-
yield break;
231-
}
232-
233-
if (continuationObject is IEnumerable items)
234-
{
235-
foreach (object? item in items)
236-
{
237-
Delegate? action = item as Delegate ?? GetFieldValue(item!, "m_action") as Delegate;
238-
if (action is object)
239-
{
240-
yield return action;
241-
}
242-
}
243-
}
244-
else
245-
{
246-
Delegate? action = continuationObject as Delegate ?? GetFieldValue(continuationObject, "m_action") as Delegate;
247-
if (action is object)
248-
{
249-
yield return action;
250-
}
251-
}
252-
}
253-
254-
/// <summary>
255-
/// A helper method to get field's value given the object and the field name.
256-
/// </summary>
257-
private static object? GetFieldValue(object obj, string fieldName)
258-
{
259-
Requires.NotNull(obj, nameof(obj));
260-
Requires.NotNullOrEmpty(fieldName, nameof(fieldName));
261-
262-
FieldInfo? field = obj.GetType().GetTypeInfo().GetDeclaredField(fieldName);
263-
if (field is object)
264-
{
265-
return field.GetValue(obj);
266-
}
267-
268-
return null;
269-
}
270-
271-
/// <summary>
272-
/// The field names of "async state machine" are not fixed; the workaround is to find the field based on the suffix.
273-
/// </summary>
274-
private static object? GetStateMachineFieldValueOnSuffix(IAsyncStateMachine stateMachine, string suffix)
275-
{
276-
Requires.NotNull(stateMachine, nameof(stateMachine));
277-
Requires.NotNullOrEmpty(suffix, nameof(suffix));
278-
279-
IEnumerable<FieldInfo>? fields = stateMachine.GetType().GetTypeInfo().DeclaredFields;
280-
FieldInfo? field = fields.FirstOrDefault((f) => f.Name.EndsWith(suffix, StringComparison.Ordinal));
281-
if (field is object)
282-
{
283-
return field.GetValue(stateMachine);
284-
}
285-
286-
return null;
287-
}
28847
}

0 commit comments

Comments
 (0)