build: bump NDK to r28 for 16 KB page-size compliance - #213
Conversation
Google Play rejects app updates without 16 KB page-size support since Nov 2025. The aw-server-rust native libs were built with NDK r25, which links LOAD segments 4 KB-aligned (verified: 0x1000). NDK r28+ links with -z max-page-size=16384 by default for all Android targets. Also extends scripts/check-jnilibs.py to fail CI when 64-bit libs have LOAD alignment below 0x4000, so an NDK downgrade can't silently regress compliance (mirrors Google's check_elf_alignment semantics: every LOAD segment must be >=16 KB aligned).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@greptileai review |
Greptile SummaryThe PR upgrades Android builds to NDK r28 and adds a CI gate for 16 KB ELF LOAD-segment alignment.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the two previously reported checker issues are addressed at the current head. Important Files Changed
Reviews (2): Last reviewed commit: "fix(check-jnilibs): also validate libaw_..." | Re-trigger Greptile |
| file=sys.stderr, | ||
| ) | ||
| return False | ||
|
|
||
| align = min_load_align(f, header, e_class) | ||
| if abi in PAGE_ALIGNED_ABIS: | ||
| if align is None or align < REQUIRED_LOAD_ALIGN: | ||
| print( | ||
| f"BAD_ALIGN {path}: LOAD align " | ||
| f"{'none' if align is None else hex(align)} < " | ||
| f"{hex(REQUIRED_LOAD_ALIGN)} (16 KB pages required; " |
There was a problem hiding this comment.
Alignment gate omits sync library
The build packages and loads libaw_sync.so, but this compliance gate validates only the hardcoded libaw_server.so path. An alignment regression in the sync library therefore remains invisible to CI, so the check cannot guarantee that every packaged 64-bit native library satisfies the Play requirement.
There was a problem hiding this comment.
Fixed in 2f25055 — both known libs (libaw_server.so, libaw_sync.so) are now required per ABI, and any other packaged *.so gets the same ELF+alignment validation.
| f.seek(e_phoff + i * e_phentsize) | ||
| phdr = f.read(e_phentsize) | ||
| p_type = struct.unpack("<I", phdr[0:4])[0] |
There was a problem hiding this comment.
Program-header reads lack validation
For a truncated ELF whose header declares a program-header entry past EOF, f.read() returns fewer bytes than requested and the following struct.unpack() raises an unhandled traceback. Validate the entry bounds or read length so the artifact check reports the malformed library with a controlled diagnostic.
There was a problem hiding this comment.
Fixed in 2f25055 — program-header table is bounds-checked against file size and short reads report CORRUPT instead of raising.
… reads Review findings: the gate only checked the hardcoded libaw_server.so while the Makefile also packages libaw_sync.so; and a truncated ELF declaring program headers past EOF crashed struct.unpack instead of reporting a diagnostic. Require both known libs, validate any other packaged *.so, and bounds-check phdr offsets/reads.
|
@TimeToBuildBob resolve the conflict, figure out why PM doesn't pick it up |
|
Why PM didn't pick it up: PR is authored by Resolving the conflict now. |
|
Prepared head is now pushed to Bob's fork at git fetch https://github.com/TimeToBuildBob/aw-android.git build/ndk-r28-16kb-pages
git push --force-with-lease origin FETCH_HEAD:build/ndk-r28-16kb-pagesVerified |
|
@TimeToBuildBob open a PR with the resolution instead |
Part of Play Store policy compliance (see Play Console: App must support 16 KB memory page sizes, enforced for updates since Nov 1, 2025 — this is what will reject any release built from current master, likely including the update currently in review).
Problem
libaw_server.sois built with NDK r25, which aligns ELF LOAD segments to 4 KB:Play requires 16 KB (
0x4000) alignment for 64-bit native libs.Fix
25.2.9519653→28.2.13676358inbuild.ymlandmobile/build.gradle— r28 links with-z max-page-size=16384by default for all Android targets. CI regenerates.cargo/configfromANDROID_NDK_HOMEviainstall-ndk.sh, so no other changes needed (the jniLibs cache key includes the NDK version, forcing a rebuild).scripts/check-jnilibs.pynow parses ELF program headers and fails when any 64-bit lib has a LOAD segment aligned below0x4000— verified locally: correctly rejects the current 4 KB-aligned r25 build, so CI enforces this permanently.Notes
aw-server-rust/install-ndk.shstill defaults to downloading r25c whenANDROID_NDK_HOMEis unset (local-dev path only; CI unaffected). Worth bumping upstream in aw-server-rust separately.