8

I had a bug that caused an integer overflow, resulting in wrong (negative) timestamps being written to the database. The code is fixed already, but I want to fix the wrong data, too.

I thought, I could just take the wrong results and add Integer.MAX_VALUE, but that didn't seem to work, it left me with to high values. I have the offset value in the code snippet below, but the input values are not stored.

The following code reproduces the bug:

@Test
public void testArexxConversion()
{
    // The input values represent seconds since midnight, Jan 1, 2000 UTC
    final int sample = 361450072; // A sample input value drawn from production
    // I use the offset from the UNIX epoch to convert the vakue to UNIX seconds
    final int offset = 946684800; // midnight, Jan 01 2000 UTC in UNIX seconds
    // This was the buggy line in my code, the assertion will fail
    long result = (sample + offset) * 1000;
    // Prints 'Result is negative: -1830153280'
    Assert.assertTrue(result > 0, String.format("Result is negative: %d", result));
    // This is for comparison
    Date dt = new Date(offset * 1000);
    Assert.assertEquals(dt.getTime() + sample * 1000, result);
}
Hanno Fietz
  • 30,799
  • 47
  • 148
  • 234
  • Also, why does the overflow happen in the first place? Adding the two numbers above does not overflow, it's the multiplication, and I sort of expected that this would already be done on a `long` since that's the result type. – Hanno Fietz Jun 15 '11 at 11:18

3 Answers3

5

How to fix the bug in your database

To fix the bug in your database you can do the following addition to all the buggy data:

long new_result = old_buggy_result + 1309965025280L;

The constant number was found like this:

  1. Check the buggy result value
  2. Find what should the correct result value be?
  3. Do an addition to the buggy result value to find the correct `result.

But this is only possible if you have saved sample and offset in your database or somewhere else.

Otherwise, it depends on the number of wraps that occured during the original calculation:

long size_of_int = (long)Math.pow(2, 32);
int number_of_wraps = 305 // Only correct in your example!
                          // You can't deduct the number of wraps from
                          // the wrong value alone, because that information
                          // is lost in the modulo (the "wrap")
long correct_number = wrong_number + size_of_int * number_of_wraps;

If the numbers in your database are close enough to your sample value, this means, you can do the above, using 305 as the number of wraps.

Explanation of the bug (for future readers)

The operation here:

 (sample + offset) * 1000;

is computed using int and not long. But the result is "too big" to be saved on an int variable. That's why you have an overflow.

Change it to:

  ((long) sample + offset) * 1000L;

So now the + and * operations will be done using long values, and the result will be a long value which won't overflow.

Hanno Fietz
  • 30,799
  • 47
  • 148
  • 234
insumity
  • 5,311
  • 8
  • 36
  • 64
  • 1
    Thanks, but the bug is already fixed. My question was: How do I fix the wrong numbers already in the database? – Hanno Fietz Jun 15 '11 at 11:22
  • @Luzhin - thanks for the update. I have the offset value, since it is fixed, but the input values were not retained, otherwise I would just run them through the fixed code. – Hanno Fietz Jun 15 '11 at 11:40
  • @Luzhin I see. Well it depends on the number of wraps around's that have been done when you used `+` and `*`. If the numbers in your database are near this sample value: `361450072`, e.g. `361450972`, `361451290`. You could somehting like the following: ` long diff = (long)Integer.MAX_VALUE - Integer.MIN_VALUE + 1; long valid_result = buggy_result + 305 * diff;` – insumity Jun 15 '11 at 11:54
  • @Luzhin - Thanks, that should help, it also helped me understand what actually happened during the overflow. The wrong results are close enough to have the same number of wraps. I'll edit that into the answer and accept it. – Hanno Fietz Jun 15 '11 at 11:59
  • @Hanno Fietz .. Nice! Thank you :) – insumity Jun 15 '11 at 12:14
2

That would be like this:

long result = ... ; // bad negative from database
long new_result = (long)((int)result - Integer.MAX_VALUE) + Integer.MAX_VALUE;
Gleb Varenov
  • 2,795
  • 3
  • 18
  • 18
  • This will force the integer to overflow in reverse, leaving the amount above the MAX_VALUE and add the MAX_VALUE again to get the correct value, right? Probably more elegant than my solution: new_result = ( ( 2L + max - (old_buggy_result * -1L) ) + max); //doing essentially the same using non-overflowing arithmetic. – Stein G. Strindhaug Jun 15 '11 at 12:20
  • 1
    This will only work if it overflowed only once. But, as far as I can see, this buggy code will not overflow more than once – Stein G. Strindhaug Jun 15 '11 at 12:22
  • 1
    The buggy code overflowed 305 times for the sample value, as pointed out in the answer I accepted. The actual values were close enough to have the same number of wraps. – Hanno Fietz Jun 15 '11 at 12:26
  • Yes, this will work if overflow happened only once. For custom number of overflows (which could not be deducted correctly in general, anyway) the formula is: `long new_result = (long)((int)result - Integer.MAX_VALUE) + Integer.MAX_VALUE + (long) 2< – Gleb Varenov Jun 16 '11 at 12:02
0

Replace this line.

long result = (long)(sample + offset) * 1000L;
Talha Ahmed Khan
  • 15,043
  • 10
  • 42
  • 49
  • 1
    Thanks, but the bug is already fixed. My question was: How do I fix the wrong numbers already in the database? – Hanno Fietz Jun 15 '11 at 11:22
  • The cast to long is not needed. – jzd Jun 15 '11 at 11:29
  • @Hanno, it is not exactly clear until I read your comment here, that you are after a fix for the bad data. Maybe you could edit your question to specifically say that. – jzd Jun 15 '11 at 11:30