Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/polkavm/src/compiler/amd64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,7 +1319,6 @@ where
));

let count = conv_reg(Reg::A2.into());
self.asm.push(mov(RegSize::R32, count, count));

match self.gas_metering {
None => {
Expand Down
7 changes: 4 additions & 3 deletions crates/polkavm/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3108,8 +3108,9 @@ define_interpreter! {
let mut result = next_instruction;

let value = visitor.get32::<DEBUG>(Reg::A1);
let mut dst = visitor.get32::<DEBUG>(Reg::A0);
let mut count = visitor.get64::<DEBUG>(Reg::A2);
let a0 = visitor.get64::<DEBUG>(Reg::A0);
let mut dst = cast(a0).truncate_to_u32();
let mut count = u64::from(visitor.get32::<DEBUG>(Reg::A2));
while count > 0 {
if gas_metering_enabled && visitor.gas == 0 {
result = not_enough_gas_impl::<DEBUG>(visitor, compiled_offset, program_counter, 0);
Expand All @@ -3129,7 +3130,7 @@ define_interpreter! {
count -= 1;
}

visitor.set64::<DEBUG>(Reg::A0, u64::from(dst));
visitor.set64::<DEBUG>(Reg::A0, a0.wrapping_add(u64::from(dst.wrapping_sub(cast(a0).truncate_to_u32()))));
visitor.set64::<DEBUG>(Reg::A2, count);

result
Expand Down
39 changes: 39 additions & 0 deletions crates/polkavm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4571,6 +4571,44 @@ fn memset_with_dynamic_paging(mut config: Config) {
assert_eq!(instance.gas(), 95);
}

fn memset_preserves_a0_and_a2(config: Config) {
let _ = env_logger::try_init();

// Memset must not truncate A0 or A2. With count=0, memset is a no-op
// and both registers must pass through with their upper 32 bits intact.
let mut builder = ProgramBlobBuilder::new(InstructionSetKind::Latest64);
builder.add_export_by_basic_block(0, b"main");
builder.set_code(
&[
// A0 = 0xffffffffffff0000 * 0xffffffffffff0000 = 0x0000000100000000
asm::load_imm(Reg::A0, 0xffff0000),
asm::mul_64(Reg::A0, Reg::A0, Reg::A0),
// A2 = sign_extend(0xff08bdbd) = 0xffffffffff08bdbd
asm::load_imm(Reg::A2, 0xff08bdbd),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Again, this part is unnecessarily complicated because you can just use set_reg to set the desired values directly instead of calculating them.

The whole set_code ideally should just look like this:

builder.set_code(&[asm::memset(), asm::ret()]);

There's no need to involve mul_64, load_imm, etc. and overcomplicate the test because this test is supposed to test memset and memset only.

// Swap A2 into A3 so we can set count=0 while keeping the test value.
asm::move_reg(Reg::A3, Reg::A2),
asm::load_imm(Reg::A2, 0),
asm::memset(),
asm::ret(),
],
&[],
);

let blob = ProgramBlob::parse(builder.into_vec().unwrap().into()).unwrap();
let engine = Engine::new(&config).unwrap();
let module = Module::from_blob(&engine, &ModuleConfig::new(), blob).unwrap();

let mut instance = module.instantiate().unwrap();
instance.set_reg(Reg::RA, crate::RETURN_TO_HOST);
instance.set_next_program_counter(ProgramCounter(0));
assert!(matches!(instance.run().unwrap(), InterruptKind::Finished));
assert_eq!(instance.reg(Reg::A0), 0x0000000100000000, "memset truncated A0");
// A2 was set to 0 (count), so it should be 0 after memset.
assert_eq!(instance.reg(Reg::A2), 0);
// A3 was never touched by memset, just a sanity check.
assert_eq!(instance.reg(Reg::A3), 0xffffffffff08bdbd);
}

fn count_leading_zero_bits_32_with_zero_input(config: Config) {
let _ = env_logger::try_init();

Expand Down Expand Up @@ -5245,6 +5283,7 @@ run_tests! {
count_trailing_zero_bits_32_with_zero_input
count_trailing_zero_bits_64_with_zero_input
count_trailing_zero_bits_64_with_ffff0000
memset_preserves_a0_and_a2
}

run_test_blob_tests! {
Expand Down
Loading