Fix undefined behaviour in 128-bit integer-to-string conversion

Summary:
The code to convert signed 128-bit integer values to strings would trigger
undefined behaviour when trying to convert the most negative possible value,
as exposed by the new test cases.

The fix is to negate the corresponding unsigned value (just like it's already
done for the 64-bit code).

This change doesn't have any performance impact.

Reviewed By: yfeldblum

Differential Revision: D3455817

fbshipit-source-id: 83782992324f443789760a0e61cd9b889faaf317

diff --git a/folly/Conv.h b/folly/Conv.h
index 53be6b8211382dd6c93a089315f0ea6b14506c0c..25af3ea058b485bdaf46c226e42e0d76167b1435 100644 (file)
--- a/folly/Conv.h
+++ b/folly/Conv.h
@@ -461,7 +461,7 @@ toAppend(__int128 value, Tgt * result) {
   size_t p;
 
   if (value < 0) {
-    p = detail::unsafeTelescope128(buffer, sizeof(buffer), Usrc(-value));
+    p = detail::unsafeTelescope128(buffer, sizeof(buffer), -Usrc(value));
     buffer[--p] = '-';
   } else {
     p = detail::unsafeTelescope128(buffer, sizeof(buffer), value);

diff --git a/folly/test/ConvTest.cpp b/folly/test/ConvTest.cpp
index cbf7510ad87edbb1021f53a44ab3b40b1ea50cee..77594c8be0d66ee0f1bd5b4f3ad10ae32d831fec 100644 (file)
--- a/folly/test/ConvTest.cpp
+++ b/folly/test/ConvTest.cpp
@@ -197,6 +197,15 @@ void test128Bit2String() {
   svalue = 0;
   EXPECT_EQ(to<String>(svalue), "0");
 
+  value = ~__int128(0);
+  EXPECT_EQ(to<String>(value), "340282366920938463463374607431768211455");
+
+  svalue = -(Uint(1) << 127);
+  EXPECT_EQ(to<String>(svalue), "-170141183460469231731687303715884105728");
+
+  svalue = (Uint(1) << 127) - 1;
+  EXPECT_EQ(to<String>(svalue), "170141183460469231731687303715884105727");
+
   // TODO: the following do not compile to<__int128> ...
 
 #if 0