From 0e307a6afddbb564ea6c34b3766d749f80e4442a Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 12 Nov 2010 11:56:40 -0800 Subject: [PATCH] SimilarityIndex: Don't overflow internal counter fields The counter portion of each pair is only 32 bits wide, but is part of a larger 64 bit integer. If the file size was larger than 4 GB the counter could overflow and impact the key, changing the hash, and later resulting in an incorrect similarity score. Guard against this overflow condition by capping the count for each record at 2^32-1. If any record contains more than that many bytes the table aborts hashing and throws TableFullException. This permits the index to scan and work on files that exceed 4 GB in size, but only if the file contains more than one unique block. The index throws TableFullException on a 4 GB file containing all zeros, but should succeed on a 6 GB file containing unique lines. The index now uses a 64 bit accumulator during the common scoring algorithm, possibly resulting in slower summations. However this index is already heavily dependent upon 64 bit integer operations being efficient, so increasing from 32 bits to 64 bits allows us to correctly handle 6 GB files. Change-Id: I14e6dbc88d54ead19336a4c0c25eae18e73e6ec2 Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/diff/SimilarityIndex.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java index 045300613..17ccb9726 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java @@ -76,6 +76,9 @@ class SimilarityIndex { */ private static final int KEY_SHIFT = 32; + /** Maximum value of the count field, also mask to extract the count. */ + private static final long MAX_COUNT = (1L << KEY_SHIFT) - 1; + /** Total size of the file we hashed into the structure. */ private long fileSize; @@ -196,11 +199,11 @@ class SimilarityIndex { return (int) ((common(dst) * maxScore) / max); } - int common(SimilarityIndex dst) { + long common(SimilarityIndex dst) { return common(this, dst); } - private static int common(SimilarityIndex src, SimilarityIndex dst) { + private static long common(SimilarityIndex src, SimilarityIndex dst) { int srcIdx = src.packedIndex(0); int dstIdx = dst.packedIndex(0); long[] srcHash = src.idHash; @@ -208,12 +211,12 @@ class SimilarityIndex { return common(srcHash, srcIdx, dstHash, dstIdx); } - private static int common(long[] srcHash, int srcIdx, // + private static long common(long[] srcHash, int srcIdx, // long[] dstHash, int dstIdx) { if (srcIdx == srcHash.length || dstIdx == dstHash.length) return 0; - int common = 0; + long common = 0; int srcKey = keyOf(srcHash[srcIdx]); int dstKey = keyOf(dstHash[dstIdx]); @@ -287,13 +290,15 @@ class SimilarityIndex { j = slot(key); continue; } - idHash[j] = (((long) key) << KEY_SHIFT) | cnt; + idHash[j] = pair(key, cnt); idSize++; return; } else if (keyOf(v) == key) { - // Same key, increment the counter. - idHash[j] = v + cnt; + // Same key, increment the counter. If it overflows, fail + // indexing to prevent the key from being impacted. + // + idHash[j] = pair(key, countOf(v) + cnt); return; } else if (++j >= idHash.length) { @@ -302,6 +307,12 @@ class SimilarityIndex { } } + private static long pair(int key, long cnt) throws TableFullException { + if (MAX_COUNT < cnt) + throw new TableFullException(); + return (((long) key) << KEY_SHIFT) | cnt; + } + private int slot(int key) { // We use 31 - idHashBits because the upper bit was already forced // to be 0 and we want the remaining high bits to be used as the @@ -346,8 +357,8 @@ class SimilarityIndex { return (int) (v >>> KEY_SHIFT); } - private static int countOf(long v) { - return (int) v; + private static long countOf(long v) { + return v & MAX_COUNT; } static class TableFullException extends Exception {