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 9 commits into
base: master
Choose a base branch
from
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 @@
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
Contributor 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

Copy link
Member

@cschuchardt88 cschuchardt88 Sep 23, 2024

Choose a reason for hiding this comment

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

Would like this method to be moved to Neo.Extensions library

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

Check failure on line 38 in src/Neo.VM/Utility.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ReadOnlySpan<byte>' does not contain a definition for 'IndexOfAnyExcept' and no accessible extension method 'IndexOfAnyExcept' accepting a first argument of type 'ReadOnlySpan<byte>' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 38 in src/Neo.VM/Utility.cs

View workflow job for this annotation

GitHub Actions / Test-Everything

'ReadOnlySpan<byte>' does not contain a definition for 'IndexOfAnyExcept' and no accessible extension method 'IndexOfAnyExcept' accepting a first argument of type 'ReadOnlySpan<byte>' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 38 in src/Neo.VM/Utility.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

'ReadOnlySpan<byte>' does not contain a definition for 'IndexOfAnyExcept' and no accessible extension method 'IndexOfAnyExcept' accepting a first argument of type 'ReadOnlySpan<byte>' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 38 in src/Neo.VM/Utility.cs

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

'ReadOnlySpan<byte>' does not contain a definition for 'IndexOfAnyExcept' and no accessible extension method 'IndexOfAnyExcept' accepting a first argument of type 'ReadOnlySpan<byte>' could be found (are you missing a using directive or an assembly reference?)
}

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;
}
}
}
}
}
Loading