From b6a090b4fd5d611df9cbc5cf66b3474c1d050750 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Fri, 11 Nov 2016 14:18:54 -0500 Subject: [PATCH 1/4] tsan: Compile with -fPIE and force clang-3.8 This fixes failures with TSAN builds like FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at 0x55deea465000 < 0x7cf000000000) FATAL: Make sure to compile with -fPIE and to link with -pie. --- .travis.yml | 16 +++++++--------- src/nvim/CMakeLists.txt | 1 + 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1437f7e25b..a09a24dc6c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,9 +9,7 @@ env: # http://docs.travis-ci.com/user/speeding-up-the-build/#Paralellizing-your-build-on-one-VM - MAKE_CMD="make -j2" # Update PATH for pip. - - PATH="$(python2.7 -c 'import site; print(site.getuserbase())')/bin:$PATH" - # LLVM symbolizer path. - - LLVM_SYMBOLIZER="$(which llvm-symbolizer-3.4)" + - PATH="$(python2.7 -c 'import site; print(site.getuserbase())')/bin:/usr/lib/llvm-symbolizer-3.8/bin:$PATH" # Build directory for Neovim. - BUILD_DIR="$TRAVIS_BUILD_DIR/build" # Build directory for third-party dependencies. @@ -36,8 +34,7 @@ env: -DCMAKE_TOOLCHAIN_FILE=$TRAVIS_BUILD_DIR/cmake/i386-linux-gnu.toolchain.cmake" # Environment variables for Clang sanitizers. - ASAN_OPTIONS="detect_leaks=1:check_initialization_order=1:log_path=$LOG_DIR/asan" - - ASAN_SYMBOLIZER_PATH="$LLVM_SYMBOLIZER" - - TSAN_OPTIONS="external_symbolizer_path=$LLVM_SYMBOLIZER log_path=$LOG_DIR/tsan" + - TSAN_OPTIONS="log_path=$LOG_DIR/tsan" - UBSAN_OPTIONS="log_path=$LOG_DIR/ubsan" # Environment variables for Valgrind. - VALGRIND_LOG="$LOG_DIR/valgrind-%p.log" @@ -70,10 +67,10 @@ matrix: compiler: gcc-5 -m32 env: BUILD_32BIT=ON - os: linux - compiler: clang + compiler: clang-3.8 env: CLANG_SANITIZER=ASAN_UBSAN - os: linux - compiler: clang + compiler: clang-3.8 env: CLANG_SANITIZER=TSAN - os: osx compiler: clang @@ -96,11 +93,12 @@ addons: # TODO: Remove PPA when Travis gets Python >=3.3. - deadsnakes - ubuntu-toolchain-r-test + - llvm-toolchain-precise-3.8 packages: - autoconf - automake - build-essential - - clang-3.4 + - clang-3.8 - cmake - g++-5-multilib - g++-multilib @@ -109,7 +107,7 @@ addons: - gdb - libc6-dev-i386 - libtool - - llvm-3.4-dev + - llvm-3.8-dev - pkg-config - python3.3-dev - unzip diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index 49edfda838..f2b75dca2a 100644 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -326,6 +326,7 @@ elseif(CLANG_TSAN) message(STATUS "Enabling Clang thread sanitizer for nvim.") set_property(TARGET nvim APPEND_STRING PROPERTY COMPILE_FLAGS "-DEXITFREE ") set_property(TARGET nvim APPEND_STRING PROPERTY COMPILE_FLAGS "-fsanitize=thread ") + set_property(TARGET nvim APPEND_STRING PROPERTY COMPILE_FLAGS "-fPIE ") set_property(TARGET nvim APPEND_STRING PROPERTY LINK_FLAGS "-fsanitize=thread ") endif() From 6d727657c8c9f404821ddd92b36a8e4bc1a4190e Mon Sep 17 00:00:00 2001 From: James McCoy Date: Sat, 12 Nov 2016 22:28:59 -0500 Subject: [PATCH 2/4] ci: Make ubsan print out a stacktrace when it finds a problem --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a09a24dc6c..039a00cce8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,7 +35,7 @@ env: # Environment variables for Clang sanitizers. - ASAN_OPTIONS="detect_leaks=1:check_initialization_order=1:log_path=$LOG_DIR/asan" - TSAN_OPTIONS="log_path=$LOG_DIR/tsan" - - UBSAN_OPTIONS="log_path=$LOG_DIR/ubsan" + - UBSAN_OPTIONS="print_stacktrace=1 log_path=$LOG_DIR/ubsan" # Environment variables for Valgrind. - VALGRIND_LOG="$LOG_DIR/valgrind-%p.log" # Cache marker for third-party dependencies cache. From 38ee85d0007dced6469f4d93b46e55a49511f172 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Sun, 13 Nov 2016 10:33:48 -0500 Subject: [PATCH 3/4] Move utf8len_tab definition to globals.h The existing code would cause utf8len_tab to be declared as non-extern when main.cpp included globals.h as well as in mbyte.c. This causes the following warning Linking C executable ../../bin/nvim /usr/bin/ld: Warning: size of symbol `utf8len_tab' changed from 256 in CMakeFiles/nvim.dir/main.c.o to 320 in CMakeFiles/nvim.dir/mbyte.c.o Moving the definition to globals.h and using INIT() ensures the array is only defined in main.cpp and other places globals.h is included see an extern declaration. --- src/nvim/globals.h | 24 ++++++++++++++++++++++-- src/nvim/mbyte.c | 18 ------------------ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/nvim/globals.h b/src/nvim/globals.h index 301a2c1663..f81fb43eaf 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -788,8 +788,28 @@ EXTERN int vr_lines_changed INIT(= 0); /* #Lines changed by "gR" so far */ /// Encoding used when 'fencs' is set to "default" EXTERN char_u *fenc_default INIT(= NULL); -// To speed up BYTELEN() we keep a table with the byte lengths for utf-8 -EXTERN char utf8len_tab[256]; +// To speed up BYTELEN(); keep a lookup table to quickly get the length in +// bytes of a UTF-8 character from the first byte of a UTF-8 string. Bytes +// which are illegal when used as the first byte have a 1. The NUL byte has +// length 1. +EXTERN char utf8len_tab[256] INIT(= { + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, + 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, + 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 1, 1, +}); # if defined(USE_ICONV) && defined(DYNAMIC_ICONV) /* Pointers to functions and variables to be loaded at runtime */ diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c index 7be0be7106..2ecd86974e 100644 --- a/src/nvim/mbyte.c +++ b/src/nvim/mbyte.c @@ -68,24 +68,6 @@ struct interval { # include "unicode_tables.generated.h" #endif -/* - * Lookup table to quickly get the length in bytes of a UTF-8 character from - * the first byte of a UTF-8 string. - * Bytes which are illegal when used as the first byte have a 1. - * The NUL byte has length 1. - */ -char utf8len_tab[256] = -{ - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, - 2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2, - 3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,4,4,4,4,4,4,4,4,5,5,5,5,6,6,1,1, -}; - /* * Like utf8len_tab above, but using a zero for illegal lead bytes. */ From ca292c97688e1a43f2b29594007effb8dd141636 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Thu, 17 Nov 2016 18:16:46 -0500 Subject: [PATCH 4/4] Avoid serializing NULL string through msgpack Attempting to serialize a NULL string through msgpack results in msgpack_sbuffer_write attempting to memcpy from a NULL pointer, which is undefined behavior. --- src/nvim/msgpack_rpc/helpers.c | 4 +++- src/nvim/shada.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index c3a909692f..b0cfe2d6cd 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -326,7 +326,9 @@ void msgpack_rpc_from_string(String result, msgpack_packer *res) FUNC_ATTR_NONNULL_ARG(2) { msgpack_pack_str(res, result.size); - msgpack_pack_str_body(res, result.data, result.size); + if (result.size > 0) { + msgpack_pack_str_body(res, result.data, result.size); + } } typedef struct { diff --git a/src/nvim/shada.c b/src/nvim/shada.c index 01c0807d82..d902079739 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -1575,7 +1575,9 @@ static char *shada_filename(const char *file) do { \ const String s_ = (s); \ msgpack_pack_bin(spacker, s_.size); \ - msgpack_pack_bin_body(spacker, s_.data, s_.size); \ + if (s_.size > 0) { \ + msgpack_pack_bin_body(spacker, s_.data, s_.size); \ + } \ } while (0) /// Write single ShaDa entry