morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6423] [MERGED] fix(flutter): honor ELF note alignment when reading build IDs #7350

New issue

Closed

opened 2026-11-01 21:47:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:15 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6423
Author: @Plastic-time
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: codex/flutter-note-alignment

📝 Commits (3)

18e7b19 fix(flutter): honor ELF note alignment when reading build IDs
4398a07 test(flutter): key shared note ranges by effective alignment
0778695 Merge remote-tracking branch 'upstream/main' into codex/maintain-6423-20261023

📊 Changes

4 files changed (+129 additions, -18 deletions)

View changed files

➕ src/domain/native/elfNoteLayout.ts (+16 -0)
📝 src/flutter/ElfBuildId.ts (+20 -6)
📝 src/native/elf/ElfNoteParser.ts (+9 -12)
📝 tests/boundary/filesystem/flutterElfNotes.test.ts (+84 -0)

📄 Description

Summary

Flutter engine build-ID inspection assumes four-byte ELF note padding. With two 20-byte GNU build-ID notes in an eight-byte-aligned PT_NOTE range, it retains the first ID, starts the next header inside padding, and reports "ELF note descriptor exceeds its declared note range" instead of retaining both candidates.

Change and scope

Read p_align/sh_addralign alongside each declared note range and retain its effective alignment in the deduplication key. Share the existing native note parser's alignment and relative-offset calculation through a small domain owner, removing the duplicate arithmetic.

Ordinary read-only format correction; additional maintainer scope approval is not applicable. No issue is linked: found during source review and reproduced against base 74ed67d. Existing #2334 bounds range reads; it does not handle alignment.

Contract and boundary impact

Semantic owner: ELF note alignment and descriptor/next-header offsets.
CLI/MCP schemas, provider authority, installation and generated metadata: unchanged.
Evidence: correctly retains later build-ID candidates; existing limits, cancellation, failure reporting and descriptor ownership remain.
Compatibility: effective eight-byte alignment is honored; other declared alignments follow the existing native parser's four-byte convention.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.

Evidence and regression coverage

Executed filesystem regressions cover ELF32/ELF64, four/eight-byte padding, and program/section/shared ownership (12 combinations). Before the fix, both new parameterized tests failed at the eight-byte program case; after it, every combination passes. Existing dense-range budget and duplicate-range tests still pass.

An independently constructed ELF64 fixture was also inspected with GNU readelf on Windows. It reported both build IDs (11 repeated 20 bytes and 22 repeated 20 bytes) in the 80-byte note range, with no malformed-note diagnostic.

Validation performed

Current head 07786951 merges upstream c106cf20, including the upstream test-seam repair in #6431. On an unmodified checkout of 74ed67d, the same three analysis-view projection failures seen in CI were reproduced. They pass after the upstream repair, both without this PR and with it.

Executed on Windows, Node 24.18.0 / npm 11.16.0:

npm run test:focused -- tests/boundary/filesystem/flutterElfNotes.test.ts tests/process-global/commandFinalizationCancellation.test.ts: 25 passed, 4 failed. All Flutter note tests and the 15 analysis-view finalization cases pass. The four failures are WASM command cases in the second file; the same four also fail in unmodified upstream c106cf20 (16 passed, 4 failed). The combined run is not claimed green.
npm run test:local -- src/native/elf/ElfNoteParser.test.ts src/flutter/DartElf.test.ts: 50 passed, 1 skipped (host ELF fixture unavailable).
npm run check:fast (typecheck and lint, including module boundaries) and git diff --check: passed.

Remote CI on this merge commit passed, including CI required. Its Linux run passes all 20 commandFinalizationCancellation cases. The local Windows limitations above remain recorded separately.

Compatibility, safety, and release

No target is executed. Reproduction uses synthetic local ELF files, not a production Flutter application; no real Android runtime or Linux/macOS lane was exercised. Full check:pr was not run. No security exploit or performance improvement is claimed.

Author readiness

Human owner: @Plastic-time

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6423 **Author:** [@Plastic-time](https://github.com/Plastic-time) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/flutter-note-alignment` --- ### 📝 Commits (3) - [`18e7b19`](https://github.com/morluto/rea/commit/18e7b19f5fae1f9f8b24a7ae4a262c2fafe817bd) fix(flutter): honor ELF note alignment when reading build IDs - [`4398a07`](https://github.com/morluto/rea/commit/4398a0793d487d566e975cb01189efc49d1714af) test(flutter): key shared note ranges by effective alignment - [`0778695`](https://github.com/morluto/rea/commit/07786951eed6f50fafe1ca4022ecea253432fd5e) Merge remote-tracking branch 'upstream/main' into codex/maintain-6423-20261023 ### 📊 Changes **4 files changed** (+129 additions, -18 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/native/elfNoteLayout.ts` (+16 -0) 📝 `src/flutter/ElfBuildId.ts` (+20 -6) 📝 `src/native/elf/ElfNoteParser.ts` (+9 -12) 📝 `tests/boundary/filesystem/flutterElfNotes.test.ts` (+84 -0) </details> ### 📄 Description ## Summary Flutter engine build-ID inspection assumes four-byte ELF note padding. With two 20-byte GNU build-ID notes in an eight-byte-aligned PT_NOTE range, it retains the first ID, starts the next header inside padding, and reports "ELF note descriptor exceeds its declared note range" instead of retaining both candidates. ## Change and scope Read p_align/sh_addralign alongside each declared note range and retain its effective alignment in the deduplication key. Share the existing native note parser's alignment and relative-offset calculation through a small domain owner, removing the duplicate arithmetic. Ordinary read-only format correction; additional maintainer scope approval is not applicable. No issue is linked: found during source review and reproduced against base `74ed67d`. Existing #2334 bounds range reads; it does not handle alignment. ## Contract and boundary impact - Semantic owner: ELF note alignment and descriptor/next-header offsets. - CLI/MCP schemas, provider authority, installation and generated metadata: unchanged. - Evidence: correctly retains later build-ID candidates; existing limits, cancellation, failure reporting and descriptor ownership remain. - Compatibility: effective eight-byte alignment is honored; other declared alignments follow the existing native parser's four-byte convention. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. ## Evidence and regression coverage Executed filesystem regressions cover ELF32/ELF64, four/eight-byte padding, and program/section/shared ownership (12 combinations). Before the fix, both new parameterized tests failed at the eight-byte program case; after it, every combination passes. Existing dense-range budget and duplicate-range tests still pass. An independently constructed ELF64 fixture was also inspected with GNU readelf on Windows. It reported both build IDs (`11` repeated 20 bytes and `22` repeated 20 bytes) in the 80-byte note range, with no malformed-note diagnostic. ## Validation performed Current head `07786951` merges upstream `c106cf20`, including the upstream test-seam repair in #6431. On an unmodified checkout of `74ed67d`, the same three analysis-view projection failures seen in CI were reproduced. They pass after the upstream repair, both without this PR and with it. Executed on Windows, Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- tests/boundary/filesystem/flutterElfNotes.test.ts tests/process-global/commandFinalizationCancellation.test.ts`: **25 passed, 4 failed**. All Flutter note tests and the 15 analysis-view finalization cases pass. The four failures are WASM command cases in the second file; the same four also fail in unmodified upstream `c106cf20` (16 passed, 4 failed). The combined run is not claimed green. - `npm run test:local -- src/native/elf/ElfNoteParser.test.ts src/flutter/DartElf.test.ts`: **50 passed, 1 skipped** (host ELF fixture unavailable). - `npm run check:fast` (typecheck and lint, including module boundaries) and `git diff --check`: passed. [Remote CI on this merge commit](https://github.com/morluto/rea/actions/runs/39912034441) passed, including `CI required`. Its Linux run passes all 20 `commandFinalizationCancellation` cases. The local Windows limitations above remain recorded separately. ## Compatibility, safety, and release No target is executed. Reproduction uses synthetic local ELF files, not a production Flutter application; no real Android runtime or Linux/macOS lane was exercised. Full `check:pr` was not run. No security exploit or performance improvement is claimed. ## Author readiness Human owner: @Plastic-time - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:15 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7350

No description provided.

Rows
Columns