Skip to content

Commit 29a17a1

Browse files
EgorBoCopilot
EgorBo
and
Copilot
committed
Skip methods where stackalloc would escape the new unsafe block
After wrapping a method body in a fresh `unsafe { ... }` block any `stackalloc` inside has block-scoped safe-to-escape. Assigning that (directly or through a local) to a parameter / out variable requires the parameter to have a wider safe-to-escape, which fails with CS9080 / CS9081 after the wrap. The repo-wide `SkipLocalsInit` policy means `stackalloc` must stay inside an unsafe context, so there is no mechanical rewrite that both removes the `unsafe` modifier and keeps the body compiling for this shape. The analyzer now detects this combination syntactically and does not report on the affected declarations - they keep the `unsafe` modifier and a human can refactor them (extract a helper that takes the buffer, restructure the stackalloc) if desired. Conservative check: collect parameter names of the enclosing member, then skip if the body contains a `stackalloc` AND any simple assignment whose left-hand side is an identifier matching one of those parameter names. False positives only mean a few extra methods keep `unsafe` - safer than emitting code that fails to build. STJ output regenerated via `dotnet format`; STJ builds clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 46fbbed commit 29a17a1

10 files changed

Lines changed: 854 additions & 796 deletions

File tree

‎src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.TryGetProperty.cs‎

Lines changed: 143 additions & 153 deletions
Large diffs are not rendered by default.

‎src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.Unescaping.cs‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -245,44 +245,39 @@ public static bool TryDecodeBase64InPlace(Span<byte> utf8Unescaped, [NotNullWhen
245245
return true;
246246
}
247247

248-
public static bool TryDecodeBase64(ReadOnlySpan<byte> utf8Unescaped, [NotNullWhen(true)] out byte[]? bytes)
248+
public static unsafe bool TryDecodeBase64(ReadOnlySpan<byte> utf8Unescaped, [NotNullWhen(true)] out byte[]? bytes)
249249
{
250-
// SAFETY-TODO: review this unsafe usage.
251-
// Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
252-
unsafe
253-
{
254-
byte[]? pooledArray = null;
255-
256-
Span<byte> byteSpan = utf8Unescaped.Length <= JsonConstants.StackallocByteThreshold ?
257-
stackalloc byte[JsonConstants.StackallocByteThreshold] :
258-
(pooledArray = ArrayPool<byte>.Shared.Rent(utf8Unescaped.Length));
250+
byte[]? pooledArray = null;
259251

260-
OperationStatus status = Base64.DecodeFromUtf8(utf8Unescaped, byteSpan, out int bytesConsumed, out int bytesWritten);
261-
262-
if (status != OperationStatus.Done)
263-
{
264-
bytes = null;
265-
266-
if (pooledArray != null)
267-
{
268-
byteSpan.Clear();
269-
ArrayPool<byte>.Shared.Return(pooledArray);
270-
}
252+
Span<byte> byteSpan = utf8Unescaped.Length <= JsonConstants.StackallocByteThreshold ?
253+
stackalloc byte[JsonConstants.StackallocByteThreshold] :
254+
(pooledArray = ArrayPool<byte>.Shared.Rent(utf8Unescaped.Length));
271255

272-
return false;
273-
}
274-
Debug.Assert(bytesConsumed == utf8Unescaped.Length);
256+
OperationStatus status = Base64.DecodeFromUtf8(utf8Unescaped, byteSpan, out int bytesConsumed, out int bytesWritten);
275257

276-
bytes = byteSpan.Slice(0, bytesWritten).ToArray();
258+
if (status != OperationStatus.Done)
259+
{
260+
bytes = null;
277261

278262
if (pooledArray != null)
279263
{
280264
byteSpan.Clear();
281265
ArrayPool<byte>.Shared.Return(pooledArray);
282266
}
283267

284-
return true;
268+
return false;
285269
}
270+
Debug.Assert(bytesConsumed == utf8Unescaped.Length);
271+
272+
bytes = byteSpan.Slice(0, bytesWritten).ToArray();
273+
274+
if (pooledArray != null)
275+
{
276+
byteSpan.Clear();
277+
ArrayPool<byte>.Shared.Return(pooledArray);
278+
}
279+
280+
return true;
286281
}
287282

288283
public static string TranscodeHelper(ReadOnlySpan<byte> utf8Unescaped)

‎src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs‎

Lines changed: 45 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -176,31 +176,26 @@ public static bool TryGetValue(ReadOnlySpan<byte> segment, bool isEscaped, out D
176176
return false;
177177
}
178178

179-
public static bool TryGetEscapedDateTime(ReadOnlySpan<byte> source, out DateTime value)
179+
public static unsafe bool TryGetEscapedDateTime(ReadOnlySpan<byte> source, out DateTime value)
180180
{
181-
// SAFETY-TODO: review this unsafe usage.
182-
// Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
183-
unsafe
184-
{
185-
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedDateTimeOffsetParseLength);
186-
Span<byte> sourceUnescaped = stackalloc byte[JsonConstants.MaximumEscapedDateTimeOffsetParseLength];
187-
188-
Unescape(source, sourceUnescaped, out int written);
189-
Debug.Assert(written > 0);
181+
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedDateTimeOffsetParseLength);
182+
Span<byte> sourceUnescaped = stackalloc byte[JsonConstants.MaximumEscapedDateTimeOffsetParseLength];
190183

191-
sourceUnescaped = sourceUnescaped.Slice(0, written);
192-
Debug.Assert(!sourceUnescaped.IsEmpty);
184+
Unescape(source, sourceUnescaped, out int written);
185+
Debug.Assert(written > 0);
193186

194-
if (JsonHelpers.IsValidUnescapedDateTimeOffsetParseLength(sourceUnescaped.Length)
195-
&& JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTime tmp))
196-
{
197-
value = tmp;
198-
return true;
199-
}
187+
sourceUnescaped = sourceUnescaped.Slice(0, written);
188+
Debug.Assert(!sourceUnescaped.IsEmpty);
200189

201-
value = default;
202-
return false;
190+
if (JsonHelpers.IsValidUnescapedDateTimeOffsetParseLength(sourceUnescaped.Length)
191+
&& JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTime tmp))
192+
{
193+
value = tmp;
194+
return true;
203195
}
196+
197+
value = default;
198+
return false;
204199
}
205200

206201
public static bool TryGetValue(ReadOnlySpan<byte> segment, bool isEscaped, out DateTimeOffset value)
@@ -229,31 +224,26 @@ public static bool TryGetValue(ReadOnlySpan<byte> segment, bool isEscaped, out D
229224
return false;
230225
}
231226

232-
public static bool TryGetEscapedDateTimeOffset(ReadOnlySpan<byte> source, out DateTimeOffset value)
227+
public static unsafe bool TryGetEscapedDateTimeOffset(ReadOnlySpan<byte> source, out DateTimeOffset value)
233228
{
234-
// SAFETY-TODO: review this unsafe usage.
235-
// Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
236-
unsafe
237-
{
238-
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedDateTimeOffsetParseLength);
239-
Span<byte> sourceUnescaped = stackalloc byte[JsonConstants.MaximumEscapedDateTimeOffsetParseLength];
240-
241-
Unescape(source, sourceUnescaped, out int written);
242-
Debug.Assert(written > 0);
229+
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedDateTimeOffsetParseLength);
230+
Span<byte> sourceUnescaped = stackalloc byte[JsonConstants.MaximumEscapedDateTimeOffsetParseLength];
243231

244-
sourceUnescaped = sourceUnescaped.Slice(0, written);
245-
Debug.Assert(!sourceUnescaped.IsEmpty);
232+
Unescape(source, sourceUnescaped, out int written);
233+
Debug.Assert(written > 0);
246234

247-
if (JsonHelpers.IsValidUnescapedDateTimeOffsetParseLength(sourceUnescaped.Length)
248-
&& JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTimeOffset tmp))
249-
{
250-
value = tmp;
251-
return true;
252-
}
235+
sourceUnescaped = sourceUnescaped.Slice(0, written);
236+
Debug.Assert(!sourceUnescaped.IsEmpty);
253237

254-
value = default;
255-
return false;
238+
if (JsonHelpers.IsValidUnescapedDateTimeOffsetParseLength(sourceUnescaped.Length)
239+
&& JsonHelpers.TryParseAsISO(sourceUnescaped, out DateTimeOffset tmp))
240+
{
241+
value = tmp;
242+
return true;
256243
}
244+
245+
value = default;
246+
return false;
257247
}
258248

259249
public static bool TryGetValue(ReadOnlySpan<byte> segment, bool isEscaped, out Guid value)
@@ -283,31 +273,26 @@ public static bool TryGetValue(ReadOnlySpan<byte> segment, bool isEscaped, out G
283273
return false;
284274
}
285275

286-
public static bool TryGetEscapedGuid(ReadOnlySpan<byte> source, out Guid value)
276+
public static unsafe bool TryGetEscapedGuid(ReadOnlySpan<byte> source, out Guid value)
287277
{
288-
// SAFETY-TODO: review this unsafe usage.
289-
// Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
290-
unsafe
291-
{
292-
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedGuidLength);
278+
Debug.Assert(source.Length <= JsonConstants.MaximumEscapedGuidLength);
293279

294-
Span<byte> utf8Unescaped = stackalloc byte[JsonConstants.MaximumEscapedGuidLength];
295-
Unescape(source, utf8Unescaped, out int written);
296-
Debug.Assert(written > 0);
280+
Span<byte> utf8Unescaped = stackalloc byte[JsonConstants.MaximumEscapedGuidLength];
281+
Unescape(source, utf8Unescaped, out int written);
282+
Debug.Assert(written > 0);
297283

298-
utf8Unescaped = utf8Unescaped.Slice(0, written);
299-
Debug.Assert(!utf8Unescaped.IsEmpty);
284+
utf8Unescaped = utf8Unescaped.Slice(0, written);
285+
Debug.Assert(!utf8Unescaped.IsEmpty);
300286

301-
if (utf8Unescaped.Length == JsonConstants.MaximumFormatGuidLength
302-
&& Utf8Parser.TryParse(utf8Unescaped, out Guid tmp, out _, 'D'))
303-
{
304-
value = tmp;
305-
return true;
306-
}
307-
308-
value = default;
309-
return false;
287+
if (utf8Unescaped.Length == JsonConstants.MaximumFormatGuidLength
288+
&& Utf8Parser.TryParse(utf8Unescaped, out Guid tmp, out _, 'D'))
289+
{
290+
value = tmp;
291+
return true;
310292
}
293+
294+
value = default;
295+
return false;
311296
}
312297

313298
#if NET

‎src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs‎

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -535,104 +535,99 @@ private bool ConsumeLiteralMultiSegment(ReadOnlySpan<byte> literal, JsonTokenTyp
535535
return true;
536536
}
537537

538-
private bool CheckLiteralMultiSegment(ReadOnlySpan<byte> span, ReadOnlySpan<byte> literal, out int consumed)
538+
private unsafe bool CheckLiteralMultiSegment(ReadOnlySpan<byte> span, ReadOnlySpan<byte> literal, out int consumed)
539539
{
540-
// SAFETY-TODO: review this unsafe usage.
541-
// Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
542-
unsafe
543-
{
544-
Debug.Assert(span.Length > 0 && span[0] == literal[0] && literal.Length <= JsonConstants.MaximumLiteralLength);
540+
Debug.Assert(span.Length > 0 && span[0] == literal[0] && literal.Length <= JsonConstants.MaximumLiteralLength);
545541

546-
Span<byte> readSoFar = stackalloc byte[JsonConstants.MaximumLiteralLength];
547-
int written = 0;
542+
Span<byte> readSoFar = stackalloc byte[JsonConstants.MaximumLiteralLength];
543+
int written = 0;
548544

549-
long prevTotalConsumed = _totalConsumed;
550-
SequencePosition copy = _currentPosition;
551-
if (span.Length >= literal.Length || IsLastSpan)
545+
long prevTotalConsumed = _totalConsumed;
546+
SequencePosition copy = _currentPosition;
547+
if (span.Length >= literal.Length || IsLastSpan)
548+
{
549+
_bytePositionInLine += FindMismatch(span, literal);
550+
551+
int amountToWrite = AmountToWrite(span, _bytePositionInLine, readSoFar, written);
552+
span.Slice(0, amountToWrite).CopyTo(readSoFar);
553+
written += amountToWrite;
554+
goto Throw;
555+
}
556+
else
557+
{
558+
if (!literal.StartsWith(span))
552559
{
553560
_bytePositionInLine += FindMismatch(span, literal);
554-
555561
int amountToWrite = AmountToWrite(span, _bytePositionInLine, readSoFar, written);
556562
span.Slice(0, amountToWrite).CopyTo(readSoFar);
557563
written += amountToWrite;
558564
goto Throw;
559565
}
560-
else
561-
{
562-
if (!literal.StartsWith(span))
563-
{
564-
_bytePositionInLine += FindMismatch(span, literal);
565-
int amountToWrite = AmountToWrite(span, _bytePositionInLine, readSoFar, written);
566-
span.Slice(0, amountToWrite).CopyTo(readSoFar);
567-
written += amountToWrite;
568-
goto Throw;
569-
}
570566

571-
ReadOnlySpan<byte> leftToMatch = literal.Slice(span.Length);
567+
ReadOnlySpan<byte> leftToMatch = literal.Slice(span.Length);
572568

573-
SequencePosition startPosition = _currentPosition;
574-
int startConsumed = _consumed;
575-
int alreadyMatched = literal.Length - leftToMatch.Length;
576-
while (true)
569+
SequencePosition startPosition = _currentPosition;
570+
int startConsumed = _consumed;
571+
int alreadyMatched = literal.Length - leftToMatch.Length;
572+
while (true)
573+
{
574+
_totalConsumed += alreadyMatched;
575+
_bytePositionInLine += alreadyMatched;
576+
if (!GetNextSpan())
577577
{
578-
_totalConsumed += alreadyMatched;
579-
_bytePositionInLine += alreadyMatched;
580-
if (!GetNextSpan())
578+
_totalConsumed = prevTotalConsumed;
579+
consumed = default;
580+
_currentPosition = copy;
581+
if (IsLastSpan)
581582
{
582-
_totalConsumed = prevTotalConsumed;
583-
consumed = default;
584-
_currentPosition = copy;
585-
if (IsLastSpan)
586-
{
587-
goto Throw;
588-
}
589-
return false;
583+
goto Throw;
590584
}
585+
return false;
586+
}
591587

592-
int amountToWrite = Math.Min(span.Length, readSoFar.Length - written);
593-
span.Slice(0, amountToWrite).CopyTo(readSoFar.Slice(written));
594-
written += amountToWrite;
588+
int amountToWrite = Math.Min(span.Length, readSoFar.Length - written);
589+
span.Slice(0, amountToWrite).CopyTo(readSoFar.Slice(written));
590+
written += amountToWrite;
595591

596-
span = _buffer;
592+
span = _buffer;
597593

598-
if (span.StartsWith(leftToMatch))
599-
{
600-
HasValueSequence = true;
601-
SequencePosition start = new SequencePosition(startPosition.GetObject(), startPosition.GetInteger() + startConsumed);
602-
SequencePosition end = new SequencePosition(_currentPosition.GetObject(), _currentPosition.GetInteger() + leftToMatch.Length);
603-
ValueSequence = _sequence.Slice(start, end);
604-
consumed = leftToMatch.Length;
605-
return true;
606-
}
607-
608-
if (!leftToMatch.StartsWith(span))
609-
{
610-
_bytePositionInLine += FindMismatch(span, leftToMatch);
594+
if (span.StartsWith(leftToMatch))
595+
{
596+
HasValueSequence = true;
597+
SequencePosition start = new SequencePosition(startPosition.GetObject(), startPosition.GetInteger() + startConsumed);
598+
SequencePosition end = new SequencePosition(_currentPosition.GetObject(), _currentPosition.GetInteger() + leftToMatch.Length);
599+
ValueSequence = _sequence.Slice(start, end);
600+
consumed = leftToMatch.Length;
601+
return true;
602+
}
611603

612-
amountToWrite = AmountToWrite(span, _bytePositionInLine, readSoFar, written);
613-
span.Slice(0, amountToWrite).CopyTo(readSoFar.Slice(written));
614-
written += amountToWrite;
604+
if (!leftToMatch.StartsWith(span))
605+
{
606+
_bytePositionInLine += FindMismatch(span, leftToMatch);
615607

616-
goto Throw;
617-
}
608+
amountToWrite = AmountToWrite(span, _bytePositionInLine, readSoFar, written);
609+
span.Slice(0, amountToWrite).CopyTo(readSoFar.Slice(written));
610+
written += amountToWrite;
618611

619-
leftToMatch = leftToMatch.Slice(span.Length);
620-
alreadyMatched = span.Length;
612+
goto Throw;
621613
}
622-
}
623614

624-
static int AmountToWrite(ReadOnlySpan<byte> span, long bytePositionInLine, ReadOnlySpan<byte> readSoFar, int written)
625-
{
626-
return Math.Min(
627-
readSoFar.Length - written,
628-
Math.Min(span.Length, (int)bytePositionInLine + 1));
615+
leftToMatch = leftToMatch.Slice(span.Length);
616+
alreadyMatched = span.Length;
629617
}
630-
Throw:
631-
_totalConsumed = prevTotalConsumed;
632-
consumed = default;
633-
_currentPosition = copy;
634-
throw GetInvalidLiteralMultiSegment(readSoFar.Slice(0, written).ToArray());
635618
}
619+
620+
static int AmountToWrite(ReadOnlySpan<byte> span, long bytePositionInLine, ReadOnlySpan<byte> readSoFar, int written)
621+
{
622+
return Math.Min(
623+
readSoFar.Length - written,
624+
Math.Min(span.Length, (int)bytePositionInLine + 1));
625+
}
626+
Throw:
627+
_totalConsumed = prevTotalConsumed;
628+
consumed = default;
629+
_currentPosition = copy;
630+
throw GetInvalidLiteralMultiSegment(readSoFar.Slice(0, written).ToArray());
636631
}
637632

638633
private static int FindMismatch(ReadOnlySpan<byte> span, ReadOnlySpan<byte> literal)

0 commit comments

Comments
 (0)