Skip to content

fix(ui-android): JSON pointer test asks the runtime instead of testing for a subnormal (#7448) - #7802

Merged
proggeramlug merged 1 commit into
mainfrom
fix/7448-android-json-subnormal
Aug 11, 2026
Merged

fix(ui-android): JSON pointer test asks the runtime instead of testing for a subnormal (#7448)#7802
proggeramlug merged 1 commit into
mainfrom
fix/7448-android-json-subnormal

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

perry-ui-android's JSON walk no longer dereferences positive subnormal numbers (#7448).

crates/perry-ui-android/src/json.rs carried a verbatim copy of the predicate #7447 removed from the main runtime:

exponent == 0 && mantissa != 0 && sign == 0

That is bit-for-bit the IEEE-754 positive-subnormal test, so every positive denormal Number was classified as an untagged heap pointer and dereferenced. In the main runtime the identical code SIGSEGV'd on JSON.stringify(1e-317) and returned a silent null for 5e-324 — both reachable from untrusted input through JSON.stringify(JSON.parse(text)).

The issue asked for two decisions to be made alongside the fix, and both point the same way: no bit test can decide this. A raw untagged pointer and a positive subnormal occupy the same bit patterns by construction, which is why the runtime's own version burned through two failed narrowings (top16 < 0x7FF8, then top16 == 0) before landing on allocation membership. A third divergent copy is how that happened in the first place.

So the copy is deleted rather than ported: perry_runtime::json::ptr_is_tracked_heap_object is now exported (it was pub(super), which is what blocked #7447 from fixing this) and the android path calls it. It answers from the page map and the malloc registry, both dereference-free, so a forged or unmapped address is rejected before any field is read.

#7447 left this unfixed because the crate could not be built or verified on the machine that made the fix. That is not true here — cargo check -p perry-ui-android builds on macOS arm64 in under a second, and both it and perry-runtime check clean with this change.

The second question the issue raises — whether the untagged-pointer branch is reachable on android at all, given it measured dead in the main runtime (155,540 rejections, zero admissions across the JSON corpus) — is deliberately left open. Deleting the branch outright would be the kill-policy answer, but that needs a measurement on a device, and this change makes the branch safe either way.

Verified: JSON.stringify of 1e-317, 5e-324, a nested object of subnormals, and a JSON.stringify(JSON.parse(…)) round-trip all match Node v26.5.1 exactly through the runtime path; perry-runtime's json unit tests are 78 passed / 0 failed and test_gap_json is 7/7. The only remaining matches for the old bit pattern in the tree are the two doc comments that quote it as history.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an Android JSON issue that could incorrectly interpret certain numeric values as object references.
    • Improved validation of JSON object pointers to prevent invalid dereferences and related crashes or corrupted results.
    • Preserved correct handling of tagged pointers while improving support for edge-case numeric values.
  • Documentation

    • Added a changelog entry describing the Android JSON reliability fix.

@proggeramlug
proggeramlug force-pushed the fix/7448-android-json-subnormal branch from f25528e to 9d28294 Compare August 10, 2026 21:37
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The runtime now publicly exposes tracked heap-object validation. Android JSON pointer extraction uses this validation instead of a local positive-subnormal bit test. The changelog records the fix and test results.

Changes

Tracked pointer validation

Layer / File(s) Summary
Expose tracked heap predicate
crates/perry-runtime/src/json/stringify.rs, crates/perry-runtime/src/json/mod.rs
ptr_is_tracked_heap_object is public and re-exported from crate::json.
Use runtime validation
crates/perry-ui-android/src/json.rs, changelog.d/7802-android-json-subnormal.md
Android JSON pointer extraction uses tracked heap-object validation for untagged values. The changelog records the fix and validation results. The existing stringify_array reference to the removed is_raw_pointer helper remains unresolved.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Possibly related PRs

  • PerryTS/perry#7447 — Extends the same positive-subnormal pointer-validation fix to Android.
  • PerryTS/perry#7473 — Uses GC-tracked heap-object validation for untagged JSON pointers.
  • PerryTS/perry#6636 — Modifies related heap-pointer validation for NaN-boxed values.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the Android JSON fix and the runtime-based pointer validation change.
Description check ✅ Passed The description explains the issue, implementation, related issue, and verification results, so it is mostly complete despite not using the template headings.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7448-android-json-subnormal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/perry-ui-android/src/json.rs (1)

340-349: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Replace the unresolved is_raw_pointer call.

crates/perry-ui-android/src/json.rs:555 calls a helper that is not defined in the file. Use perry_runtime::json::ptr_is_tracked_heap_object(elem_bits as *const u8) for the untagged-pointer branch.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-ui-android/src/json.rs` around lines 340 - 349, Update the
untagged-pointer branch at the call site around extract_pointer to replace the
unresolved is_raw_pointer helper with
perry_runtime::json::ptr_is_tracked_heap_object(elem_bits as *const u8), while
preserving the existing tagged-pointer handling and return behavior.
🧹 Nitpick comments (1)
changelog.d/7802-android-json-subnormal.md (1)

1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reduce this fragment to the shipped behavior.

Remove implementation history, unresolved investigation notes, machine-specific build details, and test counts. Keep one concise release-note entry that states that Android JSON serialization no longer treats positive subnormal numbers as heap pointers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7802-android-json-subnormal.md` around lines 1 - 19, Reduce the
changelog entry to one concise release-note sentence stating that Android JSON
serialization no longer treats positive subnormal numbers as heap pointers.
Remove the implementation details, historical context, unresolved questions,
machine-specific verification notes, and test counts.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/perry-ui-android/src/json.rs`:
- Around line 340-349: Update the untagged-pointer branch at the call site
around extract_pointer to replace the unresolved is_raw_pointer helper with
perry_runtime::json::ptr_is_tracked_heap_object(elem_bits as *const u8), while
preserving the existing tagged-pointer handling and return behavior.

---

Nitpick comments:
In `@changelog.d/7802-android-json-subnormal.md`:
- Around line 1-19: Reduce the changelog entry to one concise release-note
sentence stating that Android JSON serialization no longer treats positive
subnormal numbers as heap pointers. Remove the implementation details,
historical context, unresolved questions, machine-specific verification notes,
and test counts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7dee6921-9a27-448a-a870-7432fd7ad02f

📥 Commits

Reviewing files that changed from the base of the PR and between b9415d7 and 9d28294.

📒 Files selected for processing (4)
  • changelog.d/7802-android-json-subnormal.md
  • crates/perry-runtime/src/json/mod.rs
  • crates/perry-runtime/src/json/stringify.rs
  • crates/perry-ui-android/src/json.rs

@proggeramlug
proggeramlug merged commit 3d7ad8d into main Aug 11, 2026
11 of 18 checks passed
@proggeramlug
proggeramlug deleted the fix/7448-android-json-subnormal branch August 11, 2026 05:24
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.

1 participant