[impala] 11/20: IMPALA-3675: part 1: -Werror for ASAN

[ta...@apache.org]

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

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit efe751078f8ab674523ccfe15be50349332b16bf
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Fri Jul 20 09:58:47 2018 -0700

    IMPALA-3675: part 1: -Werror for ASAN
    
    This fixes some clang warnings and enables -Werror going forward for
    ASAN.
    
    It adds -Wno-return-type-c-linkage to suppress a warning that otherwise
    would fail the build.
    
    It also enables the mismatched tags check and fixes the various
    instances of it (this warning is irritating for YouCompleteMe users).
    
    Change-Id: If83ca41cde49fd6cfde479e45f9cfdd9e3a45bec
    Reviewed-on: http://gerrit.cloudera.org:8080/11002
    Reviewed-by: Tim Armstrong <ta...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .clang-tidy                             |  1 -
 be/CMakeLists.txt                       | 12 +++++-------
 be/src/exec/hdfs-table-writer.h         |  2 +-
 be/src/exprs/expr.h                     |  2 +-
 be/src/exprs/scalar-expr.h              |  2 +-
 be/src/runtime/krpc-data-stream-recvr.h |  2 +-
 be/src/runtime/mem-tracker.h            |  2 +-
 7 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/.clang-tidy b/.clang-tidy
index 54f095c..bf37d7a 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -46,7 +46,6 @@ Checks: "-*,clang*,\
 -clang-diagnostic-gnu-anonymous-struct,\
 -clang-diagnostic-header-hygiene,\
 -clang-diagnostic-implicit-fallthrough,\
--clang-diagnostic-mismatched-tags,\
 -clang-diagnostic-missing-prototypes,\
 -clang-diagnostic-missing-variable-declarations,\
 -clang-diagnostic-nested-anon-types,\
diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index 83d932a..c29a225 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -63,8 +63,6 @@ SET(CXX_COVERAGE_FLAGS "-fprofile-arcs -ftest-coverage")
 #        makes extra calls to clang which may have extra includes (-I) that are unused.
 #   -fcolor-diagnostics: ensure clang generates colorized output, which is necessary
 #        when using ccache as clang thinks it is not called from a terminal.
-#   -Wno-mismatched-tags: ignore harmless class/struct mismatch for forward declarations.
-#        Disabling to be consistent with gcc, which doesn't have this warning.
 #   -Wno-zero-as-null-pointer-constant: We are slowly moving towards the use of nullptr,
 #        but till we switch to it completely, we will ignore the warnings due to use of
 #        NULL as a null pointer constant.
@@ -75,10 +73,10 @@ SET(CXX_COVERAGE_FLAGS "-fprofile-arcs -ftest-coverage")
 #        destructors with 'override' which is enforced by clang by not recommended by c++
 #        core guidelines (read C.128).
 SET(CXX_CLANG_FLAGS "-Qunused-arguments -fcolor-diagnostics -Wno-unused-local-typedef")
-SET(CXX_CLANG_FLAGS "${CXX_CLANG_FLAGS} -Wno-mismatched-tags")
 SET(CXX_CLANG_FLAGS "${CXX_CLANG_FLAGS} -Wno-zero-as-null-pointer-constant")
 SET(CXX_CLANG_FLAGS "${CXX_CLANG_FLAGS} -Wno-c++17-extensions")
 SET(CXX_CLANG_FLAGS "${CXX_CLANG_FLAGS} -Wno-inconsistent-missing-destructor-override")
+SET(CXX_CLANG_FLAGS "${CXX_CLANG_FLAGS} -Wno-return-type-c-linkage")
 # For any gcc builds:
 #   -g: Enable symbols for profiler tools
 #   -Wno-unused-local-typedefs: Do not warn for local typedefs that are unused.
@@ -104,15 +102,15 @@ endif()
 # Debug information is stored as dwarf2 to be as compatible as possible
 SET(CXX_FLAGS_DEBUG "${CXX_GCC_FLAGS} -ggdb -gdwarf-2")
 # -Werror: compile warnings should be errors when using the toolchain compiler.
-#   Only enable for debug builds because this is what we test in pre-commit tests.
+#   Enabled for DEBUG, ASAN, TSAN and UBSAN builds which are built pre-commit.
 SET(CXX_FLAGS_DEBUG "${CXX_FLAGS_DEBUG} -Werror")
 SET(CXX_FLAGS_RELEASE "${CXX_GCC_FLAGS} -O3 -DNDEBUG -gdwarf-2")
 SET(CXX_FLAGS_ADDRESS_SANITIZER
-  "${CXX_CLANG_FLAGS} -O1 -g -fsanitize=address -fno-omit-frame-pointer -DADDRESS_SANITIZER")
+  "${CXX_CLANG_FLAGS} -Werror -O1 -g -fsanitize=address -fno-omit-frame-pointer -DADDRESS_SANITIZER")
 
 # Set the flags to the undefined behavior sanitizer, also known as "ubsan"
 # Turn on sanitizer and debug symbols to get stack traces:
-SET(CXX_FLAGS_UBSAN "${CXX_CLANG_FLAGS} -ggdb3 -fno-omit-frame-pointer -fsanitize=undefined")
+SET(CXX_FLAGS_UBSAN "${CXX_CLANG_FLAGS} -Werror -ggdb3 -fno-omit-frame-pointer -fsanitize=undefined")
 # Set preprocessor macros to facilitate initialization the relevant configuration.
 SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} -DUNDEFINED_SANITIZER")
 # Calling getenv() in __ubsan_default_options doesn't work, likely because of
@@ -131,7 +129,7 @@ SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} -O0")
 
 # Set the flags to the thread sanitizer, also known as "tsan"
 # Turn on sanitizer and debug symbols to get stack traces:
-SET(CXX_FLAGS_TSAN "${CXX_CLANG_FLAGS} -O1 -ggdb3 -fno-omit-frame-pointer")
+SET(CXX_FLAGS_TSAN "${CXX_CLANG_FLAGS} -Werror -O1 -ggdb3 -fno-omit-frame-pointer")
 SET(CXX_FLAGS_TSAN "${CXX_FLAGS_TSAN} -fsanitize=thread -DTHREAD_SANITIZER")
 
 SET(CXX_FLAGS_TIDY "${CXX_CLANG_FLAGS}")
diff --git a/be/src/exec/hdfs-table-writer.h b/be/src/exec/hdfs-table-writer.h
index cc6c6cc..a6f06c0 100644
--- a/be/src/exec/hdfs-table-writer.h
+++ b/be/src/exec/hdfs-table-writer.h
@@ -30,7 +30,7 @@ namespace impala {
 class HdfsPartitionDescriptor;
 class HdfsTableDescriptor;
 class HdfsTableSink;
-class OutputPartition;
+struct OutputPartition;
 class RowBatch;
 class RuntimeState;
 class ScalarExprEvaluator;
diff --git a/be/src/exprs/expr.h b/be/src/exprs/expr.h
index 1a47db6..79f980d 100644
--- a/be/src/exprs/expr.h
+++ b/be/src/exprs/expr.h
@@ -32,7 +32,7 @@
 namespace impala {
 
 class IsNullExpr;
-class LibCacheEntry;
+struct LibCacheEntry;
 class LlvmCodeGen;
 class MemTracker;
 class ObjectPool;
diff --git a/be/src/exprs/scalar-expr.h b/be/src/exprs/scalar-expr.h
index e1954b7..6eb221e 100644
--- a/be/src/exprs/scalar-expr.h
+++ b/be/src/exprs/scalar-expr.h
@@ -55,7 +55,7 @@ using impala_udf::StringVal;
 using impala_udf::DecimalVal;
 using impala_udf::CollectionVal;
 
-class LibCacheEntry;
+struct LibCacheEntry;
 class LlvmCodeGen;
 class MemTracker;
 class ObjectPool;
diff --git a/be/src/runtime/krpc-data-stream-recvr.h b/be/src/runtime/krpc-data-stream-recvr.h
index 1435e44..18454d7 100644
--- a/be/src/runtime/krpc-data-stream-recvr.h
+++ b/be/src/runtime/krpc-data-stream-recvr.h
@@ -43,7 +43,7 @@ class MemTracker;
 class RowBatch;
 class RuntimeProfile;
 class SortedRunMerger;
-class TransmitDataCtx;
+struct TransmitDataCtx;
 class TransmitDataRequestPB;
 class TransmitDataResponsePB;
 
diff --git a/be/src/runtime/mem-tracker.h b/be/src/runtime/mem-tracker.h
index 9c41693..01fd331 100644
--- a/be/src/runtime/mem-tracker.h
+++ b/be/src/runtime/mem-tracker.h
@@ -41,7 +41,7 @@ namespace impala {
 
 class ObjectPool;
 class MemTracker;
-class ReservationTrackerCounters;
+struct ReservationTrackerCounters;
 class TQueryOptions;
 
 /// A MemTracker tracks memory consumption; it contains an optional limit