Skip to content

Commit 7fe3a11

Browse files
grendellogrendello
authored
[crc64] Fix a subtle bug in CRC64 splice-by-8 implementation (#627)
Commit 9b88ce7 implemented a splice-by-8 version of the CRC64 algorithm which processes 8 bytes per loop iteration. However, the code had a subtle bug which resulted in the same CRC calculated for different strings and made some of the Xamarin.Android tests to fail. The following strings yielded the same checksum value: * `obj/Debug/lp/10/jl/bin/classes.jar` * `obj/Debug/lp/11/jl/bin/classes.jar` * `obj/Debug/lp/12/jl/bin/classes.jar` The reason for this was subtle (a stupid oversight on my part, really): the loop fetched values from the input array by using an index into the array: crc ^= (ulong)aptr[idx]; However, with `aptr` being declared as `byte* aptr` the indexing operation returned a single *byte* instead of the required 64-bit word (8 bytes) and, thus, on each iteration of the loop 7 bytes of the input arrays were ignored in calculation, thus causing the collisions. The fix is to cast `aptr` to `ulong*` and *then* index it, extracting the required 8 bytes. This commit also adds a test to check for this issue.
1 parent 9b88ce7 commit 7fe3a11

File tree

5 files changed

+60
-22
lines changed

5 files changed

+60
-22
lines changed

‎src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/Crc64.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ unsafe protected override void HashCore (byte [] array, int ibStart, int cbSize)
6161
fixed (ulong* tptr = Table) {
6262
fixed (byte* aptr = array) {
6363
while (len >= 8) {
64-
crc ^= (ulong)aptr[idx];
64+
crc ^= *((ulong*)(aptr + idx));
6565
crc =
6666
tptr [7 * 256 + (crc & 0xff)] ^
6767
tptr [6 * 256 + ((crc >> 8) & 0xff)] ^

‎tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/Crc64Tests.cs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Text;
2+
using System.Security.Cryptography;
23
using Java.Interop.Tools.JavaCallableWrappers;
34
using NUnit.Framework;
45

@@ -34,13 +35,50 @@ public void Hello ()
3435
public void XmlDocument ()
3536
{
3637
var actual = ToHash ("System.Xml.XmlDocument, System.Xml");
37-
Assert.AreEqual ("b9c1bdfc7cd47543", actual);
38+
Assert.AreEqual ("348bbd9fecf1b865", actual);
3839
}
3940

4041
[Test]
4142
public void Collision ()
4243
{
4344
Assert.AreNotEqual (ToHash (""), ToHash (new byte [32]));
4445
}
46+
47+
[Test]
48+
public void AllBytesAreProcessed ()
49+
{
50+
// Slicing processes 8 bytes (a 64-bit word) at a time, and if any of the bytes are skipped we will have a
51+
// collision here.
52+
string[] inputs = {
53+
"obj/Debug/lp/10/jl/bin/classes.jar",
54+
"obj/Debug/lp/11/jl/bin/classes.jar",
55+
"obj/Debug/lp/12/jl/bin/classes.jar",
56+
};
57+
58+
string[] expected = {
59+
"419a37c9bcfddf3c",
60+
"6ea5e242b7cc24a7",
61+
"74770a86f8b97020",
62+
};
63+
64+
string[] outputs = new string[inputs.Length];
65+
66+
for (int i = 0; i < inputs.Length; i++) {
67+
byte[] bytes = Encoding.UTF8.GetBytes (inputs [i]);
68+
using (HashAlgorithm hashAlg = new Crc64 ()) {
69+
byte [] hash = hashAlg.ComputeHash (bytes);
70+
outputs[i] = ToHash (hash);
71+
Assert.AreEqual (expected[i], outputs[i], $"hash {i} differs");
72+
}
73+
}
74+
75+
for (int i = 0; i < outputs.Length; i++) {
76+
for (int j = 0; j < outputs.Length; j++) {
77+
if (j == i)
78+
continue;
79+
Assert.AreNotEqual (outputs[i], outputs[j], $"Outputs {i} and {j} are identical");
80+
}
81+
}
82+
}
4583
}
4684
}

‎tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public void GenerateIndirectApplication (
124124
)
125125
{
126126
var actual = Generate (typeof (IndirectApplication), applicationJavaClass);
127-
var expected = @"package c64r2b57424eba1fa9d27;
127+
var expected = @"package c64r2197ae30a36756915;
128128
129129
130130
public class IndirectApplication
@@ -175,7 +175,7 @@ public void monodroidClearReferences ()
175175
public void GenerateExportedMembers ()
176176
{
177177
var actual = Generate (typeof (ExportsMembers));
178-
var expected = @"package c64r2b57424eba1fa9d27;
178+
var expected = @"package c64r2197ae30a36756915;
179179
180180
181181
public class ExportsMembers
@@ -187,7 +187,7 @@ extends java.lang.Object
187187
public static final String __md_methods;
188188
static {
189189
__md_methods =
190-
""n_GetInstance:()Lc64r2b57424eba1fa9d27/ExportsMembers;:__export__\n"" +
190+
""n_GetInstance:()Lc64r2197ae30a36756915/ExportsMembers;:__export__\n"" +
191191
""n_GetValue:()Ljava/lang/String;:__export__\n"" +
192192
""n_methodNamesNotMangled:()V:__export__\n"" +
193193
""n_CompletelyDifferentName:(Ljava/lang/String;I)Ljava/lang/String;:__export__\n"" +
@@ -198,17 +198,17 @@ extends java.lang.Object
198198
}
199199
200200
201-
public static c64r2b57424eba1fa9d27.ExportsMembers STATIC_INSTANCE = GetInstance ();
201+
public static c64r2197ae30a36756915.ExportsMembers STATIC_INSTANCE = GetInstance ();
202202
203203
204204
public java.lang.String VALUE = GetValue ();
205205
206-
public static c64r2b57424eba1fa9d27.ExportsMembers GetInstance ()
206+
public static c64r2197ae30a36756915.ExportsMembers GetInstance ()
207207
{
208208
return n_GetInstance ();
209209
}
210210
211-
private static native c64r2b57424eba1fa9d27.ExportsMembers n_GetInstance ();
211+
private static native c64r2197ae30a36756915.ExportsMembers n_GetInstance ();
212212
213213
public java.lang.String GetValue ()
214214
{
@@ -271,7 +271,7 @@ public void monodroidClearReferences ()
271271
public void GenerateInnerClass ()
272272
{
273273
var actual = Generate (typeof (ExampleOuterClass));
274-
var expected = @"package c64r2b57424eba1fa9d27;
274+
var expected = @"package c64r2197ae30a36756915;
275275
276276
277277
public class ExampleOuterClass

‎tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaNativeTypeManagerTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public void TearDown ()
2929
public void Crc64 ()
3030
{
3131
JavaNativeTypeManager.PackageNamingPolicy = PackageNamingPolicy.LowercaseCrc64;
32-
Assert.AreEqual ("c64r279bf423bcb581100", JavaNativeTypeManager.GetPackageName (typeof (string)));
32+
Assert.AreEqual ("c64r2b74743e9328eed0a", JavaNativeTypeManager.GetPackageName (typeof (string)));
3333
}
3434

3535
[Test]

‎tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGeneratorTests.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@ public void WriteJavaToManaged ()
5959
"value-offset=" + offset + "\u0000" +
6060
GetJ2MEntryLine (typeof (ActivityName), "activity/Name", offset, length) +
6161
GetJ2MEntryLine (typeof (ApplicationName), "application/Name", offset, length) +
62-
GetJ2MEntryLine (typeof (DefaultName), "c64r2b57424eba1fa9d27/DefaultName", offset, length) +
63-
GetJ2MEntryLine (typeof (DefaultName.A), "c64r2b57424eba1fa9d27/DefaultName_A", offset, length) +
64-
GetJ2MEntryLine (typeof (DefaultName.A.B), "c64r2b57424eba1fa9d27/DefaultName_A_B", offset, length) +
65-
GetJ2MEntryLine (typeof (DefaultName.C.D), "c64r2b57424eba1fa9d27/DefaultName_C_D", offset, length) +
66-
GetJ2MEntryLine (typeof (ExampleOuterClass), "c64r2b57424eba1fa9d27/ExampleOuterClass", offset, length) +
67-
GetJ2MEntryLine (typeof (ExampleOuterClass.ExampleInnerClass), "c64r2b57424eba1fa9d27/ExampleOuterClass$ExampleOuterClass_ExampleInnerClass", offset, length) +
62+
GetJ2MEntryLine (typeof (DefaultName), "c64r2197ae30a36756915/DefaultName", offset, length) +
63+
GetJ2MEntryLine (typeof (DefaultName.A), "c64r2197ae30a36756915/DefaultName_A", offset, length) +
64+
GetJ2MEntryLine (typeof (DefaultName.A.B), "c64r2197ae30a36756915/DefaultName_A_B", offset, length) +
65+
GetJ2MEntryLine (typeof (DefaultName.C.D), "c64r2197ae30a36756915/DefaultName_C_D", offset, length) +
66+
GetJ2MEntryLine (typeof (ExampleOuterClass), "c64r2197ae30a36756915/ExampleOuterClass", offset, length) +
67+
GetJ2MEntryLine (typeof (ExampleOuterClass.ExampleInnerClass), "c64r2197ae30a36756915/ExampleOuterClass$ExampleOuterClass_ExampleInnerClass", offset, length) +
6868
GetJ2MEntryLine (typeof (InstrumentationName), "instrumentation/Name", offset, length) +
6969
GetJ2MEntryLine (typeof (AbstractClass), "my/AbstractClass", offset, length) +
7070
GetJ2MEntryLine (typeof (ExampleActivity), "my/ExampleActivity", offset, length) +
@@ -139,14 +139,14 @@ public void WriteManagedToJava ()
139139
GetM2JEntryLine (typeof (AbstractClassInvoker), "my/AbstractClass", offset, length) +
140140
GetM2JEntryLine (typeof (ActivityName), "activity/Name", offset, length) +
141141
GetM2JEntryLine (typeof (ApplicationName), "application/Name", offset, length) +
142-
GetM2JEntryLine (typeof (DefaultName.A.B), "c64r2b57424eba1fa9d27/DefaultName_A_B", offset, length) +
143-
GetM2JEntryLine (typeof (DefaultName.A), "c64r2b57424eba1fa9d27/DefaultName_A", offset, length) +
144-
GetM2JEntryLine (typeof (DefaultName.C.D), "c64r2b57424eba1fa9d27/DefaultName_C_D", offset, length) +
145-
GetM2JEntryLine (typeof (DefaultName), "c64r2b57424eba1fa9d27/DefaultName", offset, length) +
142+
GetM2JEntryLine (typeof (DefaultName.A.B), "c64r2197ae30a36756915/DefaultName_A_B", offset, length) +
143+
GetM2JEntryLine (typeof (DefaultName.A), "c64r2197ae30a36756915/DefaultName_A", offset, length) +
144+
GetM2JEntryLine (typeof (DefaultName.C.D), "c64r2197ae30a36756915/DefaultName_C_D", offset, length) +
145+
GetM2JEntryLine (typeof (DefaultName), "c64r2197ae30a36756915/DefaultName", offset, length) +
146146
GetM2JEntryLine (typeof (ExampleActivity), "my/ExampleActivity", offset, length) +
147147
GetM2JEntryLine (typeof (ExampleInstrumentation), "my/ExampleInstrumentation", offset, length) +
148-
GetM2JEntryLine (typeof (ExampleOuterClass.ExampleInnerClass), "c64r2b57424eba1fa9d27/ExampleOuterClass$ExampleOuterClass_ExampleInnerClass", offset, length) +
149-
GetM2JEntryLine (typeof (ExampleOuterClass), "c64r2b57424eba1fa9d27/ExampleOuterClass", offset, length) +
148+
GetM2JEntryLine (typeof (ExampleOuterClass.ExampleInnerClass), "c64r2197ae30a36756915/ExampleOuterClass$ExampleOuterClass_ExampleInnerClass", offset, length) +
149+
GetM2JEntryLine (typeof (ExampleOuterClass), "c64r2197ae30a36756915/ExampleOuterClass", offset, length) +
150150
GetM2JEntryLine (typeof (InstrumentationName), "instrumentation/Name", offset, length) +
151151
GetM2JEntryLine (typeof (NonStaticOuterClass.NonStaticInnerClass), "register/NonStaticOuterClass$NonStaticInnerClass", offset, length) +
152152
GetM2JEntryLine (typeof (NonStaticOuterClass), "register/NonStaticOuterClass", offset, length) +

0 commit comments

Comments
 (0)