KUDU-3374 Add support for M1 and macOS Monterey

The macOS Monterey OS upgrade broke glog 0.3.5, moreover building Kudu
with LLVM 9 on Apple silicon is not really feasible. There has been
multiple non-merged patches submitted to tackle these issues:
Upgrade glog to 0.6.0 [1]
Fix codegen build on MacOS Monterey [2]
Fix building on Apple M1 [3]
Upgrade to LLVM 11 and IWYU 0.15 [4]
This patch squashes all of the above and provides the necessary
glue changes.

LLVM is updated to version 11 and glog to version 0.6.0 to fix the
initial build issues. Building the glog tests has to be turned off as
it causes linker error in TSAN build. The optional ZLIB dependency in
LLVM-IWYU is removed as the original issue -mentioned in the comments-
has been resolved, and this caused build issues on Unix. [5] The
homebrew prefix is changed from /usr/local to /opt/homebrew on ARM macs.
In places where package locations are provided, an ARM alternative
location has been added. Added a patch [6] to fix null pointer
dereference in rapidjson. Added another patch [7] containing assertions
to a similar suppress clang warnings in rapidjson. Building tests in
glog has to be turned off [8] as it causes linker error in tsan build.
With the clang upgrade it now links against libatomic in TSAN builds. In
dist-test.py libatomic is added to the list of shipped libraries as it
was missing on the target machines. A couple of new TSAN races
came up with LLVM regexes, added those to the sanitizer suppression
list.

[1] Change-Id: https://gerrit.cloudera.org/#/c/18460/
[2] Change-Id: https://gerrit.cloudera.org/#/c/18461/
[3] Change-Id: https://gerrit.cloudera.org/#/c/18464/
[4] Change-Id: https://gerrit.cloudera.org/#/c/16768/
[5] https://github.com/include-what-you-use/include-what-you-use/
    issues/539
[6] Tencent/rapidjson#727
[7] Tencent/rapidjson#757
[8] google/glog#54

Change-Id: I9877f95340b969308c317a6bac50665ff78e329e

CMakeLists.txt

@@ -165,6 +165,7 @@ if ("$ENV{NO_REBUILD_THIRDPARTY}" STREQUAL "")
   endif()
 endif()
 
+
 ############################################################
 # Compiler flags
 ############################################################

build-support/dist_test.py

@@ -241,6 +241,10 @@ def is_lib_whitelisted(lib):
     # installed versions different from the dist_test image.
     if "libcrypto" in lib or "libsasl2" in lib or "libssl" in lib:
       return True
+    # After upgrading to LLVM 11, TSAN builds link libatomic.
+    # Since it was not present on the test machines, it is whitelisted.
+    if "libatomic" in lib:
+      return True
     return False
   return True
 

build-support/iwyu/mappings/glog.imp

@@ -23,5 +23,10 @@
   { symbol: [ "CHECK_GE", private, "<glog/logging.h>", public ] },
   { symbol: [ "CHECK_GT", private, "<glog/logging.h>", public ] },
   { symbol: [ "ErrnoLogMessage", private, "<glog/logging.h>", public ] },
-  { symbol: [ "COMPACT_GOOGLE_LOG_0", private, "<glog/logging.h>", public ] }
+  { symbol: [ "COMPACT_GOOGLE_LOG_0", private, "<glog/logging.h>", public ] },
+
+# Upgrading from glog 0.3.5 to 0.6.0 the includes for log_severity.h and vlog_is_on.h
+# are changed from #include "" to #include <> which seem to cause issues.
+  { include: ["<glog/log_severity.h>", "public", "<glog/logging.h>", "public"] },
+  { include: ["<glog/vlog_is_on.h>", "public", "<glog/logging.h>", "public"] }
 ]

cmake_modules/FindKerberosPrograms.cmake

@@ -28,6 +28,8 @@ foreach(bin ${bins})
                /usr/sbin
                # Homebrew install location.
                /usr/local/opt/krb5/sbin
+               # Homebrew arm install location
+               /opt/homebrew/opt/krb5/sbin
                # Macports install location.
                /opt/local/sbin
                # SLES

src/kudu/codegen/CMakeLists.txt

@@ -69,7 +69,7 @@ if (APPLE)
   # Clang does not know about by default.
   set(PREFIXED_IR_INCLUDES
     ${PREFIXED_IR_INCLUDES}
-    -cxx-isystem "/Library/Developer/CommandLineTools/usr/include/c++/v1")
+    -cxx-isystem "${CMAKE_OSX_SYSROOT}")
 endif()
 
 # Get preprocessing definitions, which enable directives for glog and gtest.

src/kudu/codegen/code_generator.cc

@@ -158,14 +158,14 @@ int DumpAsm(FuncPtr fptr, const TargetMachine& tm, std::ostream* out, int max_in
     MCInst inst;
     uint64_t size;
     MCDisassembler::DecodeStatus stat =
-      disas->getInstruction(inst, size, mem_obj.slice(addr), addr, llvm::nulls(), llvm::nulls());
+      disas->getInstruction(inst, size, mem_obj.slice(addr), addr, llvm::nulls());
     if (stat != MCDisassembler::Success) {
       *out << "<ERROR at 0x" << std::hex << addr
            << " (absolute 0x" << (addr + base_addr) << ")"
            << ", skipping instruction>\n" << std::dec;
     } else {
       string annotations;
-      printer->printInst(&inst, os, annotations, subtarget_info);
+      printer->printInst(&inst, addr, annotations, subtarget_info, os);
       os << " " << annotations << "\n";
       // We need to check the opcode name for "RET" instead of comparing
       // the opcode to llvm::ReturnInst::getOpcode() because the native
@@ -178,7 +178,7 @@ int DumpAsm(FuncPtr fptr, const TargetMachine& tm, std::ostream* out, int max_in
       // LLVM RTTI, since subclassing an LLVM interface would require
       // identical RTTI settings between LLVM and Kudu (see:
       // http://llvm.org/docs/Packaging.html#c-features).
-      string opname = printer->getOpcodeName(inst.getOpcode());
+      string opname = printer->getOpcodeName(inst.getOpcode()).str();
       std::transform(opname.begin(), opname.end(), opname.begin(), ::toupper);
       if (opname.find("RET") != string::npos) return i + 1;
     }

src/kudu/codegen/module_builder.cc

@@ -29,7 +29,10 @@
 //       for successful run-time operation of the code generator.
 #include <glog/logging.h>
 #include <llvm/ADT/StringMap.h>
+#include <llvm/ADT/StringMapEntry.h>
 #include <llvm/ADT/StringRef.h>
+#include <llvm/ADT/ilist_iterator.h>
+#include <llvm/ADT/iterator.h>
 #include <llvm/ExecutionEngine/ExecutionEngine.h>
 #include <llvm/ExecutionEngine/MCJIT.h> // IWYU pragma: keep
 #include <llvm/IR/Attributes.h>
@@ -125,7 +128,7 @@ string ToString(const Module& m) {
 // This method is needed for the implicit conversion from
 // llvm::StringRef to std::string
 string ToString(const Function* f) {
-  return f->getName();
+  return f->getName().str();
 }
 
 bool ModuleContains(const Module& m, const Function* fptr) {
@@ -379,7 +382,7 @@ TargetMachine* ModuleBuilder::GetTargetMachine() const {
 unordered_set<string> ModuleBuilder::GetFunctionNames() const {
   unordered_set<string> ret;
   for (const JITFuture& fut : futures_) {
-    ret.insert(CHECK_NOTNULL(fut.llvm_f_)->getName());
+    ret.insert(CHECK_NOTNULL(fut.llvm_f_)->getName().str());
   }
   return ret;
 }

src/kudu/common/row_operations-test.cc

@@ -23,6 +23,7 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 #include <gflags/gflags_declare.h>
@@ -298,7 +299,7 @@ TEST_F(RowOperationsTest, SchemaFuzz) {
     g_failing_case.server_schema = &server_schema;
     g_failing_case.row = &row;
     ASAN_SET_DEATH_CALLBACK(&DumpFailingCase);
-    google::InstallFailureFunction(&GlogFailure);
+    google::InstallFailureFunction(reinterpret_cast<google::logging_fail_func_t>(GlogFailure));
 
     for (int i = 0; i < client_schema.num_columns(); i++) {
       if (client_schema.column(i).is_nullable() &&
@@ -326,7 +327,7 @@ TEST_F(RowOperationsTest, SchemaFuzz) {
 
     DoFuzzTest(server_schema, row, 100);
     ASAN_SET_DEATH_CALLBACK(NULL);
-    google::InstallFailureFunction(&abort);
+    google::InstallFailureFunction(reinterpret_cast<google::logging_fail_func_t>(abort));
   }
 }
 

src/kudu/fs/dir_util-test.cc

@@ -82,15 +82,15 @@ TEST_F(KuduTest, Locking) {
 
   // Note: we must use a death test here because file locking is only
   // disallowed across processes, and death tests spawn child processes.
-  ASSERT_DEATH({
+  ASSERT_DEATH(({
     DirInstanceMetadataFile second(env_, "", kType, kFileName);
     CHECK_OK(second.LoadFromDisk());
     CHECK_EQ(kUuid, second.uuid());
     CHECK_OK(second.Lock());
-  }, "Could not lock");
+  }), "Could not lock");
 
   ASSERT_OK(first.Unlock());
-  ASSERT_DEATH({
+  ASSERT_DEATH(({
     DirInstanceMetadataFile second(env_, "", kType, kFileName);
     CHECK_OK(second.LoadFromDisk());
     CHECK_EQ(kUuid, second.uuid());
@@ -100,7 +100,7 @@ TEST_F(KuduTest, Locking) {
     } else {
       LOG(FATAL) << "Could not lock: " << s.ToString();
     }
-  }, "Lock successfully acquired");
+  }), "Lock successfully acquired");
 }
 
 } // namespace fs

src/kudu/gutil/atomicops-internals-macosx.h

@@ -270,11 +270,15 @@ inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) {
   *ptr = value;
 }
 
-// Issue the x86 "pause" instruction, which tells the CPU that we
-// are in a spinlock wait loop and should allow other hyperthreads
-// to run, not speculate memory access, etc.
+// Issue the x86 "pause" instruction (or "yield" on aarch64), which
+// tells the CPU that we are in a spinlock wait loop and should allow
+// other hyperthreads to run, not speculate memory access, etc.
 inline void PauseCPU() {
+#ifdef __aarch64__
+  __asm__ __volatile__("yield" : : : "memory");
+#else
   __asm__ __volatile__("pause" : : : "memory");
+#endif
 }
 
 inline void Acquire_Store(volatile Atomic64 *ptr, Atomic64 value) {

src/kudu/gutil/dynamic_annotations.c

@@ -113,7 +113,7 @@ void AnnotateBenignRace(const char *file, int line,
                         const char *description){}
 void AnnotateBenignRaceSized(const char *file, int line,
                              const volatile void *mem,
-                             long size,
+                             size_t size,
                              const char *description) {}
 void AnnotateMutexIsUsedAsCondVar(const char *file, int line,
                                   const volatile void *mu){}

src/kudu/gutil/dynamic_annotations.h

@@ -522,7 +522,7 @@ void AnnotateBenignRace(const char *file, int line,
                         const char *description);
 void AnnotateBenignRaceSized(const char *file, int line,
                         const volatile void *address,
-                        long size,
+                        size_t size,
                         const char *description);
 void AnnotateMutexIsUsedAsCondVar(const char *file, int line,
                                   const volatile void *mu);

src/kudu/ranger/ranger_client.cc

@@ -99,7 +99,7 @@ TAG_FLAG(ranger_logtostdout, advanced);
 TAG_FLAG(ranger_logtostdout, evolving);
 
 DECLARE_int32(max_log_files);
-DECLARE_int32(max_log_size);
+DECLARE_uint32(max_log_size);
 DECLARE_string(log_dir);
 
 METRIC_DEFINE_histogram(server, ranger_subprocess_execution_time_ms,

src/kudu/security/test/mini_kdc.cc

@@ -105,6 +105,8 @@ Status GetBinaryPath(const string& binary, string* path) {
   static const vector<string> kCommonLocations = {
     "/usr/local/opt/krb5/sbin", // Homebrew
     "/usr/local/opt/krb5/bin", // Homebrew
+    "/opt/homebrew/opt/krb5/sbin", // Homebrew arm
+    "/opt/homebrew/opt/krb5/bin", // Homebrew arm
     "/opt/local/sbin", // Macports
     "/opt/local/bin", // Macports
     "/usr/lib/mit/sbin", // SLES

src/kudu/server/diagnostics_log.cc

@@ -17,14 +17,14 @@
 
 #include "kudu/server/diagnostics_log.h"
 
+#include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
 #include <optional>
 #include <ostream>
 #include <queue>
 #include <string>
-#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -67,7 +67,7 @@ namespace google {
 // symbol name to "out".  The symbol name is demangled if possible
 // (supports symbols generated by GCC 3.x or newer).  Otherwise,
 // returns false.
-bool Symbolize(void *pc, char *out, int out_size);
+bool Symbolize(void *pc, char *out, size_t out_size);
 }
 
 DEFINE_int32(diagnostics_log_stack_traces_interval_ms, 60000,

src/kudu/server/pprof_path_handlers.cc

@@ -22,6 +22,7 @@
 #include <cstdint>
 #include <cstdlib>
 #include <fstream>
+#include <functional>
 #include <string>
 #include <unordered_map>
 #include <utility>
@@ -66,7 +67,7 @@ namespace google {
 // symbol name to "out".  The symbol name is demangled if possible
 // (supports symbols generated by GCC 3.x or newer).  Otherwise,
 // returns false.
-bool Symbolize(void *pc, char *out, int out_size);
+bool Symbolize(void *pc, char *out, size_t out_size);
 }
 
 namespace kudu {

src/kudu/tablet/concurrent_btree.h

@@ -90,6 +90,10 @@ struct BTreeTraits {
 
 template<class T>
 inline void PrefetchMemory(const T *addr) {
+  // During the LLVM 11 upgrade, the sanitizer runs reported null pointer dereference. Wrapping the
+  // prefetch() call with an if statement fixed the errors. In the end, it was decided to add a
+  // DCHECK() here, and wrap all uses of PrefetchMemory() with the null pointer guard.
+  DCHECK(addr);
   int size = std::min<int>(sizeof(T), 4 * CACHELINE_SIZE);
 
   for (int i = 0; i < size; i += CACHELINE_SIZE) {
@@ -1168,7 +1172,9 @@ class CBTree {
 
     while (node.type() != NodePtr<Traits>::LEAF_NODE) {
 #ifdef TRAVERSE_PREFETCH
-      PrefetchMemory(node.internal_node_ptr());
+      if (node.internal_node_ptr()) {
+        PrefetchMemory(node.internal_node_ptr());
+      }
 #endif
       retry_in_node:
       int num_children = node.internal_node_ptr()->num_children_;
@@ -1206,7 +1212,9 @@ class CBTree {
       version = child_version;
     }
 #ifdef TRAVERSE_PREFETCH
-    PrefetchMemory(node.leaf_node_ptr());
+    if (node.leaf_node_ptr()) {
+      PrefetchMemory(node.leaf_node_ptr());
+    }
 #endif
     *stable_version = version;
     return node.leaf_node_ptr();
@@ -1777,7 +1785,9 @@ class CBTreeIterator {
       AtomicVersion version;
       LeafNode<Traits> *leaf = tree_->TraverseToLeaf(key, &version);
 #ifdef SCAN_PREFETCH
-      PrefetchMemory(leaf->next_);
+      if (leaf->next_) {
+        PrefetchMemory(leaf->next_);
+      }
 #endif
 
       // If the tree is frozen, we don't need to follow optimistic concurrency.
@@ -1815,7 +1825,9 @@ class CBTreeIterator {
       return false;
     }
 #ifdef SCAN_PREFETCH
-    PrefetchMemory(next->next_);
+    if (next->next_) {
+      PrefetchMemory(next->next_);
+    }
     for (int i = 0; i < next->num_entries(); i++) {
       next->vals_[i].prefetch();
     }

src/kudu/util/async_logger.cc

@@ -61,7 +61,7 @@ void AsyncLogger::Stop() {
 void AsyncLogger::Write(bool force_flush,
                         time_t timestamp,
                         const char* message,
-                        int message_len) {
+                        size_t message_len) {
   {
     MutexLock l(lock_);
     DCHECK_EQ(state_, RUNNING);

src/kudu/util/async_logger.h

@@ -86,7 +86,7 @@ class AsyncLogger : public google::base::Logger {
   void Write(bool force_flush,
              time_t timestamp,
              const char* message,
-             int message_len) override;
+             size_t message_len) override;
 
   // Flush any buffered messages.
   void Flush() override;

src/kudu/util/debug-util.cc

@@ -108,7 +108,7 @@ extern int GetStackTrace(void** result, int max_depth, int skip_count);
 // symbol name to "out".  The symbol name is demangled if possible
 // (supports symbols generated by GCC 3.x or newer).  Otherwise,
 // returns false.
-bool Symbolize(void *pc, char *out, int out_size);
+bool Symbolize(void *pc, char *out, size_t out_size);
 
 namespace glog_internal_namespace_ {
 extern void DumpStackTraceToString(string *s);

src/kudu/util/debug/trace_logging.h

@@ -102,9 +102,9 @@ class TraceGLog {
   class TraceLogSink : public google::LogSink {
    public:
     explicit TraceLogSink(const char* category) : category_(category) {}
-    void send(google::LogSeverity severity, const char* full_filename,
+    void send(google::LogSeverity severity, const char*  /*full_filename*/,
               const char* base_filename, int line,
-              const struct ::tm* tm_time, const char* message,
+              const ::google::LogMessageTime& logmsgtime, const char* message,
               size_t message_len) override {
       // Rather than calling TRACE_EVENT_INSTANT here, we have to do it from
       // the destructor. This is because glog holds its internal mutex while
@@ -116,7 +116,7 @@ class TraceGLog {
       // we defer the tracing until the google::LogMessage has destructed and the
       // glog lock is available again.
       str_ = ToString(severity, base_filename, line,
-                      tm_time, message, message_len);
+                      logmsgtime, message, message_len);
     }
     virtual ~TraceLogSink() {
       TRACE_EVENT_INSTANT1(category_, "vlog", TRACE_EVENT_SCOPE_THREAD,

src/kudu/util/flags.cc

@@ -246,7 +246,7 @@ DECLARE_bool(logtostderr);
 TAG_FLAG(logtostderr, stable);
 TAG_FLAG(logtostderr, runtime);
 
-DECLARE_int32(max_log_size);
+DECLARE_uint32(max_log_size);
 TAG_FLAG(max_log_size, stable);
 TAG_FLAG(max_log_size, runtime);
 

src/kudu/util/logging-test.cc

@@ -29,6 +29,7 @@
 
 #include <glog/logging.h>
 #include <gmock/gmock-matchers.h>
+#include <gtest/gtest-matchers.h>
 #include <gtest/gtest.h>
 
 #include "kudu/gutil/strings/substitute.h"
@@ -108,7 +109,7 @@ class CountingLogger : public google::base::Logger {
   void Write(bool force_flush,
              time_t /*timestamp*/,
              const char* /*message*/,
-             int /*message_len*/) override {
+             size_t /*message_len*/) override {
     message_count_++;
     if (force_flush) {
       Flush();
@@ -222,9 +223,9 @@ TEST(LoggingTest, TestRedactionIllustrateUsage) {
   ASSERT_EQ("public=abc, private=def", KUDU_DISABLE_REDACTION(SomeComplexStringify("abc", "def")));
 
   // Or we can execute an entire scope with redaction disabled.
-  KUDU_DISABLE_REDACTION({
+  KUDU_DISABLE_REDACTION(({
     ASSERT_EQ("public=abc, private=def", SomeComplexStringify("abc", "def"));
-  });
+  }));
 }