Add Clang 15 with UBSan to our CI (ad-freiburg#834)

1. Add Clang 15 with undefined-behavior sanitizer (UBSan) to our CI 2. Fix all warnings raised by Clang 15 3. Fix overflow errors raised by UBSan 4. Update the submodules to minimize warnings with Clang 15 and fix remaining warnings
schlegan · Nov 30, 2022 · f72d75b · f72d75b
1 parent 4fae1b5
commit f72d75b
Show file tree

Hide file tree

Showing 55 changed files with 4,496 additions and 5,911 deletions.
diff --git a/.github/workflows/native-build.yml b/.github/workflows/native-build.yml
@@ -19,18 +19,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        compiler: [ gcc11, gcc12, clang13, clang14 ]
-        os: [ ubuntu-20.04, ubuntu-22.04 ]
-        warnings: [  "-Wall -Wextra -Werror -fsanitize=address -fno-omit-frame-pointer" ]
-        exclude:
-          - compiler: gcc11
-            os: ubuntu-22.04
-          - compiler: gcc12
-            os: ubuntu-20.04
-          - compiler: clang13
-            os: ubuntu-20.04
-          - compiler: clang14
-            os: ubuntu-20.04
+        compiler: [ gcc11, gcc12, clang13, clang14, clang15-ubsan ]
+        os: [ ubuntu-22.04 ]
+        warnings: [  "-Wall -Wextra -Werror" ]
     runs-on: ${{ matrix.os }}
 
 
@@ -61,6 +52,9 @@ jobs:
     - name: Install clang 14
       run:  sudo apt install clang-14
       if:  matrix.compiler == 'clang14'
+    - name: Install clang 15
+      run:  wget https://apt.llvm.org/llvm.sh && sudo chmod +x llvm.sh && sudo ./llvm.sh 15 && sudo apt install clang-15
+      if:  matrix.compiler == 'clang15-ubsan'
 
     - name: Python dependencies
       run: sudo apt-get install python3-yaml unzip pkg-config python3-icu

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -40,21 +40,29 @@ endif ()
 if (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") AND
    (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "12") AND
    (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "12.2"))
-    message(STATUS "Adding -Wno-restrict and -Wno-maybe-uninitialized for g++12.0 because of false positives")
-    add_compile_options(-Wno-restrict -Wno-maybe-uninitialized)
+    message(STATUS "Adding -Wno-error for g++12.0 because of the many false positive warnings from stdlib headers")
+    add_compile_options(-Wno-error)
     else()
 endif ()
 
 ###############################################################################
 ##### Essential settings #####
 ###############################################################################
 
+################################
+# GTEST AND GMOCK
+################################
+add_subdirectory(third_party/googletest EXCLUDE_FROM_ALL)
+include_directories(third_party/googletest/googletest/include)
+include_directories(third_party/googletest/googlemock/include)
+
 ###############################
 # ANTLR CPP RUNTIME FOR THE SPARQL PARSER
 ###############################
-add_subdirectory(third_party/antlr4/runtime/Cpp EXCLUDE_FROM_ALL)
-target_compile_options(antlr4_static PRIVATE -Wno-implicit-fallthrough -Wno-attributes -Wno-ambiguous-reversed-operator)
-include_directories(third_party/antlr4/runtime/Cpp/runtime/src)
+set(ANTLR_BUILD_CPP_TESTS OFF CACHE BOOL "don't try to build googletest twice")
+add_subdirectory(third_party/antlr4/runtime/Cpp EXCLUDE_FROM_ALL )
+target_compile_options(antlr4_static PRIVATE -Wno-all -Wno-extra -Wno-unqualified-std-cast-call -Wno-error -Wno-deprecated-declarations)
+include_directories(SYSTEM third_party/antlr4/runtime/Cpp/runtime/src )
 
 ################################
 # Threading
@@ -129,12 +137,6 @@ set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
 set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}")
 
-################################
-# GTEST AND GMOCK
-################################
-add_subdirectory(third_party/googletest EXCLUDE_FROM_ALL)
-include_directories(third_party/googletest/googletest/include)
-include_directories(third_party/googletest/googlemock/include)
 
 ################################
 # NLOHNMANN-JSON
@@ -152,10 +154,8 @@ include_directories(third_party/ctre/include)
 # ABSEIL
 ################################
 set(BUILD_TESTING OFF CACHE BOOL "Don't build tests for abseil" FORCE)
-add_subdirectory(third_party/abseil-cpp EXCLUDE_FROM_ALL)
-include_directories(third_party/abseil-cpp/)
-
-
+add_subdirectory(third_party/abseil-cpp  EXCLUDE_FROM_ALL)
+include_directories(SYSTEM third_party/abseil-cpp/)
 
 if (USE_PARALLEL)
     include(FindOpenMP)
@@ -192,7 +192,6 @@ include_directories(SYSTEM ${STXXL_INCLUDE_DIRS})
 ################################
 # RE2
 ################################
-
 # RE2 has a lot of unused-parameter warnings, we will deactivate
 # these for the subproject
 set(LOCAL_CXX_BACKUP_FLAGS "${CMAKE_CXX_FLAGS}")

diff --git a/src/engine/sparqlExpressions/NaryExpression.h b/src/engine/sparqlExpressions/NaryExpression.h
@@ -145,6 +145,9 @@ inline auto multiply = [](const auto& a, const auto& b) -> double {
 using MultiplyExpression = NARY<2, FV<decltype(multiply), NumericValueGetter>>;
 
 /// Division.
+// TODO<joka921> If `b == 0` this is technically undefined behavior and
+// should lead to an expression error in SPARQL. Fix this as soon as we
+// introduce the proper semantics for expression errors.
 inline auto divide = [](const auto& a, const auto& b) -> double {
   return static_cast<double>(a) / b;
 };

diff --git a/src/index/IndexImpl.Text.cpp b/src/index/IndexImpl.Text.cpp
@@ -239,7 +239,6 @@ void IndexImpl::processWordsForInvertedLists(const string& contextFile,
   size_t nofEntityPostings = 0;
   size_t entityNotFoundErrorMsgCount = 0;
 
-  size_t numLines = 0;
   for (auto line : wordsInTextRecords(contextFile, addWordsFromLiterals)) {
     if (line._contextId != currentContext) {
       ++nofContexts;
@@ -282,7 +281,6 @@ void IndexImpl::processWordsForInvertedLists(const string& contextFile,
       }
       wordsInContext[wid] += line._score;
     }
-    ++numLines;
   }
   if (entityNotFoundErrorMsgCount > 0) {
     LOG(WARN) << "Number of mentions of entities not found in the vocabulary: "

diff --git a/src/index/IndexImpl.cpp b/src/index/IndexImpl.cpp
@@ -489,7 +489,6 @@ IndexImpl::createPermutationPairImpl(const string& fileName1,
       THRESHOLD_RELATION_CREATION, fileName1 + ".tmp.mmap-buffer");
   bool functional = true;
   size_t distinctCol1 = 0;
-  size_t sizeOfRelation = 0;
   Id lastLhs = ID_NO_VALUE;
   uint64_t totalNumTriples = 0;
   for (auto triple : sortedTriples) {
@@ -513,7 +512,6 @@ IndexImpl::createPermutationPairImpl(const string& fileName1,
       currentRel = triple[c0];
       functional = true;
     } else {
-      sizeOfRelation++;
       if (triple[c1] == lastLhs) {
         functional = false;
       } else {

diff --git a/src/index/PrefixHeuristic.cpp b/src/index/PrefixHeuristic.cpp
@@ -179,7 +179,10 @@ std::vector<string> calculatePrefixes(const string& filename,
   ad_utility::Tree t;
   ad_utility::TreeNode* lastPos{nullptr};
   string nextWord;
-  size_t totalSavings = 0;
+  // The total amount of saved characters. For very few input words (e.g. in
+  // unit tests) this might become negative, so we need a signed integer to
+  // make the UB sanitizer happy.
+  int64_t totalSavings = 0;
   size_t numWords = 0;
 
   LOG(INFO) << "Building prefix tree from internal vocabulary ..." << std::endl;
@@ -226,7 +229,7 @@ std::vector<string> calculatePrefixes(const string& filename,
     totalSavings -= codelength * numWords;
   }
   int reductionInPercent =
-      static_cast<int>(0.5 + 100.0 * totalSavings / totalChars);
+      static_cast<size_t>(0.5 + 100.0 * totalSavings / totalChars);
   LOG(DEBUG) << "Total number of bytes : " << totalChars << std::endl;
   LOG(DEBUG) << "Total chars compressed : " << totalSavings << '\n';
   LOG(INFO) << "Reduction of size of internal vocabulary: "

diff --git a/src/parser/sparqlParser/CMakeLists.txt b/src/parser/sparqlParser/CMakeLists.txt
@@ -5,3 +5,6 @@ add_library(sparqlParser SparqlQleverVisitor.h
         generated/SparqlAutomaticParser.h generated/SparqlAutomaticParser.cpp
         generated/SparqlAutomaticVisitor.h generated/SparqlAutomaticVisitor.cpp)
 target_link_libraries(sparqlParser antlr4_static sparqlExpressions rdfEscaping absl::flat_hash_map)
+# Silence warnings in files that are auto-generated by ANTLR.
+# TODO<joka921> Submit a pull request to ANTLR to fix those warnings.
+target_compile_options(sparqlParser PRIVATE -Wno-logical-op-parentheses -Wno-parentheses)
diff --git a/src/parser/sparqlParser/generated/CMakeLists.txt b/src/parser/sparqlParser/generated/CMakeLists.txt
diff --git a/src/parser/sparqlParser/generated/README.md b/src/parser/sparqlParser/generated/README.md
@@ -10,12 +10,12 @@ the workflow is as follows:
 
 * Inside this directory, Download the ANTLR parser generator
 ```
-wget http://www.antlr.org/download/antlr-4.9.2-complete.jar
+wget http://www.antlr.org/download/antlr-4.11.1-complete.jar
 ```
 * Inside this directory, Run ANTLR
 ```
-java -Xmx500M -cp "./antlr-4.9.2-complete.jar:$CLASSPATH" org.antlr.v4.Tool -Dlanguage=Cpp SparqlAutomatic.g4 -visitor
+java -Xmx500M -cp "./antlr-4.11.1-complete.jar:$CLASSPATH" org.antlr.v4.Tool -Dlanguage=Cpp SparqlAutomatic.g4 -visitor
 ```
 
 * If necessary, adapt the `SparqlQleverVisitor.h/.cpp` (one folder above)
-* Commit all the changed files (except the `antlr.jar`)
+* Commit all the changed files (except the `antlr.jar`)
diff --git a/src/parser/sparqlParser/generated/SparqlAutomatic.interp b/src/parser/sparqlParser/generated/SparqlAutomatic.interp
diff --git a/src/parser/sparqlParser/generated/SparqlAutomaticBaseListener.cpp b/src/parser/sparqlParser/generated/SparqlAutomaticBaseListener.cpp
@@ -1,4 +1,4 @@
 
-// Generated from SparqlAutomatic.g4 by ANTLR 4.9.2
+// Generated from SparqlAutomatic.g4 by ANTLR 4.11.1
 
 #include "SparqlAutomaticBaseListener.h"
diff --git a/src/parser/sparqlParser/generated/SparqlAutomaticBaseListener.h b/src/parser/sparqlParser/generated/SparqlAutomaticBaseListener.h
@@ -1,5 +1,5 @@
 
-// Generated from SparqlAutomatic.g4 by ANTLR 4.9.2
+// Generated from SparqlAutomatic.g4 by ANTLR 4.11.1
 
 #pragma once
 

diff --git a/src/parser/sparqlParser/generated/SparqlAutomaticBaseVisitor.cpp b/src/parser/sparqlParser/generated/SparqlAutomaticBaseVisitor.cpp
@@ -1,4 +1,4 @@
 
-// Generated from SparqlAutomatic.g4 by ANTLR 4.9.2
+// Generated from SparqlAutomatic.g4 by ANTLR 4.11.1
 
 #include "SparqlAutomaticBaseVisitor.h"