You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kg...@apache.org on 2019/11/05 14:55:09 UTC

[qpid-dispatch] branch master updated: DISPATCH-1467: enable AddressSanitizer build option (ASAN)

This is an automated email from the ASF dual-hosted git repository.

kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git


The following commit(s) were added to refs/heads/master by this push:
     new 1fafe27  DISPATCH-1467: enable AddressSanitizer build option (ASAN)
1fafe27 is described below

commit 1fafe273a5d7f82cccb6d3a0ce5a8c6a6cef84cc
Author: Kenneth Giusti <kg...@apache.org>
AuthorDate: Fri Nov 1 10:39:58 2019 -0400

    DISPATCH-1467: enable AddressSanitizer build option (ASAN)
    
    To run the AddressSanitizer, define RUNTIME_CHECK=asan via CMake:
    
      cmake .. -DRUNTIME_CHECK=asan
    
    This patch updates the .travis.yml file to enable address and
    undefined behavior checking.  Also included are minor code tweaks to
    issues found when running with the sanitizer enabled.
    
    This closes #608
---
 .travis.yml               |  3 ++-
 README                    | 22 +++++++++++++++-
 cmake/RuntimeChecks.cmake | 42 ++++++++++++++++++++-----------
 pom.xml                   |  2 ++
 router/src/main.c         | 44 ++++++++++++++++----------------
 run.py.in                 |  4 ++-
 src/alloc_pool.c          |  2 +-
 tests/asan.supp           |  3 +++
 tests/lsan.supp           | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 144 insertions(+), 42 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 54335c2..efe6ce6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -52,6 +52,7 @@ matrix:
   - os: linux
     env:
     - PATH="/usr/bin:$PATH" PROTON_VERSION=0.29.0 BUILD_TYPE=RelWithDebInfo
+    - DISPATCH_CMAKE_ARGS='-DRUNTIME_CHECK=asan'
   - os: osx
     osx_image: xcode10.1
     env:
@@ -103,7 +104,7 @@ before_script:
 - source qpid-proton/build/config.sh
 - mkdir build
 - pushd build
-- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DUSE_VALGRIND=NO -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DDISPATCH_TEST_TIMEOUT=600
+- cmake .. -DCMAKE_INSTALL_PREFIX=$PREFIX -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DDISPATCH_TEST_TIMEOUT=600 ${DISPATCH_CMAKE_ARGS}
 - cmake --build . --target install -- -j $NPROC
 
 script:
diff --git a/README b/README
index 91cc94f..14f1f06 100644
--- a/README
+++ b/README
@@ -144,7 +144,7 @@ GCC/Clang Thread Sanitizer (TSAN)
 ---------------------------------
 This option turns on extra run time threading verification.
 
-Applicable only to GCC versions >= 4.8 and Clang versions >= 4.1.
+Applicable only to GCC versions >= 7.4 and Clang versions >= 6.0.
 
 To enable the thread sanitizer set the RUNTIME_CHECK build flag to "tsan":
 
@@ -159,3 +159,23 @@ displayed in the CTest output. For example:
 
 RuntimeError: Errors during teardown:
 Process XXXX error: exit code 66, expected 0
+
+False positives can be suppressed via the tsan.supp file in the tests
+directory.
+
+GCC/Clang Address Sanitizer (ASAN)
+----------------------------------
+This option turns on extra run time memory verification, including
+leak checks.
+
+Applicable only to GCC versions >= 5.4 and Clang versions >= 6.0.
+
+To enable the address sanitizer set the RUNTIME_CHECK build flag to "asan":
+
+cmake .. -DRUNTIME_CHECK=asan
+
+The ASAN (libasan) and UBSAN (libubsan) libraries must be installed in
+order to use this option.
+
+False positive leak errors can be suppressed via the lsan.supp file in
+the tests directory.
diff --git a/cmake/RuntimeChecks.cmake b/cmake/RuntimeChecks.cmake
index 929bb1d..0b952e7 100644
--- a/cmake/RuntimeChecks.cmake
+++ b/cmake/RuntimeChecks.cmake
@@ -22,17 +22,15 @@
 # The RUNTIME_CHECK variable enables run-time checking when running
 # the CTest test suite. The following tools are supported
 #
-# -DRUNTIME_CHECK=memcheck   # runs qdrouter under valgrind's leak checker
-# -DRUNTIME_CHECK=tsan       # turns on thread sanitizer
+# -DRUNTIME_CHECK=tsan      # turns on thread sanitizer
+# -DRUNTIME_CHECK=asan      # address and undefined behavior sanitizer
+# -DRUNTIME_CHECK=memcheck  # valgrind memcheck (in progress)
+# -DRUNTIME_CHECK=helgrind  # valgrind helgrind (in progress)
 #
 # This file updates the QDROUTERD_RUNNER and CMAKE_C_FLAGS
 # appropriately for use when running the ctest suite.
 
 
-# Valid options for RUNTIME_CHECK
-#
-set(runtime_checks OFF tsan memcheck helgrind)
-
 # Valgrind configuration
 #
 find_program(VALGRIND_EXECUTABLE valgrind DOC "Location of the valgrind program")
@@ -46,12 +44,12 @@ macro(assert_has_valgrind)
 endmacro()
 
 # Check for compiler's support of sanitizers.
-# Currently have tested back to gcc 7.4.0 and clang 6.0.0, older
+# Currently have tested back to gcc 5.4.0 and clang 6.0.0, older
 # versions may require more work
 #
 if((CMAKE_C_COMPILER_ID MATCHES "GNU"
-      AND (CMAKE_C_COMPILER_VERSION VERSION_GREATER 7.4
-        OR CMAKE_C_COMPILER_VERSION VERSION_EQUAL 7.4))
+      AND (CMAKE_C_COMPILER_VERSION VERSION_GREATER 5.4
+        OR CMAKE_C_COMPILER_VERSION VERSION_EQUAL 5.4))
     OR (CMAKE_C_COMPILER_ID MATCHES "Clang"
       AND (CMAKE_C_COMPILER_VERSION VERSION_GREATER 6.0
         OR CMAKE_C_COMPILER_VERSION VERSION_EQUAL 6.0)))
@@ -63,9 +61,14 @@ macro(assert_has_sanitizers)
   endif()
 endmacro()
 
+# Valid options for RUNTIME_CHECK
+#
+set(runtime_checks OFF tsan asan memcheck helgrind)
+
 # Set RUNTIME_CHECK value and deal with the older cmake flags for
 # valgrind and TSAN
 #
+set(RUNTIME_CHECK_DEFAULT OFF)
 macro(deprecated_enable_check old new doc)
   if (${old})
     message("WARNING: option ${old} is deprecated, use -DRUNTIME_CHECK=${new} instead")
@@ -92,11 +95,20 @@ elseif(RUNTIME_CHECK STREQUAL "helgrind")
   message(STATUS "Runtime race checker: valgrind helgrind")
   set(QDROUTERD_RUNNER "${VALGRIND_EXECUTABLE} --tool=helgrind ${VALGRIND_COMMON_ARGS}")
 
-#elseif(RUNTIME_CHECK STREQUAL "asan")
-#  assert_has_sanitizers()
-#  message(STATUS "Runtime memory checker: gcc/clang memory sanitizers")
-#  set(SANITIZE_FLAGS "-g -fno-omit-frame-pointer -fsanitize=address,undefined")
-#  set(TEST_WRAP_PREFIX "${CMAKE_SOURCE_DIR}/tests/preload_asan.sh $<TARGET_FILE:qpid-proton-core>")
+elseif(RUNTIME_CHECK STREQUAL "asan")
+  assert_has_sanitizers()
+  find_library(ASAN_LIBRARY NAME asan libasan)
+  if(ASAN_LIBRARY-NOTFOUND)
+    message(FATAL_ERROR "libasan not installed - address sanitizer not available")
+  endif(ASAN_LIBRARY-NOTFOUND)
+  find_library(UBSAN_LIBRARY NAME ubsan libubsan)
+  if(UBSAN_LIBRARY-NOTFOUND)
+    message(FATAL_ERROR "libubsan not installed - address sanitizer not available")
+  endif(UBSAN_LIBRARY-NOTFOUND)
+  message(STATUS "Runtime memory checker: gcc/clang address sanitizers")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -fno-omit-frame-pointer -fsanitize=address,undefined")
+  set(RUNTIME_ASAN_ENV_OPTIONS "detect_leaks=true suppressions=${CMAKE_SOURCE_DIR}/tests/asan.supp")
+  set(RUNTIME_LSAN_ENV_OPTIONS "suppressions=${CMAKE_SOURCE_DIR}/tests/lsan.supp")
 
 elseif(RUNTIME_CHECK STREQUAL "tsan")
   assert_has_sanitizers()
@@ -108,6 +120,6 @@ elseif(RUNTIME_CHECK STREQUAL "tsan")
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -fno-omit-frame-pointer -fsanitize=thread")
   set(RUNTIME_TSAN_ENV_OPTIONS "second_deadlock_stack=1 suppressions=${CMAKE_SOURCE_DIR}/tests/tsan.supp")
 
-elseif(NOT RUNTIME_CHECK STREQUAL "")
+elseif(RUNTIME_CHECK)
   message(FATAL_ERROR "'RUNTIME_CHECK=${RUNTIME_CHECK}' is invalid, valid values: ${runtime_checks}")
 endif()
diff --git a/pom.xml b/pom.xml
index 95fece9..67e6e4d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -98,6 +98,8 @@
                       <exclude>**/MANIFEST.in</exclude>
                       <exclude>**/valgrind.supp</exclude>
                       <exclude>**/tsan.supp</exclude>
+                      <exclude>**/asan.supp</exclude>
+                      <exclude>**/lsan.supp</exclude>
                       <exclude>**/*.json.in</exclude>
                       <exclude>**/*.json</exclude>
                       <exclude>**/*.svg</exclude>
diff --git a/router/src/main.c b/router/src/main.c
index aae176e..fa41f95 100644
--- a/router/src/main.c
+++ b/router/src/main.c
@@ -29,6 +29,7 @@
 #include <stdlib.h>
 #include <getopt.h>
 #include <errno.h>
+#include <limits.h>
 #include "config.h"
 
 static int            exit_with_sigint = 0;
@@ -179,43 +180,40 @@ static void daemon_process(const char *config_path, const char *python_pkgdir, b
 
 
             //
-            // If config path is not represented by its full path, then
-            // save current path before changing to /
+            // If config path is not a fully qualified path, then construct the
+            // fully qualified path to the config file.  This needs to be done
+            // since the daemon will set "/" to its working directory.
             //
             char *config_path_full = NULL;
             if (strncmp("/", config_path, 1)) {
-                char *cur_path = NULL;
-                size_t path_size = 256;
-                int getcwd_error = 0;
-                cur_path = (char *) calloc(path_size, sizeof(char));
+                size_t path_size = PATH_MAX;
+                char *cur_path = (char *) calloc(path_size, sizeof(char));
                 errno = 0;
 
                 while (getcwd(cur_path, path_size) == NULL) {
                     free(cur_path);
-                    if ( errno != ERANGE ) {
-                        // If unable to get current directory
-                        getcwd_error = 1;
-                        break;
+                    if (errno != ERANGE) {
+                        // Hard failure - can't recover from this
+                        perror("Unable to determine current directory");
+                        exit(1);
                     }
-                    // If current path does not fit, allocate more memory
+                    // errno == ERANGE: the current path does not fit, allocate
+                    // more memory
                     path_size += 256;
                     cur_path = (char *) calloc(path_size, sizeof(char));
                     errno = 0;
                 }
 
                 // Populating fully qualified config file name
-                if (!getcwd_error) {
-                    size_t cpf_len = path_size + strlen(config_path) + 1;
-                    config_path_full = calloc(cpf_len, sizeof(char));
-                    snprintf(config_path_full, cpf_len, "%s%s%s",
-                             cur_path,
-                             !strcmp("/", cur_path)? "":"/",
-                             config_path);
-
-                    // Releasing temporary path variable
-                    memset(cur_path, 0, path_size * sizeof(char));
-                    free(cur_path);
-                }
+                const char *path_sep = !strcmp("/", cur_path) ? "" : "/";
+                size_t cpf_len = strlen(cur_path) + strlen(path_sep) + strlen(config_path) + 1;
+                config_path_full = calloc(cpf_len, sizeof(char));
+                snprintf(config_path_full, cpf_len, "%s%s%s",
+                         cur_path, path_sep, config_path);
+
+                // Releasing temporary path variable
+                memset(cur_path, 0, path_size * sizeof(char));
+                free(cur_path);
             }
 
             //
diff --git a/run.py.in b/run.py.in
index 1ab6379..67cf5d1 100755
--- a/run.py.in
+++ b/run.py.in
@@ -84,7 +84,9 @@ env_vars = {
     'QPID_DISPATCH_RUNNER': qdrouterd_runner,
     'MALLOC_CHECK_': "3",      # 3 = print error and abort
     'MALLOC_PERTURB_': "153",  # 153 = 0x99 freed memory pattern
-    'TSAN_OPTIONS': "${RUNTIME_TSAN_ENV_OPTIONS}"
+    'TSAN_OPTIONS': "${RUNTIME_TSAN_ENV_OPTIONS}",
+    'ASAN_OPTIONS': "${RUNTIME_ASAN_ENV_OPTIONS}",
+    'LSAN_OPTIONS': "${RUNTIME_LSAN_ENV_OPTIONS}",
 }
 os.environ.update(env_vars)
 
diff --git a/src/alloc_pool.c b/src/alloc_pool.c
index 35c0873..8454141 100644
--- a/src/alloc_pool.c
+++ b/src/alloc_pool.c
@@ -46,7 +46,7 @@ DEQ_DECLARE(qd_alloc_type_t, qd_alloc_type_list_t);
 #define PATTERN_BACK  0xbabecafe
 
 struct qd_alloc_item_t {
-    uint32_t              sequence;
+    uintmax_t             sequence;  // uintmax_t ensures proper alignment of following data
 #ifdef QD_MEMORY_DEBUG
     qd_alloc_type_desc_t *desc;
     uint32_t              header;
diff --git a/tests/asan.supp b/tests/asan.supp
new file mode 100644
index 0000000..0f8aec1
--- /dev/null
+++ b/tests/asan.supp
@@ -0,0 +1,3 @@
+# Suppression file for address issues found by AddressSanitizer (ASAN)
+# Note: memory leaks should not be suppressed here - update lsan.supp
+# instead.
diff --git a/tests/lsan.supp b/tests/lsan.supp
new file mode 100644
index 0000000..b2d8c81
--- /dev/null
+++ b/tests/lsan.supp
@@ -0,0 +1,64 @@
+# Suppression file for memory leaks
+# found by AddressSanitizer (ASAN)
+#
+leak:_flush_send_queue_CT
+leak:add_session_to_free_list
+leak:compose_message_annotations
+leak:qd_parse_annotations
+leak:qd_compose_insert_string_n
+leak:qd_core_agent_query_handler
+leak:qd_dispatch_configure_connector
+leak:qd_dispatch_configure_listener
+leak:qd_dispatch_configure_ssl_profile
+leak:qd_manage_response_handler
+leak:qd_message_receive
+leak:qd_policy_amqp_open
+leak:qd_policy_amqp_open_connector
+leak:qd_policy_c_counts_alloc
+leak:qd_policy_open_fetch_settings
+leak:qdi_router_configure_body
+leak:qdr_action
+leak:qdr_add_local_address_CT
+leak:qdr_connection_opened
+leak:qdr_core_subscribe
+leak:qdr_error_description
+leak:qdr_forward_balanced_CT
+leak:qdr_forward_closest_CT
+leak:qdr_forward_multicast_CT
+leak:qdr_link_deliver
+leak:qdr_link_deliver_to_routed_link
+leak:qdr_link_deliver_to_routed_link
+leak:qdr_link_outbound_detach_CT
+leak:qdr_manage_advance_address_CT
+leak:qdr_node_connect_deliveries
+leak:qdr_route_add_auto_link_CT
+leak:qdr_route_add_link_route_CT
+leak:qdr_route_connection_opened_CT
+leak:qdr_subscribe_CT
+leak:qdr_terminus
+leak:router_annotate_message
+leak:qd_container_register_node_type
+
+# Ubuntu 18.04 (Bionic)
+leak:qdr_link_issue_credit_CT
+leak:qdr_delivery_push_CT
+
+# Ubuntu 16.04 (Xenial)
+leak:_ctypes_alloc_format_string
+leak:__strdup
+leak:add_link_to_free_list
+leak:qd_parse_internal
+
+
+####
+#### Miscellaneous 3rd party libraries, test code, etc:
+####
+
+leak:*libpython*
+leak:*libwebsockets*
+
+# We should be able to uncomment these once all known dispatch leaks have been fixed
+leak:*libqpid-proton*
+
+# Ignore test code
+leak:run_unit_tests.c


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org