Skip to content

feat: Accept Android assembly store v4 versions - #5574

Open
jamescrosswell wants to merge 5 commits into
version7from
feat/assemblystore-v4
Open

jamescrosswell wants to merge 5 commits into
version7from
feat/assemblystore-v4

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

First step of porting the upstream dotnet/android AssemblyStore reader changes (up to f1aecf9) so we can read .NET 11 (CoreCLR) v4 assembly stores.

Part of #5454

Summary

  • StoreReader now accepts 0x_0000004 store versions for all four ABIs.
  • v4 adds an 8-byte content_id to the header, so it's read when the format number is >= 4 and Header.NativeSize is derived from the version rather than being a constant. Accepting the version alone isn't enough - the index would be read 8 bytes early and Prepare() would fail.
  • New StoreReaderTests build minimal in-memory v3/v4 stores (32- and 64-bit) and check they parse, and that v2/v5 are rejected.

Notes

🤖 Generated with Claude Code

…eader

v4 stores (CoreCLR, .NET 11) add a content_id field to the header, so the
index starts 8 bytes later. Header.NativeSize is now derived from the format
number rather than being a constant.

Refs #5454

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (version7@21f45f0). Learn more about missing BASE report.

Additional details and impacted files
@@             Coverage Diff             @@
##             version7    #5574   +/-   ##
===========================================
  Coverage            ?   74.74%           
===========================================
  Files               ?      515           
  Lines               ?    18896           
  Branches            ?     3686           
===========================================
  Hits                ?    14123           
  Misses              ?     3893           
  Partials            ?      880           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell
jamescrosswell marked this pull request as ready for review September 15, 2026 03:48
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Sep 15, 2026
@jamescrosswell
jamescrosswell added this pull request to stack #5581 September 15, 2026 03:49
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
}

// Index
if (is64Bit)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The test helper CreateStore unconditionally writes an ignore byte, but StoreReader.Prepare() only reads it on .NET 10.0_OR_GREATER, causing a stream misalignment on older frameworks.
Severity: LOW

Suggested Fix

The test helper CreateStore should be updated to conditionally write the ignore byte, matching the logic in the production StoreReader.Prepare() method. This can be achieved by using the same #if NET10_0_OR_GREATER preprocessor directive in the test code to ensure the test data format is consistent with what the reader expects on all target frameworks.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: test/Sentry.Android.AssemblyReader.Tests/StoreReaderTests.cs#L60

Potential issue: The test helper method `CreateStore` unconditionally writes an `ignore`
byte when creating a test store. However, the `StoreReader.Prepare()` method, which
reads this store, only consumes this byte when compiled for `.NET 10.0_OR_GREATER` due
to a preprocessor directive. On older target frameworks, this byte is not read, causing
the reader's stream position to be misaligned by one byte. This misalignment will lead
to incorrect data being read for subsequent assembly descriptors, causing tests to fail
or produce garbage data on pre-.NET 10.0 targets. This is a test-only issue but prevents
correct validation across all supported frameworks.

Also affects:

  • src/Sentry.Android.AssemblyReader/V2/StoreReader.cs:163~167

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We only target .NET 10 and later for Android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

public API Additions/modifications to, or removals from, the public API surface area. risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants