sdk: honour the platform's MAX_APP_BINARY_SIZE when injecting metadata - #1827
Open
AKlitbo wants to merge 1 commit into
Open
sdk: honour the platform's MAX_APP_BINARY_SIZE when injecting metadata#1827AKlitbo wants to merge 1 commit into
AKlitbo wants to merge 1 commit into
Conversation
tools/pebble_sdk_platform.py gives emery and gabbro a MAX_APP_BINARY_SIZE of 0x20000, but inject_metadata.py carried its own module-scope MAX_APP_BINARY_SIZE = 0x10000 and nothing ever passed the platform's value in. Every platform was therefore held to 64 KB, and an emery or gabbro app that went past it failed with "App image size is N ... Must be smaller than 65536 bytes" while the memory report printed alongside it correctly said 128 KB. Thread the platform's limit through generate_bin_file -> gen_inject_metadata_rule -> inject_metadata, defaulting to the old constant so callers that pass nothing are unaffected. The value is read in generate_bin_file rather than inside the rule because bld.env only holds the platform's ConfigSet while the caller's per-platform loop is running, and the rule does not run until later. Raising the total exposes a second ceiling that was previously unreachable: PebbleProcessInfo.load_size and .virtual_size are uint16_t, so neither can exceed 65535 however large the platform's total allows. Only the relocation table, stored past load_size, can use the space above 64 KB. Without a check an app in that range died in pack() with a bare struct.error, so both fields are now range-checked with a message naming the limit that was hit. Testing: - A 15700-entry pointer array builds on emery and gabbro to a 126932 byte image (load_size 64132, 15700 relocation entries, 96.8% of the 128 KB ceiling), installs, and reports all 15700 pointers correctly relocated at runtime. The same app fails on stock at 65536. - Older platforms are unchanged: building the same project against the stock and patched SDK produces byte-identical app and worker binaries for aplite, basalt, chalk, diorite, emery, flint and gabbro, once the per-build resource_timestamp is masked. - basalt, chalk, diorite and flint still reject an oversized image at exactly 65536, and aplite still fails earlier at link time on its 24 KB APP region. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrew Klitbo <andrew.klitbo@gmail.com>
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
tools/pebble_sdk_platform.pygives emery and gabbro aMAX_APP_BINARY_SIZEof0x20000, butsdk/tools/inject_metadata.pycarried its own module-scopeMAX_APP_BINARY_SIZE = 0x10000and nothing ever passed the platform's value in.inject_metadata()took no platform argument and its caller supplied none, so every platform was held to 64 KB regardless of what its config declared. An emery or gabbro app past that failed withApp image size is N (app N relocation table N). Must be smaller than 65536 bytes, while the memory report printed a few lines earlier in the same build said128.0KB—sdk/waftools/report_memory_usage.pydoes readenv.PLATFORM, so the two halves of the build disagreed with each other.This threads the platform's limit through
generate_bin_file->gen_inject_metadata_rule->inject_metadata, defaulting to the old constant so callers that pass nothing are unaffected. The value is read ingenerate_bin_filerather than inside the rule closure:bld.envonly holds the platform's ConfigSet whileprocess_bundle.py's per-platform loop is running, and the rule does not run until later. This matches the two pre-existing eager reads in the same function (BUILD_DIR,TIMESTAMP).Raising the total exposes a second ceiling that was previously unreachable.
PebbleProcessInfo.load_sizeand.virtual_sizeareuint16_t, so neither can exceed 65535 however large the platform's total allows — only the relocation table, stored pastload_size, can use the space above 64 KB. Both fields are now range-checked with a message naming the limit that was hit. This also covers a latent case in the old code: the check wastotal > MAX_APP_BINARY_SIZE, so a total of exactly 65536 passed and then died inpack("<H", ...)with a barestruct.errorand no indication of the cause.Scope notes:
virtual_sizestill caps static footprint at 65535 on every platform; widening that would need aPROCESS_INFO_CURRENT_STRUCT_VERSIONbump and matching changes in the mobile bundle parsers. The space this opens up is usable only by an oversized relocation table.apps/stored/wscript_buildalso callsgen_inject_metadata_rulewithout amax_binary_sizeand so keeps the 64 KB default. Left alone deliberately — it is the firmware build rather than the SDK, and stored apps are far below either limit. Happy to follow up if you would prefer it consistent.src/fw/services/process_management/app_storage.c'sAPP_MAX_SIZE = 0x10000is checked againstvirtual_size, which isuint16_tand so can never reach0x10000. The check is unreachable either way and is untouched here.No new API surface, so the
docs/development/sdk_export.mdprocess does not apply.sdk/wscript_buildalready bundles all three modified files, so no export list needed updating.Testing
load_size64132, 15700 relocation entries, 96.8% of the 128 KB ceiling). It installs in the emery emulator and reports all 15700 pointers correctly relocated at runtime, confirming the firmware loads a relocation table this far past 64 KB. The same app fails on an unpatched SDK at 65536.resource_timestampis masked.APPregion, unchanged.virtual_sizeguard was exercised in passing: an intermediate build hit 66060 bytes and reported the field and its limit, where the unpatched SDK raisedstruct.error.ruff format --checkclean on all three files;gitlintclean against the repo's.gitlint.