From: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Tue, 25 Jun 2019 21:44:32 +0000 (-0400)
Subject: Fix: bt2: erroneous integer comparison of Field and Value
X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=7bb4180f2758d78af3f4d539f6b3e4e1fa60335f

Fix: bt2: erroneous integer comparison of Field and Value

Issue
=====
The `{Unsigned, Signed}IntegerValue` class inherit its __eq__() method
from the `_NumericValue` class. This method converts the `other` object
to a `complex` number to make the comparison as generic as possible. The
issue is that the large complex number comparison is unreliable as it's
using float comparison[1]. The timestamps we are dealing with will often
be going beyond the limit of exact comparison possible with the IEEE 754
double-precision[2].

I encountered this issue while comparing a
`bt2.value.SignedIntegerValue` to a large Python Integer representing a
timestamp in nanosecond.

Here is a showcase of the issue reproduced without the `bt2` bindings:
  In [1]: 42 == complex(42)
  Out[1]: True

  In [2]: 1561489497327275497 == complex(1561489497327275497)
  Out[2]: False

The same issue is reproducible with the `_{Unsigned,
Signed}IntegerField`.

Solution
========
Implement the __eq__() and __lt__() methods in the `_IntegralValue`
class and but still use `_NumericValue`'s methods if the `other` object
is not an instance of `numbers.Integral`.

Implement the __eq__() and __lt__() methods in the `_IntegralField`
class and but still use `_NumericField`'s methods if the `other` object
is not an instance of `numbers.Integral`.

Drawback
========
None.

Note
====
I added test cases to cover large integer comparison for both Integer Value and
integer Field classes.

[1]: https://stackoverflow.com/questions/5595425/what-is-the-best-way-to-compare-floats-for-almost-equality-in-python
[2]: https://en.wikipedia.org/wiki/Double-precision_floating-point_format

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I8448bd42704ad1a2d215bece8534c839b9468f25
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1538
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
---

diff --git a/src/bindings/python/bt2/bt2/field.py b/src/bindings/python/bt2/bt2/field.py
index 99daff5a..f5b9b596 100644
--- a/src/bindings/python/bt2/bt2/field.py
+++ b/src/bindings/python/bt2/bt2/field.py
@@ -250,6 +250,18 @@ class _IntegralField(_NumericField, numbers.Integral):
         self.value = self | other
         return self
 
+    def __lt__(self, other):
+        if not isinstance(other, numbers.Integral):
+            return super().__lt__(other);
+
+        return self._value < int(other)
+
+    def _spec_eq(self, other):
+        if not isinstance(other, numbers.Integral):
+            return super()._spec_eq(other);
+
+        return self._value == int(other)
+
 
 class _IntegerField(_IntegralField, _Field):
     pass
diff --git a/src/bindings/python/bt2/bt2/value.py b/src/bindings/python/bt2/bt2/value.py
index 22623c22..c5c23fe6 100644
--- a/src/bindings/python/bt2/bt2/value.py
+++ b/src/bindings/python/bt2/bt2/value.py
@@ -312,6 +312,18 @@ class _IntegralValue(_NumericValue, numbers.Integral):
         self.value = self | other
         return self
 
+    def __lt__(self, other):
+        if not isinstance(other, numbers.Integral):
+            return super().__lt__(other)
+
+        return self._value < int(other)
+
+    def __eq__(self, other):
+        if not isinstance(other, numbers.Integral):
+            return super().__eq__(other)
+
+        return self._value == int(other)
+
 
 class _RealValue(_NumericValue, numbers.Real):
     pass
diff --git a/tests/bindings/python/bt2/test_field.py b/tests/bindings/python/bt2/test_field.py
index 61eeb933..f6fc4ca3 100644
--- a/tests/bindings/python/bt2/test_field.py
+++ b/tests/bindings/python/bt2/test_field.py
@@ -836,6 +836,15 @@ class _TestIntegerFieldCommon(_TestNumericField):
         field.value = 1777
         self.assertEqual(field, raw)
 
+    def test_assign_big_uint(self):
+        uint_fc = self._tc.create_unsigned_integer_field_class(64)
+        field = _create_field(self._tc, uint_fc)
+        # Larger than the IEEE 754 double-precision exact representation of
+        # integers.
+        raw = (2**53) + 1
+        field.value = (2**53) + 1
+        self.assertEqual(field, raw)
+
     def test_assign_uint_invalid_neg(self):
         uint_fc = self._tc.create_unsigned_integer_field_class(32)
         field = _create_field(self._tc, uint_fc)
diff --git a/tests/bindings/python/bt2/test_value.py b/tests/bindings/python/bt2/test_value.py
index 51e94895..8f886c72 100644
--- a/tests/bindings/python/bt2/test_value.py
+++ b/tests/bindings/python/bt2/test_value.py
@@ -1031,6 +1031,13 @@ class SignedIntegerValueTestCase(_TestIntegerValue, unittest.TestCase):
         self._def.value = raw
         self.assertEqual(self._def, raw)
 
+    def test_compare_big_int(self):
+        # Larger than the IEEE 754 double-precision exact representation of
+        # integers.
+        raw = (2**53) + 1
+        v = bt2.create_value(raw)
+        self.assertEqual(v, raw)
+
 
 _inject_numeric_testing_methods(SignedIntegerValueTestCase)