Skip to content

Commit 584418e

Browse files
jridgewelltjgq
authored andcommitted
There were two overflow cases hidden in the fromVLQSigned function:
1. Integer.MIN_VALUE would return 0, since -0 is just 0. All other negative numbers were fine. 2. Any number with the 31st bit set would be negated, because it used >> instead of >>>. So the 31st bit would be set to the 32nd (sign bit) when encoding, then remain at the 32nd bit when decoding. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=257839681
1 parent 224ba4b commit 584418e

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

src/com/google/debugging/sourcemap/Base64VLQ.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,16 @@ private static int toVLQSigned(int value) {
6161
*/
6262
private static int fromVLQSigned(int value) {
6363
boolean negate = (value & 1) == 1;
64-
value = value >> 1;
65-
return negate ? -value : value;
64+
value = value >>> 1;
65+
if (!negate) {
66+
return value;
67+
}
68+
// We need to OR 0x80000000 here to ensure the 32nd bit (the sign bit) is
69+
// always set for negative numbers. If `value` were 1, (meaning `negate` is
70+
// true and all other bits were zeros), `value` would now be 0. -0 is just
71+
// 0, and doesn't flip the 32nd bit as intended. All positive numbers will
72+
// successfully flip the 32nd bit without issue, so it's a noop for them.
73+
return -value | 0x80000000;
6674
}
6775

6876
/**

test/com/google/debugging/sourcemap/Base64VLQTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ public void testBase64VLQSelectedSignedValues2() {
6565
}
6666
}
6767

68+
@Test
69+
public void testBase64VLQSelectedSignedValues3() {
70+
testValue(Integer.MAX_VALUE);
71+
testValue(Integer.MAX_VALUE - 1);
72+
testValue(Integer.MAX_VALUE - 2);
73+
testValue(0x70000000);
74+
testValue(Integer.MIN_VALUE);
75+
testValue(Integer.MIN_VALUE + 1);
76+
testValue(Integer.MIN_VALUE + 2);
77+
}
78+
6879
static class CharIteratorImpl implements Base64VLQ.CharIterator {
6980
private int current;
7081
private int length;

0 commit comments

Comments
 (0)