Skip to content

fix(gnovm): respect Unicode range in 64-bit integer-to-string conversions#5807

Open
omarsy wants to merge 3 commits into
gnolang:masterfrom
omarsy:fix-string-conv-range
Open

fix(gnovm): respect Unicode range in 64-bit integer-to-string conversions#5807
omarsy wants to merge 3 commits into
gnolang:masterfrom
omarsy:fix-string-conv-range

Conversation

@omarsy

@omarsy omarsy commented Jun 11, 2026

Copy link
Copy Markdown
Member

Description

Per the Go spec (Conversions to and from a string type), converting an integer to a string yields the UTF-8 of the code point, and "values outside the range of valid Unicode code points are converted to ".

GnoVM implemented the conversion for the 64-bit-capable kinds (int, int64, uint, uint64) as string(rune(tv.GetInt64())) etc. The rune(...) wrapper truncates to int32 before Go's own range check runs, so an out-of-range value can alias onto a valid code point instead of yielding :

input Go gno (master)
string(uint64(0x10001F600)) 😀
string(int(-4294967231)) A
string(uint64(0x100000000)) "\x00"

The fix checks that the value fits in an int32 first and returns utf8.RuneError otherwise; once a value fits in int32, Go's native string(rune) already maps every invalid case (negatives, surrogate halves, > 0x10FFFF) to . Both the runtime and constant-evaluation paths flow through the same ConvertTo, so one fix covers both (including untyped rune constants like 'A' + 0x100000000).

The remaining integer kinds are intentionally unchanged — int8/int16/uint8/uint16 cannot exceed the int32 range, Int32 converts without narrowing, and uint32 is provably equivalent (values > MaxInt32 reinterpret as negative runes → ; values in (0x10FFFF, MaxInt32] are out of range → ). Details in the included ADR.

Verification

Found via differential testing against the Go toolchain. Verified byte-for-byte against go run across all integer kinds, named types, and untyped constants at every range boundary (0xD7FF/0xD800/0xDFFF/0xE000, 0x10FFFF/0x110000, MaxInt32±1, MinInt32, ±2^32±k, MinInt64/MaxInt64/MaxUint64), in multiple syntactic positions (var init, const decl, concatenation, map keys, switch, struct literals). The new filetest fails on master and passes with the fix; the full TestFiles suite passes.

Note

This PR is AI-assisted (found and developed with Claude); it includes an ADR per AGENTS.md (gnovm/adr/prxxxx_string_conversion_unicode_range.md — to be renamed with the PR number).

Related spec-compliance fixes from the same differential-testing effort: #5784, #5785.

🤖 Generated with Claude Code

…ions

Converting int/int64/uint/uint64 values to string narrowed the value to
int32 via rune(...) before Go's own range check could run, so out-of-range
values aliased onto valid code points instead of yielding the replacement
character per the Go spec:

    string(uint64(0x10001F600))  // was "😀", Go yields "�"
    string(int(-4294967231))     // was "A",  Go yields "�"

Check that the value fits in an int32 first and return utf8.RuneError
otherwise; in-int32 invalid values (negatives, surrogates, > 0x10FFFF) are
already mapped to "�" by Go's native string(rune) conversion. The
remaining integer kinds are unaffected (see ADR for why uint32 is provably
safe). Covers both the runtime and constant evaluation paths, which share
ConvertTo.
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jun 11, 2026
@Gno2D2

Gno2D2 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: omarsy/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 User davd-gzl already reviewed PR 5807 with state APPROVED
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

@davd-gzl davd-gzl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Verified on b228597: output matches the Go toolchain byte-for-byte across the full boundary table, and reverting the fix brings back the old truncation aliasing (string(uint64(0x10001F600)) gives 😀 instead of ).

Full review: https://github.com/samouraiworld/gno-agent-workspace/blob/main/reviews/pr/5xxx/5807-string-conversion-unicode-range/1-b22859722/review_claude-opus-4-8_davd-gzl.md

(AI Agent)

Related: #5749

@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a/vm GnoVM, Security, Runtime team 📦 🤖 gnovm Issues or PRs gnovm related

Projects

Development

Successfully merging this pull request may close these issues.

4 participants