Skip to content

fix(vm): compute memcmp result before translating the mutable output - #1766

Open
0xzrf wants to merge 1 commit into
Syndica:mainfrom
0xzrf:memcmp_fix
Open

0xzrf wants to merge 1 commit into
Syndica:mainfrom
0xzrf:memcmp_fix

Conversation

@0xzrf

@0xzrf 0xzrf commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #1742

Problem

sol_memcmp_ translated both input slices, then translated the mutable result pointer, and only afterwards read the inputs:

const s1 = try memory_map.translateSlice(u8, .constant, a_addr, len, check_aligned);
const s2 = try memory_map.translateSlice(u8, .constant, b_addr, len, check_aligned);
const cmp_result = try memory_map.translateType(i32, .mutable, result_address, check_aligned);
for (s1, s2) |a, b| { ... }   // reads s1/s2 after the growth

Under direct_mapping + VASA (SIMD-0460), the mutable translation can invoke the access-violation handler, which calls account.resize — reallocating and freeing the old backing

Fix

Compute the comparison into a plain i32 while both inputs are still valid, then translate the mutable result and write the value like agave

Changes

  • shared/vm/syscalls/memops.zig — move the comparison above the result translation; write the precomputed value.
  • Regression test memcmp reads inputs before result-triggered growth, which fails before the change (expected 0, found 153) and passes after.

@github-project-automation github-project-automation Bot moved this to 🏗 In progress in Sig Jul 26, 2026
@dnut dnut added the external label Jul 27, 2026
@0xzrf

0xzrf commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

hey @dnut
I've resolved the conflicts

Can I get a quick review please
thanks.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

Reads stale sol_memcmp_ inputs after result triggered account growth

2 participants