You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/02/21 00:10:43 UTC

[2/3] kudu git commit: KUDU-2275. Upgrade libunwind to 1.3-rc1

KUDU-2275. Upgrade libunwind to 1.3-rc1

Per KUDU-2275, the version of libunwind that we were previously using
can occasionally crash during stack unwinding if it attempts to access a
page which is mprotected to be non-readable. This could be due to
incorrect interpretation of dwarf unwinding info or some other bug.

As we are trying to collect stacks more aggressively now, it's important
to upgrade. This new version uses a more robust mechanism for checking
whether a page is valid to access before accessing it during unwinding.

This also includes a patch from the libunwind git repo which fixes a
potential issue with libunwind in ASAN builds. I didn't run into this
issue yet myself, but seems like a straightforward and necessary fix.

Change-Id: I3f942a0d566e1b1e5ebe48f09362bf8aa3e1e33e
Reviewed-on: http://gerrit.cloudera.org:8080/9335
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c182d626
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c182d626
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c182d626

Branch: refs/heads/master
Commit: c182d626c111dde30d5602aaba102fb47b7404df
Parents: e0e440e
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Feb 15 09:28:01 2018 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Feb 20 21:34:47 2018 +0000

----------------------------------------------------------------------
 src/kudu/util/file_cache-test.cc                | 13 ++++--
 thirdparty/download-thirdparty.sh               |  7 ++++
 ...rectly-in-write_validate-to-avoid-ASAN.patch | 43 ++++++++++++++++++++
 thirdparty/vars.sh                              |  2 +-
 4 files changed, 61 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/src/kudu/util/file_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache-test.cc b/src/kudu/util/file_cache-test.cc
index 0595e5b..217a2be 100644
--- a/src/kudu/util/file_cache-test.cc
+++ b/src/kudu/util/file_cache-test.cc
@@ -29,8 +29,10 @@
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/cache.h"
+#include "kudu/util/debug-util.h"
 #include "kudu/util/env.h"
 #include "kudu/util/metrics.h"  // IWYU pragma: keep
 #include "kudu/util/random.h"
@@ -55,13 +57,18 @@ template <class FileType>
 class FileCacheTest : public KuduTest {
  public:
   FileCacheTest()
-      : rand_(SeedRandom()),
-        initial_open_fds_(CountOpenFds(env_)) {
+      : rand_(SeedRandom()) {
     // Simplify testing of the actual cache capacity.
     FLAGS_cache_force_single_shard = true;
 
     // Speed up tests that check the number of descriptors.
     FLAGS_file_cache_expiry_period_ms = 1;
+
+    // libunwind internally uses two file descriptors as a pipe.
+    // Make sure it gets initialized early so that our fd count
+    // doesn't get affected by it.
+    ignore_result(GetStackTraceHex());
+    initial_open_fds_ = CountOpenFds(env_);
   }
 
   void SetUp() override {
@@ -96,7 +103,7 @@ class FileCacheTest : public KuduTest {
   }
 
   Random rand_;
-  const int initial_open_fds_;
+  int initial_open_fds_;
   unique_ptr<FileCache<FileType>> cache_;
 };
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/download-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index 3bff35b..ba76d12 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -244,8 +244,15 @@ if [ ! -d $CRCUTIL_SOURCE ]; then
   echo
 fi
 
+LIBUNWIND_PATCHLEVEL=1
+delete_if_wrong_patchlevel $LIBUNWIND_SOURCE $LIBUNWIND_PATCHLEVEL
 if [ ! -d $LIBUNWIND_SOURCE ]; then
   fetch_and_expand libunwind-${LIBUNWIND_VERSION}.tar.gz
+  pushd $LIBUNWIND_SOURCE
+  patch -p1 < $TP_DIR/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
+  touch patchlevel-$LIBUNWIND_PATCHLEVEL
+  popd
+  echo
 fi
 
 if [ ! -d $PYTHON_SOURCE ]; then

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
----------------------------------------------------------------------
diff --git a/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch b/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
new file mode 100644
index 0000000..ef68266
--- /dev/null
+++ b/thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
@@ -0,0 +1,43 @@
+From 7d6cc6696ab8a808da3dbe23ca2493ddf2799b56 Mon Sep 17 00:00:00 2001
+From: Dave Watson <da...@fb.com>
+Date: Wed, 17 Jan 2018 08:04:05 -0800
+Subject: [PATCH 1/1] Use syscall directly in write_validate to avoid ASAN
+ errors
+
+ASAN will complain about this write call with the following error:
+
+ERROR: AddressSanitizer: stack-buffer-underflow on address
+HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
+
+This is similar to what google's abseil does to work around the issue.
+
+Reported-by: qiwang@fb.com
+---
+ src/x86_64/Ginit.c | 4 +++-
+ 2 files changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/src/x86_64/Ginit.c b/src/x86_64/Ginit.c
+index 6281b76..2a84a1e 100644
+--- a/src/x86_64/Ginit.c
++++ b/src/x86_64/Ginit.c
+@@ -34,6 +34,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.  */
+ #include <stdlib.h>
+ #include <string.h>
+ #include <sys/mman.h>
++#include <sys/syscall.h>
+ 
+ #include "unwind_i.h"
+ 
+@@ -107,7 +108,8 @@ write_validate (void *addr)
+ 
+   do
+     {
+-      ret = write (mem_validate_pipe[1], addr, 1);
++      /* use syscall insteadof write() so that ASAN does not complain */
++      ret = syscall (SYS_write, mem_validate_pipe[1], addr, 1);
+     }
+   while ( errno == EINTR );
+ 
+-- 
+2.7.4
+

http://git-wip-us.apache.org/repos/asf/kudu/blob/c182d626/thirdparty/vars.sh
----------------------------------------------------------------------
diff --git a/thirdparty/vars.sh b/thirdparty/vars.sh
index 472970c..6937d06 100644
--- a/thirdparty/vars.sh
+++ b/thirdparty/vars.sh
@@ -134,7 +134,7 @@ CRCUTIL_VERSION=42148a6df6986a257ab21c80f8eca2e54544ac4d
 CRCUTIL_NAME=crcutil-$CRCUTIL_VERSION
 CRCUTIL_SOURCE=$TP_SOURCE_DIR/$CRCUTIL_NAME
 
-LIBUNWIND_VERSION=1.1a
+LIBUNWIND_VERSION=1.3-rc1
 LIBUNWIND_NAME=libunwind-$LIBUNWIND_VERSION
 LIBUNWIND_SOURCE=$TP_SOURCE_DIR/$LIBUNWIND_NAME