Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] unalignment memory load in Neo.VM Unsafe.NotZero(ReadOnlySpan<byte>) and remove the use of unsafe #3492

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,5 @@ paket-files/
PublishProfiles
/.vscode
launchSettings.json
/coverages
**/.DS_Store
2 changes: 1 addition & 1 deletion src/Neo.VM/Types/ByteString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ internal bool Equals(StackItem? other, ref uint limits)
public override bool GetBoolean()
{
if (Size > Integer.MaxSize) throw new InvalidCastException();
return Unsafe.NotZero(GetSpan());
return GetSpan().NotZero();
Copy link
Member

Choose a reason for hiding this comment

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

The implementation is quite straightforward, but GetBoolean is frequently-used in VM, thus we have to check mainnet/testnet networks for compatibility. There should be no issues, but let's do it just in case.

Copy link
Member

Choose a reason for hiding this comment

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

@superboyiii could you check it?

}

public override int GetHashCode()
Expand Down
42 changes: 0 additions & 42 deletions src/Neo.VM/Unsafe.cs

This file was deleted.

11 changes: 11 additions & 0 deletions src/Neo.VM/Utility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ static Utility()
StrictUTF8.EncoderFallback = EncoderFallback.ExceptionFallback;
}

/// <summary>
/// All bytes are zero or not in a byte array
/// </summary>
/// <param name="x">The byte array</param>
/// <returns>false if all bytes are zero, true otherwise</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool NotZero(this ReadOnlySpan<byte> x)
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be moved to the extensions library. For ReadOnlySpans.cs

Copy link
Author

Choose a reason for hiding this comment

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

This needs to be moved to the extensions library. For ReadOnlySpans.cs

Move it with the rest of the content together?

Copy link
Member

Choose a reason for hiding this comment

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

Just that extension method

{
return x.IndexOfAnyExcept((byte)0) >= 0;
}

public static bool TryGetString(this ReadOnlySpan<byte> bytes, out string? value)
{
try
Expand Down
33 changes: 0 additions & 33 deletions tests/Neo.VM.Tests/UT_Unsafe.cs

This file was deleted.

31 changes: 31 additions & 0 deletions tests/Neo.VM.Tests/UT_Utility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,36 @@ public void TestModInverseTest()

Assert.AreEqual(new BigInteger(52), new BigInteger(19).ModInverse(141));
}

[TestMethod]
public void TestNotZero()
{
Assert.IsFalse(new ReadOnlySpan<byte>(System.Array.Empty<byte>()).NotZero());
Assert.IsFalse(new ReadOnlySpan<byte>(new byte[4]).NotZero());
Assert.IsFalse(new ReadOnlySpan<byte>(new byte[7]).NotZero());
Assert.IsFalse(new ReadOnlySpan<byte>(new byte[8]).NotZero());
Assert.IsFalse(new ReadOnlySpan<byte>(new byte[9]).NotZero());
Assert.IsFalse(new ReadOnlySpan<byte>(new byte[11]).NotZero());

Assert.IsTrue(new ReadOnlySpan<byte>(new byte[4] { 0x00, 0x00, 0x00, 0x01 }).NotZero());
Assert.IsTrue(new ReadOnlySpan<byte>(new byte[7] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }).NotZero());
Assert.IsTrue(new ReadOnlySpan<byte>(new byte[8] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }).NotZero());
Assert.IsTrue(new ReadOnlySpan<byte>(new byte[9] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00 }).NotZero());
Assert.IsTrue(new ReadOnlySpan<byte>(new byte[11] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }).NotZero());

var bytes = new byte[64];
for (int i = 0; i < bytes.Length; i++)
{
ReadOnlySpan<byte> span = bytes.AsSpan();
Assert.IsFalse(span[i..].NotZero());

for (int j = i; j < bytes.Length; j++)
{
bytes[j] = 0x01;
Assert.IsTrue(span[i..].NotZero());
bytes[j] = 0x00;
}
}
}
}
}