Skip to content

Fix TarReader_SparseFileTests reflection breaking under Native AOT by replacing with [UnsafeAccessor]#126854

Merged
rzikm merged 2 commits intomainfrom
copilot/fix-tarreader-sparsefiletests-reflection
Apr 14, 2026
Merged

Fix TarReader_SparseFileTests reflection breaking under Native AOT by replacing with [UnsafeAccessor]#126854
rzikm merged 2 commits intomainfrom
copilot/fix-tarreader-sparsefiletests-reflection

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

TarReader_SparseFileTests.WriteSparseEntry used reflection to inject GNU.sparse.realsize into TarEntry._header.ExtendedAttributes after construction (to bypass validation for negative-value test cases). Under Native AOT outerloop runs, the trimmer removes metadata for these internal members, causing NullReferenceException.

Changes

  • Replaced reflection (GetField/GetProperty/GetValue) in WriteSparseEntry with [UnsafeAccessor], which is AOT-safe and statically analyzable by the trimmer.
  • Approach: access the mutable Dictionary<string, string> by calling the public PaxTarEntry.ExtendedAttributes property (which lazily initializes _header._ea) and then unwrapping the ReadOnlyDictionary<string, string> via a generic accessor class targeting the stable m_dictionary field.
  • Added ReadOnlyDictionaryAccessors<TKey, TValue> private nested class (required pattern for [UnsafeAccessor] on a generic target type):
private static class ReadOnlyDictionaryAccessors<TKey, TValue> where TKey : notnull
{
    [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "m_dictionary")]
    public static extern ref IDictionary<TKey, TValue> GetInnerDictionary(ReadOnlyDictionary<TKey, TValue> d);
}

// usage:
var ea = (Dictionary<string, string>)ReadOnlyDictionaryAccessors<string, string>
    .GetInnerDictionary((ReadOnlyDictionary<string, string>)entry.ExtendedAttributes);
ea["GNU.sparse.realsize"] = realSize.ToString();
  • Removed using System.Reflection; added using System.Collections.ObjectModel and using System.Runtime.CompilerServices.
Original prompt

Problem

PR #125283 introduced TarReader_SparseFileTests that use reflection to access internal members, which breaks under Native AOT outerloop runs because the trimmer removes the reflection metadata.

The failing test:

[FAIL] System.Formats.Tar.Tests.TarReader_SparseFileTests.CorruptedSparseMap_InvalidDataException(sparseMapContent: "abc\n0\n256\n", useAsync: False)
System.NullReferenceException : Object reference not set to an instance of an object.
   at System.Formats.Tar.Tests.TarReader_SparseFileTests.WriteSparseEntry(TarWriter writer, String realName, Int64 realSize, Byte[] rawSparseData) in /_/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.SparseFile.Tests.cs:line 42

Root Cause

In src/libraries/System.Formats.Tar/tests/TarReader/TarReader.SparseFile.Tests.cs, the WriteSparseEntry method uses reflection at lines 40-43:

var headerField = typeof(TarEntry).GetField("_header", BindingFlags.NonPublic | BindingFlags.Instance)!;
var header = headerField.GetValue(entry)!;
var eaProp = header.GetType().GetProperty("ExtendedAttributes", BindingFlags.NonPublic | BindingFlags.Instance)!;
var ea = (Dictionary<string, string>)eaProp.GetValue(header)!;
ea["GNU.sparse.realsize"] = realSize.ToString();

Under Native AOT, GetField and GetProperty return null because reflection metadata for these internal members is not preserved. The ! operator then causes a NullReferenceException.

Required Fix

Replace the reflection-based access with [UnsafeAccessor] attribute, which works under Native AOT. The internal types/members that need to be accessed:

  1. TarEntry._header - an internal field of type TarHeader on the abstract class TarEntry (in src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs, line 17: internal TarHeader _header;)

  2. TarHeader.ExtendedAttributes - an internal property on the sealed class TarHeader that returns Dictionary<string, string>. It's defined as:

    internal Dictionary<string, string> ExtendedAttributes => _ea ??= new Dictionary<string, string>();

    The backing field is private Dictionary<string, string>? _ea;

The TarHeader class is internal sealed partial class TarHeader in namespace System.Formats.Tar.

Implementation

In the test file src/libraries/System.Formats.Tar/tests/TarReader/TarReader.SparseFile.Tests.cs:

  1. Add using System.Runtime.CompilerServices; at the top.

  2. Remove using System.Reflection; (should no longer be needed).

  3. Add [UnsafeAccessor] static extern methods to the test class (or as file-scoped static methods). For example:

[UnsafeAccessor(UnsafeAccessorKind.Field, Name = "_header")]
static extern ref TarHeader GetHeader(TarEntry entry);

Note: TarHeader is internal, so the test project needs to have access to it. Check if the test project already has InternalsVisibleTo or uses [assembly: InternalsVisibleTo]. If not, you might need to access the _ea field directly instead of going through the ExtendedAttributes property:

[UnsafeAccessor(UnsafeAccessorKind.Field, Name = "_ea")]
static extern ref Dictionary<string, string>? GetEa(TarHeader header);

However, since UnsafeAccessor with UnsafeAccessorKind.Field returns a ref to the field, and _ea might be null, you'd need to handle the initialization. An alternative approach: since the ExtendedAttributes property initializes _ea via ??=, you could use the _ea field ref but it might be simpler to just set it.

Actually, the simplest approach: The test only needs to add a key to the internal extended attributes dictionary. Since TarHeader is internal, check if the test assembly can see it. Look for InternalsVisibleTo in the product assembly. If the tests can already see TarHeader, then [UnsafeAccessor] only needs to access _header on TarEntry (since _header is internal but the class TarHeader itself is also internal).

Important: [UnsafeAccessor] can access internal types from other assemblies. The field accessor returns a ref, so you can directly work with it. The method should look something like:

[UnsafeAccessor(UnsafeAccessorKind.Field, Name = "_ea")]
static extern ref Dictionary<string, string>? GetEa(TarHeader header);

But since TarHeader is internal to the product assembly and can't be named in the test assembly's source code directly... you need to check if test projects have InternalsVisibleTo.

  1. Replace lines 40-44 in WriteSparseEntry with calls to the [UnsafeAccessor] methods to get the _header field and then the ExtendedAttributes/_ea field, and set ea["GNU.sparse.realsize"] = realSize.ToString();.

Please check whether the test project has access to internal types (via InternalsVisibleTo or similar) and choose the simplest working approach. The key requirement is that the code must work under Native AOT without relying on reflection APIs (`Type....

This pull request was created from Copilot chat.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-formats-tar
See info in area-owners.md if you want to be subscribed.

Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 22:42
Copilot AI changed the title [WIP] Fix TarReader sparse file tests for Native AOT compatibility Fix TarReader_SparseFileTests reflection breaking under Native AOT by replacing with [UnsafeAccessor] Apr 13, 2026
Copilot AI requested a review from MichalStrehovsky April 13, 2026 22:46
@MichalStrehovsky
Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes TarReader_SparseFileTests failing under Native AOT outerloop by removing reflection-based access to internal TAR header state (which is trim-sensitive) and replacing it with [UnsafeAccessor]-based access that is AOT-/trimmer-friendly.

Changes:

  • Replace reflection (GetField/GetProperty/GetValue) in WriteSparseEntry with [UnsafeAccessor].
  • Unwrap ReadOnlyDictionary<TKey,TValue> to its backing IDictionary<TKey,TValue> via a generic ReadOnlyDictionaryAccessors<TKey,TValue> helper to inject GNU.sparse.realsize.
  • Update usings to remove System.Reflection and add required namespaces for ReadOnlyDictionary + UnsafeAccessor.

@MichalStrehovsky
Copy link
Copy Markdown
Member

Copilot came up with a creative replacement that I scratched my head about for a bit. We can ask it to keep using reflection, just rewrite it into typeof(SomeType).GetField(...) or Type.GetType("SomeStringLiteral").GetField(...) (that are still reflection, but at least it can now be statically analyzed). I don't have a preference, leave this up to the maintainers.

Copy link
Copy Markdown
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (if that works, that is), thanks!

@rzikm
Copy link
Copy Markdown
Member

rzikm commented Apr 14, 2026

/azp list

@azure-pipelines
Copy link
Copy Markdown

CI/CD Pipelines for this repository:

@rzikm
Copy link
Copy Markdown
Member

rzikm commented Apr 14, 2026

oh, then nativeAoT was already run

----- start Tue Apr 14 01:53:23 AM UTC 2026 =============== To repro directly: =====================================================
pushd .
chmod +rwx System.Formats.Tar.Tests ^&^& ./System.Formats.Tar.Tests -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -xml testResults.xml 
popd
===========================================================================================================
/root/helix/work/workitem/e /root/helix/work/workitem/e
Running assembly:System.Formats.Tar.Tests, Version=11.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
[SKIP] System.Formats.Tar.Tests.TarEntry_ExtractToFile_Tests_Unix.Extract_SpecialFiles_Async
[SKIP] System.Formats.Tar.Tests.TarEntry_ExtractToFile_Tests_Unix.Extract_SpecialFiles
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.Add_BlockDevice
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.CreateEntryFromFileOwnedByNonExistentUser
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.Add_CharacterDevice
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.Add_Fifo
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.CreateEntryFromFileOwnedByNonExistentGroup
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntry_File_Tests.CreateEntryFromFileOwnedByNonExistentGroupAndUser
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.Add_BlockDevice_Async
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.CreateEntryFromFileOwnedByNonExistentGroup_Async
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.CreateEntryFromFileOwnedByNonExistentUser_Async
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.CreateEntryFromFileOwnedByNonExistentGroupAndUser_Async
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.Add_Fifo_Async
[SKIP] System.Formats.Tar.Tests.TarWriter_WriteEntryAsync_File_Tests.Add_CharacterDevice_Async
Finished System.Formats.Tar.Tests, Version=11.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51

Tests run: 5868, Errors: 0, Failures: 0, Skipped: 14. Time: 3.645058s

Seems like the fix works, thanks again

@rzikm rzikm enabled auto-merge (squash) April 14, 2026 06:44
@rzikm
Copy link
Copy Markdown
Member

rzikm commented Apr 14, 2026

/ba-g test failure is #126867

@rzikm rzikm merged commit 4f21ff0 into main Apr 14, 2026
98 of 118 checks passed
@rzikm rzikm deleted the copilot/fix-tarreader-sparsefiletests-reflection branch April 14, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants