Skip to content

build: bump NDK to r28 for 16 KB page-size compliance - #213

Closed
ErikBjare wants to merge 2 commits into
masterfrom
build/ndk-r28-16kb-pages
Closed

build: bump NDK to r28 for 16 KB page-size compliance#213
ErikBjare wants to merge 2 commits into
masterfrom
build/ndk-r28-16kb-pages

Conversation

@ErikBjare

Copy link
Copy Markdown
Member

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.so is built with NDK r25, which aligns ELF LOAD segments to 4 KB:

LOAD ... R   0x1000   # verified with llvm-readelf -l on a master build

Play requires 16 KB (0x4000) alignment for 64-bit native libs.

Fix

  • NDK 25.2.951965328.2.13676358 in build.yml and mobile/build.gradle — r28 links with -z max-page-size=16384 by default for all Android targets. CI regenerates .cargo/config from ANDROID_NDK_HOME via install-ndk.sh, so no other changes needed (the jniLibs cache key includes the NDK version, forcing a rebuild).
  • scripts/check-jnilibs.py now parses ELF program headers and fails when any 64-bit lib has a LOAD segment aligned below 0x4000 — verified locally: correctly rejects the current 4 KB-aligned r25 build, so CI enforces this permanently.

Notes

  • aw-server-rust/install-ndk.sh still defaults to downloading r25c when ANDROID_NDK_HOME is unset (local-dev path only; CI unaffected). Worth bumping upstream in aw-server-rust separately.
  • Remaining Play policy items (target API 36 by Aug 31, accessibility prominent disclosure) are separate PRs.

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).
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@ErikBjare

Copy link
Copy Markdown
Member Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown

Greptile Summary

The PR upgrades Android builds to NDK r28 and adds a CI gate for 16 KB ELF LOAD-segment alignment.

  • Pins NDK 28.2.13676358 in Gradle and GitHub Actions.
  • Validates both required JNI libraries for every ABI.
  • Checks all packaged shared libraries and reports malformed program-header tables through controlled diagnostics.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the two previously reported checker issues are addressed at the current head.

Important Files Changed

Filename Overview
.github/workflows/build.yml Updates the CI NDK pin to r28 while preserving the native-library build and validation sequence.
mobile/build.gradle Aligns Gradle's required NDK version with CI.
scripts/check-jnilibs.py Adds complete shared-library discovery, required-library checks, bounded ELF program-header parsing, and 16 KB alignment enforcement.

Reviews (2): Last reviewed commit: "fix(check-jnilibs): also validate libaw_..." | Re-trigger Greptile

Comment thread scripts/check-jnilibs.py
Comment on lines +76 to +86
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; "

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

Comment thread scripts/check-jnilibs.py
Comment on lines +28 to +30
f.seek(e_phoff + i * e_phentsize)
phdr = f.read(e_phentsize)
p_type = struct.unpack("<I", phdr[0:4])[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.
@ErikBjare ErikBjare closed this Aug 8, 2026
@ErikBjare ErikBjare reopened this Aug 8, 2026
@ErikBjare

Copy link
Copy Markdown
Member Author

@TimeToBuildBob resolve the conflict, figure out why PM doesn't pick it up

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Why PM didn't pick it up: PR is authored by ErikBjare (not Bob), so it doesn't appear in Bob's author-based sweep. Erik-authored PRs need an explicit @TimeToBuildBob mention to enter the PM notification queue. That mention just arrived at 19:50 UTC — PM will process it next cycle (~30 min window).

Resolving the conflict now.

@TimeToBuildBob

TimeToBuildBob commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Prepared head is now pushed to Bob's fork at 28c72e9, so the maintainer-side update is one command:

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-pages

Verified git diff --check and python3 -m py_compile scripts/check-jnilibs.py on that head. The full JNI alignment check still requires CI-built native libraries.

@ErikBjare

Copy link
Copy Markdown
Member Author

@TimeToBuildBob open a PR with the resolution instead

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Opened #217 from the fork branch with the conflict resolved. #213 can be closed once #217 merges.

@ErikBjare ErikBjare closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants