From 6b6a4d63ec585badcd69890608bc144ef4d89af7 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Tue, 8 Jan 2019 00:22:57 +0100 Subject: [PATCH 1/3] assert.h: Check overflow with STRICT_ADD, STRICT_SUB --- src/nvim/assert.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/nvim/assert.h b/src/nvim/assert.h index 761636305e..48c5363d5a 100644 --- a/src/nvim/assert.h +++ b/src/nvim/assert.h @@ -15,6 +15,10 @@ # define __has_extension __has_feature #endif +#ifndef __has_builtin +# define __has_builtin __has_feature +#endif + /// @def STATIC_ASSERT /// @brief Assert at compile time if condition is not satisfied. /// @@ -121,4 +125,31 @@ ((enum { ASSERT_CONCAT(assert_line_, __LINE__) = 1/(!!(e)) }) 0) #endif +/// @def STRICT_ADD +/// +/// Requires GCC 5+ and Clang 3.8+ +/// https://clang.llvm.org/docs/LanguageExtensions.html +/// https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html +/// +/// Alternative for compilers without __builtin_xx_overflow ? +/// https://stackoverflow.com/a/44830670/152142 +/// +/// @param MAX Maximum value of the narrowest type of operand. +/// Not used if compiler supports __builtin_add_overflow. +#if __has_builtin(__builtin_add_overflow) +# define STRICT_ADD(a, b, c) \ + do { if (__builtin_add_overflow(a, b, c)) { abort(); } } while (0) +#else +# define STRICT_ADD(a, b, c) \ + do { *c = a + b; } while (0) +#endif + +#if __has_builtin(__builtin_sub_overflow) +# define STRICT_SUB(a, b, c) \ + do { if (__builtin_sub_overflow(a, b, c)) { abort(); } } while (0) +#else +# define STRICT_SUB(a, b, c) \ + do { *c = a - b; } while (0) +#endif + #endif // NVIM_ASSERT_H From 596f020e904b2da2dac1bc080eb69f66deb998d7 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sat, 5 Jan 2019 00:57:27 +0100 Subject: [PATCH 2/3] PVS/V1028: cast operands, not the result --- src/nvim/assert.h | 22 ++++++++++------------ src/nvim/cursor.c | 14 ++++++++++---- src/nvim/indent.c | 16 +++++++++++++--- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/nvim/assert.h b/src/nvim/assert.h index 48c5363d5a..3f985475a7 100644 --- a/src/nvim/assert.h +++ b/src/nvim/assert.h @@ -15,10 +15,6 @@ # define __has_extension __has_feature #endif -#ifndef __has_builtin -# define __has_builtin __has_feature -#endif - /// @def STATIC_ASSERT /// @brief Assert at compile time if condition is not satisfied. /// @@ -136,20 +132,22 @@ /// /// @param MAX Maximum value of the narrowest type of operand. /// Not used if compiler supports __builtin_add_overflow. -#if __has_builtin(__builtin_add_overflow) -# define STRICT_ADD(a, b, c) \ +#if (defined(__clang__) && __has_builtin(__builtin_add_overflow)) \ + || (__GNUC__ >= 5) +# define STRICT_ADD(a, b, c, t) \ do { if (__builtin_add_overflow(a, b, c)) { abort(); } } while (0) #else -# define STRICT_ADD(a, b, c) \ - do { *c = a + b; } while (0) +# define STRICT_ADD(a, b, c, t) \ + do { *(c) = (t)(a + b); } while (0) #endif -#if __has_builtin(__builtin_sub_overflow) -# define STRICT_SUB(a, b, c) \ +#if (defined(__clang__) && __has_builtin(__builtin_sub_overflow)) \ + || (__GNUC__ >= 5) +# define STRICT_SUB(a, b, c, t) \ do { if (__builtin_sub_overflow(a, b, c)) { abort(); } } while (0) #else -# define STRICT_SUB(a, b, c) \ - do { *c = a - b; } while (0) +# define STRICT_SUB(a, b, c, t) \ + do { *(c) = (t)(a - b); } while (0) #endif #endif // NVIM_ASSERT_H diff --git a/src/nvim/cursor.c b/src/nvim/cursor.c index ee1bc6cf31..409eb653a0 100644 --- a/src/nvim/cursor.c +++ b/src/nvim/cursor.c @@ -4,6 +4,7 @@ #include #include +#include "nvim/assert.h" #include "nvim/cursor.h" #include "nvim/charset.h" #include "nvim/fold.h" @@ -170,7 +171,9 @@ static int coladvance2( if (line[idx] == NUL) { /* Append spaces */ int correct = wcol - col; - char_u *newline = xmallocz((size_t)(idx + correct)); + size_t newline_size; + STRICT_ADD(idx, correct, &newline_size, size_t); + char_u *newline = xmallocz(newline_size); memcpy(newline, line, (size_t)idx); memset(newline + idx, ' ', (size_t)correct); @@ -187,14 +190,17 @@ static int coladvance2( if (-correct > csize) return FAIL; - newline = xmallocz((size_t)(linelen - 1 + csize)); + size_t n; + STRICT_ADD(linelen - 1, csize, &n, size_t); + newline = xmallocz(n); // Copy first idx chars memcpy(newline, line, (size_t)idx); // Replace idx'th char with csize spaces memset(newline + idx, ' ', (size_t)csize); // Copy the rest of the line - memcpy(newline + idx + csize, line + idx + 1, - (size_t)(linelen - idx - 1)); + STRICT_SUB(linelen, idx, &n, size_t); + STRICT_SUB(n, 1, &n, size_t); + memcpy(newline + idx + csize, line + idx + 1, n); ml_replace(pos->lnum, newline, false); changed_bytes(pos->lnum, idx); diff --git a/src/nvim/indent.c b/src/nvim/indent.c index 7c3f354c13..13534ac1a9 100644 --- a/src/nvim/indent.c +++ b/src/nvim/indent.c @@ -6,6 +6,7 @@ #include #include "nvim/ascii.h" +#include "nvim/assert.h" #include "nvim/indent.h" #include "nvim/eval.h" #include "nvim/charset.h" @@ -204,7 +205,12 @@ int set_indent(int size, int flags) // after the if (!curbuf->b_p_et) below. if (orig_char_len != -1) { assert(orig_char_len + size - ind_done + line_len >= 0); - newline = xmalloc((size_t)(orig_char_len + size - ind_done + line_len)); + size_t n; // = orig_char_len + size - ind_done + line_len + size_t n2; + STRICT_ADD(orig_char_len, size, &n, size_t); + STRICT_ADD(ind_done, line_len, &n2, size_t); + STRICT_SUB(n, n2, &n, size_t); + newline = xmalloc(n); todo = size - ind_done; // Set total length of indent in characters, which may have been @@ -226,7 +232,9 @@ int set_indent(int size, int flags) } else { todo = size; assert(ind_len + line_len >= 0); - newline = xmalloc((size_t)(ind_len + line_len)); + size_t newline_size; + STRICT_ADD(ind_len, line_len, &newline_size, size_t); + newline = xmalloc(newline_size); s = newline; } @@ -392,7 +400,9 @@ int copy_indent(int size, char_u *src) // and the rest of the line. line_len = (int)STRLEN(get_cursor_line_ptr()) + 1; assert(ind_len + line_len >= 0); - line = xmalloc((size_t)(ind_len + line_len)); + size_t line_size; + STRICT_ADD(ind_len, line_len, &line_size, size_t); + line = xmalloc(line_size); p = line; } } From fc4ca5bdd8c5a2b37b6efe34a9b32a1bd75c57af Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Tue, 8 Jan 2019 22:00:52 +0100 Subject: [PATCH 3/3] CMake: Feature-detect __builtin_{add,sub}_overflow --- CMakeLists.txt | 10 ++++++++++ src/nvim/assert.h | 9 +++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e73443ec2..8272a1a469 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -248,6 +248,16 @@ int main(void) } " HAVE_EXECINFO_BACKTRACE) +check_c_source_compiles(" +int main(void) +{ + int a; + __builtin_add_overflow(a, a, &a); + __builtin_sub_overflow(a, a, &a); + return 0; +} +" HAVE_BUILTIN_ADD_OVERFLOW) + if(MSVC) # XXX: /W4 gives too many warnings. #3241 add_definitions(/W3 -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE) diff --git a/src/nvim/assert.h b/src/nvim/assert.h index 3f985475a7..29195a49dc 100644 --- a/src/nvim/assert.h +++ b/src/nvim/assert.h @@ -122,6 +122,7 @@ #endif /// @def STRICT_ADD +/// @brief Adds (a + b) and stores result in `c`. Aborts on overflow. /// /// Requires GCC 5+ and Clang 3.8+ /// https://clang.llvm.org/docs/LanguageExtensions.html @@ -132,8 +133,7 @@ /// /// @param MAX Maximum value of the narrowest type of operand. /// Not used if compiler supports __builtin_add_overflow. -#if (defined(__clang__) && __has_builtin(__builtin_add_overflow)) \ - || (__GNUC__ >= 5) +#if HAVE_BUILTIN_ADD_OVERFLOW # define STRICT_ADD(a, b, c, t) \ do { if (__builtin_add_overflow(a, b, c)) { abort(); } } while (0) #else @@ -141,8 +141,9 @@ do { *(c) = (t)(a + b); } while (0) #endif -#if (defined(__clang__) && __has_builtin(__builtin_sub_overflow)) \ - || (__GNUC__ >= 5) +/// @def STRICT_SUB +/// @brief Subtracts (a - b) and stores result in `c`. Aborts on overflow. +#if HAVE_BUILTIN_ADD_OVERFLOW # define STRICT_SUB(a, b, c, t) \ do { if (__builtin_sub_overflow(a, b, c)) { abort(); } } while (0) #else