On 14/12/2016 18:12, Mike Williams wrote:
On 14/12/2016 11:16, Mike Williams wrote:
On 14/12/2016 08:44, Dominique Pellé wrote:
Yegappan Lakshmanan wrote:
eval.c:4432:10: runtime error: negation of -9223372036854775808 cannot
be represented in type 'varnumber_T' (aka 'long long'); cast to an
unsigned type to negate this value to itself
I see it too.
It's coming from this line in Test_num64() in test_viml.vim:
call assert_equal(-9223372036854775808, 0 / 0)
Running :echo -9223372036854775808
also gives the ubsan error.
viml parser is considering the number 9223372036854775808
(which is not a valid long long) and then negating it, instead
of considering it as number -9223372036854775808
(which is a valid long long). Not sure how it can be fix.
Even though it's undefined behavior, it probably works
on all platforms in practice. The test would probably
fail if it did not work. But it would be good to avoid undefined
behavior.
The usual idiom for writing an integer constant for the largest negative
value 2s complement integer is to negate the largest positive value and
subtract 1. So use (-9223372036854775807 - 1)?
Apologies. That just solves the issue with the test and not the
underlying problem. Caffeine took too long to kick in.
The cause is a combination of things. eval7() parses integers as
unsigned values but uses the signed value returned from vim_str2nr()
before applying any unary minus. In turn vim_str2nr() casts the
unsigned value of the integer to a signed value, so the value it returns
depends on whether the top bit is set from the unsigned calculation.
(Casting is lying to the compiler who will have its revenge one day -
looks like today is that day ;))
VIM doesn't signal (AFAIK) out of range integer values. At the moment,
with the wrapping unsigned integer math in vim_str2nr(), the result for
large integers is unpredictable (for those not concerned with integer
representation) Applying a saturating calculation would at least return
results as close as possible to the original integer value maintaining sign.
So the simplest change would be to have vim_str2nr() saturate and return
9223372036854775807 as the largest positive varnumber_T supported.
eval7() would then apply the unary minus and return -9223372036854775807
You will still need to use -9223372036854775807 - 1 in the original test
to generate the largest negative integer value possible in VIM.
Patch attached to limit range of parsed integer numbers, cope with 2s
complement asymmetry, and modify test that was triggering the UB
warning. Tests pass but I don't have clang to check the UB warning so
please test for that.
Mike
--
I couldn't repair your brakes, so I made your horn louder.
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
diff --git a/src/charset.c b/src/charset.c
--- a/src/charset.c
+++ b/src/charset.c
@@ -1901,7 +1901,10 @@ vim_str2nr(
n += 2; /* skip over "0b" */
while ('0' <= *ptr && *ptr <= '1')
{
- un = 2 * un + (unsigned long)(*ptr - '0');
+ if (un < UVARNUM_MAX / 2)
+ un = 2 * un + (unsigned long)(*ptr - '0');
+ else
+ un = UVARNUM_MAX;
++ptr;
if (n++ == maxlen)
break;
@@ -1912,7 +1915,10 @@ vim_str2nr(
/* octal */
while ('0' <= *ptr && *ptr <= '7')
{
- un = 8 * un + (uvarnumber_T)(*ptr - '0');
+ if (un < UVARNUM_MAX / 8)
+ un = 8 * un + (uvarnumber_T)(*ptr - '0');
+ else
+ un = UVARNUM_MAX;
++ptr;
if (n++ == maxlen)
break;
@@ -1925,7 +1931,10 @@ vim_str2nr(
n += 2; /* skip over "0x" */
while (vim_isxdigit(*ptr))
{
- un = 16 * un + (uvarnumber_T)hex2nr(*ptr);
+ if (un < UVARNUM_MAX / 16)
+ un = 16 * un + (uvarnumber_T)hex2nr(*ptr);
+ else
+ un = UVARNUM_MAX;
++ptr;
if (n++ == maxlen)
break;
@@ -1936,7 +1945,10 @@ vim_str2nr(
/* decimal */
while (VIM_ISDIGIT(*ptr))
{
- un = 10 * un + (uvarnumber_T)(*ptr - '0');
+ if (un < UVARNUM_MAX / 10)
+ un = 10 * un + (uvarnumber_T)(*ptr - '0');
+ else
+ un = UVARNUM_MAX;
++ptr;
if (n++ == maxlen)
break;
@@ -1950,9 +1962,18 @@ vim_str2nr(
if (nptr != NULL)
{
if (negative) /* account for leading '-' for decimal numbers */
- *nptr = -(varnumber_T)un;
+ {
+ if (un > VARNUM_MAX)
+ *nptr = VARNUM_MIN;
+ else
+ *nptr = -(varnumber_T)un;
+ }
else
+ {
+ if (un > VARNUM_MAX)
+ un = VARNUM_MAX;
*nptr = (varnumber_T)un;
+ }
}
if (unptr != NULL)
*unptr = un;
diff --git a/src/structs.h b/src/structs.h
--- a/src/structs.h
+++ b/src/structs.h
@@ -1133,25 +1133,43 @@ typedef long_u hash_T; /* Type for hi_h
# ifdef PROTO
typedef long varnumber_T;
typedef unsigned long uvarnumber_T;
+#define VARNUM_MIN LONG_MIN
+#define VARNUM_MAX LONG_MAX
+#define UVARNUM_MAX ULONG_MAX
# else
typedef __int64 varnumber_T;
typedef unsigned __int64 uvarnumber_T;
+#define VARNUM_MIN _I64_MIN
+#define VARNUM_MAX _I64_MAX
+#define UVARNUM_MAX _UI64_MAX
# endif
# elif defined(HAVE_STDINT_H)
typedef int64_t varnumber_T;
typedef uint64_t uvarnumber_T;
+#define VARNUM_MIN INT64_MIN
+#define VARNUM_MAX INT64_MAX
+#define UVARNUM_MAX UINT64_MAX
# else
typedef long varnumber_T;
typedef unsigned long uvarnumber_T;
+#define VARNUM_MIN LONG_MIN
+#define VARNUM_MAX LONG_MAX
+#define UVARNUM_MAX ULONG_MAX
# endif
#else
/* Use 32-bit Number. */
# if VIM_SIZEOF_INT <= 3 /* use long if int is smaller than 32 bits */
typedef long varnumber_T;
typedef unsigned long uvarnumber_T;
+#define VARNUM_MIN LONG_MIN
+#define VARNUM_MAX LONG_MAX
+#define UVARNUM_MAX ULONG_MAX
# else
typedef int varnumber_T;
typedef unsigned int uvarnumber_T;
+#define VARNUM_MIN INT_MIN
+#define VARNUM_MAX INT_MAX
+#define UVARNUM_MAX UINT_MAX
# endif
#endif
diff --git a/src/testdir/test_viml.vim b/src/testdir/test_viml.vim
--- a/src/testdir/test_viml.vim
+++ b/src/testdir/test_viml.vim
@@ -1226,7 +1226,7 @@ func Test_num64()
call assert_equal( 9223372036854775807, 1 / 0)
call assert_equal(-9223372036854775807, -1 / 0)
- call assert_equal(-9223372036854775808, 0 / 0)
+ call assert_equal(-9223372036854775807 - 1, 0 / 0)
call assert_equal( 0x7FFFffffFFFFffff, float2nr( 1.0e150))
call assert_equal(-0x7FFFffffFFFFffff, float2nr(-1.0e150))
