1203389 - TimeStampValue comparator operators on Windows do not handle overflow

Status: NEW

(Core :: mozglue, defect)

Description

[Brian Birtles (:birtles)]

I went to write a test as part of bug 1203351 to check we detect underflow when doing timestamp addition/subtraction but ran into trouble because on Windows TimeStampValue::operator< and friends don't return the correct result when CheckQPC encounters overflow.

Specifically, the test I went to write was:

    ts -= TimeDuration::Forever();
    EXPECT_TRUE(ts.IsNull());

However, what happens is that in operator -= we perform the following check (as of bug 1202556):

    TimeStampValue value = mValue - aOther.mValue;
    // Check for underflow/overflow.
    if ((aOther.mValue > 0 && value > mValue) ||
        ....

The second part, value > mValue, *should* return true since 'value' has wrapped. We have:

  value:
  mGTC     9227401781218472887
  mQPC     9227401939217279809
  mHasQPC  true
  mIsNull  false

  mValue:
  mGTC     4029744363697078
  mQPC     4029902362504000
  mHasQPC  true
  mIsNull  false

However, operator> here does the following:

  return int64_t(*this - aOther) > 0;

i.e. it calls operator- which calls CheckQPC. CheckQPC returns a uint64_t of the difference. In this case the result will be:

  value.mQPC - mValue.QPC
  = 4029902362504000 - 9227401939217279809
  = 9223372036854775809 or so since it underflows

Casting to int64_t gives -9223372036854775807

Hence operator> returns false, despite the fact that mQPC and mGTC and clearly greater.

I guess we need a version of CheckQPC that just does comparison and avoids overflow.

But that begs the question, what should CheckQPC (and operator-) return if they encounter overflow. Perhaps null?

Comment 1

[Brian Birtles (:birtles)]

I was also thinking that perhaps we could get CheckQPC to simply return 0 when it detects overflow.