[metascroy]

Adds a private reader that extracts Core AI processed buffers from a PTE without constructing a Program or Method, so cache maintenance can find a program's Core AI models without loading it (runtime/coreai_pte.{h,cpp}).

• inspect_coreai_pte(DataLoader&) returns the CoreAIBackend processed buffers in method/delegate order.
• Always runs the flatbuffer structural verifier, the supported-schema-version check and the runtime's semantic validator, even with ET_ENABLE_PROGRAM_VERIFICATION=0; also checks method metadata, unique method names, exact backend IDs, data ranges, alignment and CPU accessibility. Reuses the existing extended-header parser; no shared runtime code changes.
• Inline buffers borrow the retained program image. Only external segments referenced by CoreAIBackend delegates are loaded, keeping their original FreeableBuffer callbacks; unrelated payloads and named data are not loaded.
• CoreAIPteData owns the program image and buffers; it is move-only and the loader must outlive it.
• Tests: a synthetic PTE builder (runtime/test/coreai_pte_fixture.{h,mm}) and a suite covering inline and segmented delegates, method-order routing, exact backend-ID matching, header formats, load-error propagation and buffer release on late failure. The PTE is treated as trusted exporter output, so malformed-PTE coverage is limited to the verifier and version checks.
• README: "Backend-local PTE Inspection".

No changes outside backends/apple/coreai.

Stack: 8 of 9, based on #23393. Review only this PR's commit. Next: #23395.

Test plan: built locally for macOS 27.0; ctest -N lists coreai_host_test, coreai_swift_bridge_test and coreai_runtime_smoke, which the Core AI workflow runs on the macOS 27 runner. lintrunner is clean.

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23394

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[metascroy]

@claude review this PR

[claude]

Claude finished @metascroy's task in 2m 44s —— View job

---

PR Review: #23394

I found no correctness bugs. The main issue is that inspect_coreai_pte adds several validation checks that Program::load doesn't have and that no test exercises; the rest are small cleanups.

Detailed review

Summary

This PR adds inspect_coreai_pte, which pulls CoreAIBackend processed buffers out of a PTE without building a Program. I checked it against Program::load / Program::LoadSegment. Header parsing, segment offsets, the order buffers are freed in (processed before program_storage) and error propagation are all correct. The concerns are:

• validation code that goes beyond the "trusted exporter output" stance and has no tests
• gaps in the tests
• a few naming and housekeeping items

Code Quality

• Validation goes further than both Program::load and the stated trust model. backends/apple/coreai/runtime/coreai_pte.cpp:107-121 reads the header a second time from the retained image and compares every field. That only catches a loader that returns different bytes for the same offset, which the header contract already rules out ("loader and its data must remain stable"). Several other checks have no counterpart in Program::load:

  • unique method names
  • non-null non_const_buffer_sizes, inputs and outputs
  • segment_base_offset >= program_size
  • non-empty inline processed data
  • buffer->size() != size after cpu_data already checked < size

  Two problems follow:

  1. None of these checks is tested (see Testing).
  2. The reader can reject a PTE that Program::load accepts. The cache maintenance that relies on it would then not see that program's Core AI models.

  Program::load also returns InvalidArgument for a misaligned image, while this code returns InvalidProgram (:103-106).

  Given CLAUDE.md's "when uncertain: choose simpler", I'd match Program::load + LoadSegment semantics and drop the extra checks. Otherwise, document and test each one as a deliberate requirement.

• Dead null checks. In a verified buffer, Get() on a flatbuffer vector of tables never returns null. That makes delegate == nullptr (:146), buffers->Get(index) == nullptr (:162) and segments->Get(index) == nullptr (:174) unreachable.

• Unused include. #include <cstring> in coreai_pte.cpp isn't used.

• Names don't match. The suite is coreai_program_data_test.mm / CoreAIProgramDataTest, but it tests coreai_pte.{h,cpp}. The fixture types (CacheDelegateSpec, CacheMethodSpec) are named for cache use rather than PTE building. Renaming to coreai_pte_test.mm / CoreAIPteTest would make the suite easier to find.

• README describes internals of a private API. The "Backend-local PTE Inspection" section covers the 64-byte prefix read, the load order and buffer ownership of a function the PR itself calls private. The ownership and lifetime contract belongs in the coreai_pte.h doc comment, where callers will look. I'd keep the README to one or two sentences.

Testing

• Validation branches are untested. Nothing exercises:

  • header bounds: program_size > file_size, segment_base_offset out of range, segment_data_size overflow
  • an out-of-range processed index for a CoreAIBackend delegate, inline or segment
  • a missing processed reference on a CoreAIBackend delegate
  • SEGMENT location without an extended header
  • duplicate or null method names
  • a misaligned program image

  The fixture already supports index_override and missing_processed. Both are only used on delegates whose backend ID doesn't match, so the inspector skips them before the checks run. Pointing them at CoreAIBackend delegates would cover most of the list cheaply. If you take the simplification above, the list gets much shorter.

• Unused fixture fields. fail_backend_index (coreai_pte_fixture.h) is never set by a test. LoadFault has only None/Error, so a bool would do. The DedupVtables(false) comment talks about "malformed-field tests", but only the version test changes a field.

• Weak assertion. ReleasesEarlierBuffersOnLateFailure uses EXPECT_GE(pte.backend_loads, 1). With fault_call = 3 the count is always exactly 1, so EXPECT_EQ would catch a regression that GE misses.

Recommendation

Approve. Nothing blocks merging. Before landing, I'd recommend either trimming the validation back to Program::load parity or adding tests for the extra checks.

[metascroy]

@claude review this code

Updated head: ba2439e5cc22621102df1df10d37deff55c93af0. The stack and parent PR have been refreshed; please review against the current base.

Context on the previous findings: unrelated method-metadata validation, unique-name checks, and the second header comparison were removed. One correction to the earlier review: required method metadata is checked by Program::method_meta, although not by Program::load; accepting it here does not establish method executability.

We intentionally retained FlatBuffer verification, overflow/range checks, checks for references actually dereferenced by this reader, and the exact returned-buffer-size guarantee. The exact-size check is not redundant with a minimum-size check: it rejects oversized buffers that would otherwise be returned intact. Those cases now have focused coverage, alongside malformed CoreAI references, bounds, alignment, and release-on-failure tests. Misalignment returns InvalidArgument to match runtime loading. The fault enum is retained because it now represents distinct error, short, oversized, and misaligned responses rather than a boolean failure.

[claude]

Claude finished @metascroy's task in 3m 45s —— View job

---

PR Review: #23394 (head ba2439e)

I re-reviewed against the refreshed base and found no correctness bugs. The previous findings are resolved, and I agree with your corrections. What remains is minor: one stated guarantee has no test, a few places differ from the runtime, and two comments are out of date.

Detailed review

Summary

inspect_coreai_pte reads the CoreAIBackend processed buffers out of a PTE without constructing a Program. I checked the header bounds, segment offsets, ownership and destruction order, and load-error propagation against Program::load / LoadSegment, and they are correct. Three independent reviewers read all seven files. Every item below was re-checked against the code. Nothing was built or run.

Code Quality

• A malformed non-CoreAI delegate makes the whole inspection fail (runtime/coreai_pte.cpp:124-126).
  • What happens: the delegate->id() == nullptr check runs before the backend filter. validate_program doesn't check delegate ids, so this line is the only thing that rejects them.
  • Consequence: if any method has a delegate with no id, even one belonging to another backend, inspection returns InvalidProgram. Cache maintenance then can't see that program's Core AI models.
  • Why it's inconsistent: the runtime only fails when that particular method is loaded. That conflicts with the rule "only check references this reader dereferences".
  • Suggested fix: a delegate with a null id can't be CoreAIBackend, so treat it as unselected and continue.
• Some error codes and header checks differ from Program::load/LoadSegment, even though alignment now matches.
  • Error codes: the runtime returns NotFound in three cases where this code returns InvalidProgram (coreai_pte.cpp:140, :151-152, locked in by tests at coreai_pte_test.mm:305-327):
    • an inline index out of range (program.cpp:570)
    • a segment reference with no header or segment_base_offset == 0 (:605)
    • a segment index out of range (:616)
  • Header checks: coreai_pte.cpp:77-81 checks segment_base_offset against program_size/file_size even when segment_data_size == 0. Program::load skips that check for the pre-1.0 BC case (program.cpp:99-103), so a legacy-header PTE with inline-only Core AI data is loaded by the runtime but rejected here. Real exporter output is unlikely to hit this.
  • Suggested fix: match the runtime in both places, or note in coreai_pte.h that the reader uses its own, stricter error contract.
• The ownership comment is misleading (runtime/coreai_pte.h:21). "Borrowed inline data and segment descriptors must die before the program" suggests segment buffers borrow the image, but they own their data. The rule that actually matters is that processed is declared after program_storage, so it is destroyed first and the inline borrows stay valid. Saying that the member order is load-bearing would stop someone from reordering the fields.
• The backend ID string is duplicated (coreai_pte.cpp:127 and coreai_backend.mm:398). If the registered name changes, inspection won't fail; it will silently return an empty result. A shared constant would prevent that.
• README Host Tests paragraph is outdated (README.md:278-283). It still lists only manifest, assets, storage, acquisition and bookmarks, but coreai_host_test now includes the PTE inspection suite too.

Testing

• validate_program never fails in any test (coreai_pte.cpp:113-114). The PR says the semantic validator always runs, but RejectsMalformedFlatbufferAndUnsupportedVersion (coreai_pte_test.mm:415-431) only covers the verifier (bad identifier) and the version check. If you deleted the validate_program(...) != Error::Ok clause, every test would still pass. A buffer that passes the verifier but fails semantic validation (for example clear_field(plan, fb::ExecutionPlan::VT_VALUES) with the existing helper) would cover it.
• Two rejection branches have no test:
  • a null delegate id (:124). The fixture already supports backend == nullptr, but no test uses it.
  • an unknown DataLocation (:187). The FlatBuffer verifier doesn't range-check enums, so this branch is the only guard.
• All tests use synthetic PTEs (coreai_pte_fixture.mm). The fixture writes the extended header, 16-byte segment padding and segment_data_size its own way. exir/_serialize/_program.py uses different alignment defaults. If the fixture and the reader misread the layout the same way, the tests still pass. One test on a real exported PTE with a CoreAIBackend delegate, even a tiny checked-in one, would cover this. Low priority.

Recommendation

Approve. Nothing here blocks the merge. Of the items above, the validate_program test and the null-id continue are the most worthwhile follow-ups.