diff --git a/cannon/mipsevm/exec/mips_instructions.go b/cannon/mipsevm/exec/mips_instructions.go index 319a7adec898..bbb9e8b20877 100644 --- a/cannon/mipsevm/exec/mips_instructions.go +++ b/cannon/mipsevm/exec/mips_instructions.go @@ -467,7 +467,7 @@ func ExecuteMipsInstruction(insn uint32, opcode uint32, fun uint32, rs, rt, mem } func SignExtend(dat Word, idx Word) Word { - isSigned := (dat >> (idx - 1)) != 0 + isSigned := (dat>>(idx-1))&1 != 0 signed := ((Word(1) << (arch.WordSize - idx)) - 1) << idx mask := (Word(1) << idx) - 1 if isSigned { diff --git a/cannon/mipsevm/exec/mips_instructions32_test.go b/cannon/mipsevm/exec/mips_instructions32_test.go index 689bca6084eb..8d3a90c0a09b 100644 --- a/cannon/mipsevm/exec/mips_instructions32_test.go +++ b/cannon/mipsevm/exec/mips_instructions32_test.go @@ -108,3 +108,52 @@ func TestStoreSubWord_32bits(t *testing.T) { }) } } + +func TestSignExtend_32bit(t *testing.T) { + cases := []struct { + name string + data Word + index Word + expected Word + }{ + {name: "idx 1, signed", data: 0x0000_0001, index: 1, expected: 0xFFFF_FFFF}, + {name: "idx 1, unsigned", data: 0x0000_0000, index: 1, expected: 0x0000_0000}, + {name: "idx 2, signed", data: 0x0000_0002, index: 2, expected: 0xFFFF_FFFE}, + {name: "idx 2, unsigned", data: 0x0000_0001, index: 2, expected: 0x0000_0001}, + {name: "idx 4, signed", data: 0x0000_0008, index: 4, expected: 0xFFFF_FFF8}, + {name: "idx 4, unsigned", data: 0x0000_0005, index: 4, expected: 0x0000_0005}, + {name: "idx 8, signed", data: 0x0000_0092, index: 8, expected: 0xFFFF_FF92}, + {name: "idx 8, unsigned", data: 0x0000_0075, index: 8, expected: 0x0000_0075}, + {name: "idx 16, signed", data: 0x0000_A123, index: 16, expected: 0xFFFF_A123}, + {name: "idx 16, unsigned", data: 0x0000_7123, index: 16, expected: 0x0000_7123}, + {name: "idx 32, signed", data: 0x8123_4567, index: 32, expected: 0x8123_4567}, + {name: "idx 32, unsigned", data: 0x7123_4567, index: 32, expected: 0x7123_4567}, + {name: "idx 1, signed, nonzero upper bits", data: 0x1234_5671, index: 1, expected: 0xFFFF_FFFF}, + {name: "idx 1, unsigned, nonzero upper bits", data: 0x1234_567E, index: 1, expected: 0x0000_0000}, + {name: "idx 2, signed, nonzero upper bits", data: 0xABCD_EFE6, index: 2, expected: 0xFFFF_FFFE}, + {name: "idx 2, unsigned, nonzero upper bits", data: 0xABCD_EFED, index: 2, expected: 0x0000_0001}, + {name: "idx 4, signed, nonzero upper bits", data: 0x1230_0008, index: 4, expected: 0xFFFF_FFF8}, + {name: "idx 4, unsigned, nonzero upper bits", data: 0xFFF0_0005, index: 4, expected: 0x0000_0005}, + {name: "idx 8, signed, nonzero upper bits", data: 0x1111_1192, index: 8, expected: 0xFFFF_FF92}, + {name: "idx 8, unsigned, nonzero upper bits", data: 0xFFFF_FF75, index: 8, expected: 0x0000_0075}, + {name: "idx 16, signed, nonzero upper bits", data: 0x1234_A123, index: 16, expected: 0xFFFF_A123}, + {name: "idx 16, unsigned, nonzero upper bits", data: 0x1234_7123, index: 16, expected: 0x0000_7123}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual := SignExtend(c.data, c.index) + expected := signExtend64(c.expected) + require.Equal(t, expected, actual) + }) + } +} + +func signExtend64(w Word) Word { + // If bit at index 31 == 1, then sign extend the higher bits on 64-bit architectures + if !arch.IsMips32 && (w>>31)&1 == 1 { + upperBits := uint64(0xFFFF_FFFF_0000_0000) + return Word(upperBits) | w + } + return w +} diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index ddad1c7c64ce..eb7d69ecfa14 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -140,16 +140,16 @@ "sourceCodeHash": "0xf4f83ca89d2519045a2916c670bda66f39b431a13921e639a5342bfc6157b178" }, "src/cannon/MIPS.sol": { - "initCodeHash": "0x7d0fc7c7b51b74fa2611aaa8cc1a5967e2e48f0726ea894eb2c43f36b0ff2ab7", - "sourceCodeHash": "0xc18f51210c9d75f9bc33e55f879e8f0ab2514924718022264c0a2993134821e0" + "initCodeHash": "0x9d8a3c089fb84919159403a961fe0514d8be00f07b3a8be1a13a9289cc0ad11a", + "sourceCodeHash": "0x0c4805985e4c5b1c670a6ae057f5d330a1c3755216f61e38fe8b564a89b28da3" }, "src/cannon/MIPS2.sol": { - "initCodeHash": "0x4e1dbd0a6ac84873622af9234aca42e6e7b2bfda1186bbfd3ff83081f141ef86", - "sourceCodeHash": "0x62c820b22c72399efd7688dcf713c34a6ee6821835ec66d5e7b98f33bbbfb209" + "initCodeHash": "0x1f4e7cfdbcf7a8ca0ebac69bc7fe74143286a0e51a06ee9cbd699d68efd26dba", + "sourceCodeHash": "0x20256a2196daca39b56bfae1c90b8871349916dc47461b5ca078c2013c067571" }, "src/cannon/MIPS64.sol": { - "initCodeHash": "0x6c433a3fdba3af72d2d0399572e25b1b878ae53fb13cd237e2f4c1964b51644f", - "sourceCodeHash": "0x9d1af96777dc76b215aec7111c4fab2af4116dfefc734908e661ec641421dae2" + "initCodeHash": "0x7c957949e0fa8c20bd6ef2dca20e555ff58496a72f287dfee3acc87ae541ac6e", + "sourceCodeHash": "0x8ad227488b687501133454920f22c7c1e0261cf2e5641364d8b58ecebf270f86" }, "src/cannon/PreimageOracle.sol": { "initCodeHash": "0x17d3b3df1aaaf7a705b8d48de8a05e6511b910fdafdbe5eb7f7f95ec944fba9a", diff --git a/packages/contracts-bedrock/src/cannon/MIPS.sol b/packages/contracts-bedrock/src/cannon/MIPS.sol index bc976ee2667d..d29e5aeca4b4 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS.sol @@ -47,8 +47,8 @@ contract MIPS is ISemver { } /// @notice The semantic version of the MIPS contract. - /// @custom:semver 1.3.0-beta.1 - string public constant version = "1.3.0-beta.1"; + /// @custom:semver 1.3.0-beta.2 + string public constant version = "1.3.0-beta.2"; /// @notice The preimage oracle contract. IPreimageOracle internal immutable ORACLE; diff --git a/packages/contracts-bedrock/src/cannon/MIPS2.sol b/packages/contracts-bedrock/src/cannon/MIPS2.sol index 19c437466ed4..107edca117e5 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS2.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS2.sol @@ -61,8 +61,8 @@ contract MIPS2 is ISemver { } /// @notice The semantic version of the MIPS2 contract. - /// @custom:semver 1.0.0-beta.28 - string public constant version = "1.0.0-beta.28"; + /// @custom:semver 1.0.0-beta.29 + string public constant version = "1.0.0-beta.29"; /// @notice The preimage oracle contract. IPreimageOracle internal immutable ORACLE; diff --git a/packages/contracts-bedrock/src/cannon/MIPS64.sol b/packages/contracts-bedrock/src/cannon/MIPS64.sol index f206873fca1c..c8adc2680ce9 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS64.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS64.sol @@ -63,8 +63,8 @@ contract MIPS64 is ISemver { } /// @notice The semantic version of the MIPS64 contract. - /// @custom:semver 1.0.0-beta.1 - string public constant version = "1.0.0-beta.1"; + /// @custom:semver 1.0.0-beta.2 + string public constant version = "1.0.0-beta.2"; /// @notice The preimage oracle contract. IPreimageOracle internal immutable ORACLE; diff --git a/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol b/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol index f33a53705554..c2e323832ff5 100644 --- a/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol +++ b/packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol @@ -594,7 +594,7 @@ library MIPS64Instructions { /// @notice Extends the value leftwards with its most significant bit (sign extension). function signExtend(uint64 _dat, uint64 _idx) internal pure returns (uint64 out_) { unchecked { - bool isSigned = (_dat >> (_idx - 1)) != 0; + bool isSigned = (_dat >> (_idx - 1)) & 1 != 0; uint256 signed = ((1 << (arch.WORD_SIZE - _idx)) - 1) << _idx; uint256 mask = (1 << _idx) - 1; return uint64(_dat & mask | (isSigned ? signed : 0)); diff --git a/packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol b/packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol index 70ed5e20c3bc..b7b61c260dad 100644 --- a/packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol +++ b/packages/contracts-bedrock/src/cannon/libraries/MIPSInstructions.sol @@ -444,7 +444,7 @@ library MIPSInstructions { /// @notice Extends the value leftwards with its most significant bit (sign extension). function signExtend(uint32 _dat, uint32 _idx) internal pure returns (uint32 out_) { unchecked { - bool isSigned = (_dat >> (_idx - 1)) != 0; + bool isSigned = (_dat >> (_idx - 1)) & 1 != 0; uint256 signed = ((1 << (32 - _idx)) - 1) << _idx; uint256 mask = (1 << _idx) - 1; return uint32(_dat & mask | (isSigned ? signed : 0));