build: add support for Clang-Tidy tool.

.bazelrc

@@ -8,6 +8,11 @@ build:clang --action_env=BAZEL_COMPILER=clang
 build:clang --action_env=CC=clang
 build:clang --action_env=CXX=clang++
 
+# Use Clang-Tidy tool.
+build:clang-tidy --aspects @bazel_clang_tidy//clang_tidy:clang_tidy.bzl%clang_tidy_aspect
+build:clang-tidy --@bazel_clang_tidy//:clang_tidy_config=@proxy_wasm_cpp_host//:clang_tidy_config
+build:clang-tidy --output_groups=report
+
 # Use GCC compiler.
 build:gcc --action_env=BAZEL_COMPILER=gcc
 build:gcc --action_env=CC=gcc

.clang-tidy

@@ -0,0 +1,25 @@
+Checks:            clang-*,
+                   -clang-analyzer-optin.portability.UnixAPI,
+                   -clang-analyzer-unix.Malloc,
+                   -clang-diagnostic-pragma-once-outside-header,
+                   cppcoreguidelines-pro-type-member-init,
+                   cppcoreguidelines-pro-type-static-cast-downcast,
+                   misc-*,
+                   -misc-non-private-member-variables-in-classes,
+                   modernize-*,
+                   -modernize-avoid-c-arrays,
+                   -modernize-return-braced-init-list,
+                   -modernize-use-trailing-return-type,
+                   llvm-include-order,
+                   performance-*,
+                   -performance-no-int-to-ptr,
+                   portability-*,
+                   readability-*,
+                   -readability-convert-member-functions-to-static,
+                   -readability-function-cognitive-complexity,
+                   -readability-inconsistent-declaration-parameter-name,
+                   -readability-magic-numbers,
+                   -readability-make-member-function-const,
+                   -readability-simplify-boolean-expr,
+
+WarningsAsErrors:  '*'

.github/workflows/format.yml

@@ -28,6 +28,9 @@ on:
     - 'envoy-release/**'
     - 'istio-release/**'
 
+  schedule:
+    - cron: '0 0 * * *'
+
 jobs:
 
   addlicense:
@@ -105,3 +108,61 @@ jobs:
       run: |
         find . -name "*.h" -o -name "*.cc" -o -name "*.proto" | grep -v ".pb." | xargs -n1 clang-format-12 -i
         git diff --exit-code
+
+  clang_tidy:
+    name: check format with clang-tidy
+
+    runs-on: ubuntu-20.04
+
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Install dependencies (Linux)
+      run: sudo apt-get install -y clang-tidy-12
+
+    - name: Bazel cache
+      uses: PiotrSikora/[email protected]
+      with:
+        path: |
+          ~/.cache/bazel
+        key: clang_tidy-${{ hashFiles('WORKSPACE', '.bazelrc', '.bazelversion', 'bazel/dependencies.bzl', 'bazel/repositories.bzl', 'bazel/cargo/wasmsign/crates.bzl') }}
+
+    - name: Bazel build
+      run: >
+        bazel build
+        --config clang-tidy
+        --define engine=multi
+        --copt=-DPROXY_WASM_VERIFY_WITH_ED25519_PUBKEY=\"$(xxd -p -c 256 test/test_data/signature_key1.pub | cut -b9-)\"
+        //...
+
+#    - name: Skip Bazel cache update
+#      if: ${{ github.ref != 'refs/heads/master' }}
+#      run: echo "CACHE_SKIP_SAVE=true" >> $GITHUB_ENV
+
+    - name: Cleanup Bazel cache
+#      if: ${{ github.ref == 'refs/heads/master' }}
+      run: |
+        export OUTPUT=$(${{ matrix.run_under }} bazel info output_base)
+        echo "===== BEFORE ====="
+        du -s ${OUTPUT}/external/* $(dirname ${OUTPUT})/* | sort -rn | head -20
+        # BoringSSL's test data (90 MiB).
+        rm -rf ${OUTPUT}/external/boringssl/crypto_test_data.cc
+        rm -rf ${OUTPUT}/external/boringssl/src/crypto/*/test/
+        rm -rf ${OUTPUT}/external/boringssl/src/third_party/wycheproof_testvectors/
+        # LLVM's tests (500 MiB).
+        rm -rf ${OUTPUT}/external/llvm*/test/
+        # V8's tests (100 MiB).
+        if [ -d "${OUTPUT}/external/v8/test/torque" ]; then
+          mv ${OUTPUT}/external/v8/test/torque ${OUTPUT}/external/v8/test_torque
+          rm -rf ${OUTPUT}/external/v8/test/*
+          mv ${OUTPUT}/external/v8/test_torque ${OUTPUT}/external/v8/test/torque
+        fi
+        # Unnecessary CMake tools (65 MiB).
+        rm -rf ${OUTPUT}/external/cmake-*/bin/{ccmake,cmake-gui,cpack,ctest}
+        # Distfiles for Rust toolchains (350 MiB).
+        rm -rf ${OUTPUT}/external/rust_*/*.tar.gz
+        # Bazel's repository cache (650-800 MiB) and install base (155 MiB).
+        rm -rf ${OUTPUT}/../cache
+        rm -rf ${OUTPUT}/../install
+        echo "===== AFTER ====="
+        du -s ${OUTPUT}/external/* $(dirname ${OUTPUT})/* | sort -rn | head -20

BUILD

@@ -14,6 +14,11 @@ package(default_visibility = ["//visibility:public"])
 
 exports_files(["LICENSE"])
 
+filegroup(
+    name = "clang_tidy_config",
+    data = [".clang-tidy"],
+)
+
 cc_library(
     name = "wasm_vm_headers",
     hdrs = [
@@ -191,7 +196,7 @@ cc_library(
     ],
     hdrs = ["include/proxy-wasm/wavm.h"],
     copts = [
-        '-DWAVM_API=""',
+        "-DWAVM_API=",
         "-Wno-non-virtual-dtor",
         "-Wno-old-style-cast",
     ],

bazel/external/bazel_clang_tidy.patch

@@ -0,0 +1,27 @@
+# 1. Treat .h files as C++ headers.
+# 2. Hardcode clang-tidy-12.
+
+diff --git a/clang_tidy/clang_tidy.bzl b/clang_tidy/clang_tidy.bzl
+index 3a5ed07..5db5c6c 100644
+--- a/clang_tidy/clang_tidy.bzl
++++ b/clang_tidy/clang_tidy.bzl
+@@ -16,6 +16,9 @@ def _run_tidy(ctx, exe, flags, compilation_context, infile, discriminator):
+     # add source to check
+     args.add(infile.path)
+
++    # treat .h files as C++ headers
++    args.add("--extra-arg-before=-xc++")
++
+     # start args passed to the compiler
+     args.add("--")
+
+diff --git a/clang_tidy/run_clang_tidy.sh b/clang_tidy/run_clang_tidy.sh
+index 582bab1..b9ebb94 100755
+--- a/clang_tidy/run_clang_tidy.sh
++++ b/clang_tidy/run_clang_tidy.sh
+@@ -11,4 +11,4 @@ shift
+ touch $OUTPUT
+ truncate -s 0 $OUTPUT
+
+-clang-tidy "$@"
++clang-tidy-12 "$@"

bazel/repositories.bzl

@@ -29,6 +29,16 @@ def proxy_wasm_cpp_host_repositories():
         sha256 = "c6966ec828da198c5d9adbaa94c05e3a1c7f21bd012a0b29ba8ddbccb2c93b0d",
     )
 
+    maybe(
+        http_archive,
+        name = "bazel_clang_tidy",
+        sha256 = "6ed23cbff9423a30ef10becf57210a26d54fe198a211f4037d931c06f843c023",
+        strip_prefix = "bazel_clang_tidy-c2fe98cfec0430e78bff4169e9ca0a43123e4c99",
+        url = "https://github.com/erenon/bazel_clang_tidy/archive/c2fe98cfec0430e78bff4169e9ca0a43123e4c99.tar.gz",
+        patches = ["@proxy_wasm_cpp_host//bazel/external:bazel_clang_tidy.patch"],
+        patch_args = ["-p1"],
+    )
+
     maybe(
         http_archive,
         name = "bazel-zig-cc",

include/proxy-wasm/context.h

@@ -143,11 +143,11 @@ class ContextBase : public RootInterface,
                     public SharedQueueInterface,
                     public GeneralInterface {
 public:
-  ContextBase();                                                   // Testing.
-  ContextBase(WasmBase *wasm);                                     // Vm Context.
-  ContextBase(WasmBase *wasm, std::shared_ptr<PluginBase> plugin); // Root Context.
+  ContextBase();                                                          // Testing.
+  ContextBase(WasmBase *wasm);                                            // Vm Context.
+  ContextBase(WasmBase *wasm, const std::shared_ptr<PluginBase> &plugin); // Root Context.
   ContextBase(WasmBase *wasm, uint32_t parent_context_id,
-              std::shared_ptr<PluginHandleBase> plugin_handle); // Stream context.
+              const std::shared_ptr<PluginHandleBase> &plugin_handle); // Stream context.
   virtual ~ContextBase();
 
   WasmBase *wasm() const { return wasm_; }

include/proxy-wasm/exports.h

@@ -57,7 +57,7 @@ struct RegisterForeignFunction {
    * @param function_name is the key for this foreign function.
    * @param f is the function instance.
    */
-  RegisterForeignFunction(std::string function_name, WasmForeignFunction f);
+  RegisterForeignFunction(const std::string &function_name, WasmForeignFunction f);
 };
 
 namespace exports {

include/proxy-wasm/null_vm.h

@@ -36,7 +36,7 @@ struct NullVm : public WasmVm {
   Cloneable cloneable() override { return Cloneable::InstantiatedModule; };
   std::unique_ptr<WasmVm> clone() override;
   bool load(std::string_view bytecode, std::string_view precompiled,
-            std::unordered_map<uint32_t, std::string> function_names) override;
+            const std::unordered_map<uint32_t, std::string> &function_names) override;
   bool link(std::string_view debug_name) override;
   uint64_t getMemorySize() override;
   std::optional<std::string_view> getMemory(uint64_t pointer, uint64_t size) override;

include/proxy-wasm/vm_id_handle.h

@@ -31,6 +31,6 @@ class VmIdHandle {
 };
 
 std::shared_ptr<VmIdHandle> getVmIdHandle(std::string_view vm_id);
-void registerVmIdHandleCallback(std::function<void(std::string_view vm_id)> f);
+void registerVmIdHandleCallback(const std::function<void(std::string_view vm_id)> &f);
 
 } // namespace proxy_wasm

include/proxy-wasm/wasm.h

@@ -52,15 +52,15 @@ class WasmBase : public std::enable_shared_from_this<WasmBase> {
            std::string_view vm_configuration, std::string_view vm_key,
            std::unordered_map<std::string, std::string> envs,
            AllowedCapabilitiesMap allowed_capabilities);
-  WasmBase(const std::shared_ptr<WasmHandleBase> &other, WasmVmFactory factory);
+  WasmBase(const std::shared_ptr<WasmHandleBase> &other, const WasmVmFactory &factory);
   virtual ~WasmBase();
 
   bool load(const std::string &code, bool allow_precompiled = false);
   bool initialize();
   void startVm(ContextBase *root_context);
   bool configure(ContextBase *root_context, std::shared_ptr<PluginBase> plugin);
   // Returns the root ContextBase or nullptr if onStart returns false.
-  ContextBase *start(std::shared_ptr<PluginBase> plugin);
+  ContextBase *start(const std::shared_ptr<PluginBase> &plugin);
 
   std::string_view vm_id() const { return vm_id_; }
   std::string_view vm_key() const { return vm_key_; }
@@ -340,9 +340,11 @@ using WasmHandleCloneFactory =
     std::function<std::shared_ptr<WasmHandleBase>(std::shared_ptr<WasmHandleBase> wasm)>;
 
 // Returns nullptr on failure (i.e. initialization of the VM fails).
-std::shared_ptr<WasmHandleBase>
-createWasm(std::string vm_key, std::string code, std::shared_ptr<PluginBase> plugin,
-           WasmHandleFactory factory, WasmHandleCloneFactory clone_factory, bool allow_precompiled);
+std::shared_ptr<WasmHandleBase> createWasm(const std::string &vm_key, const std::string &code,
+                                           const std::shared_ptr<PluginBase> &plugin,
+                                           const WasmHandleFactory &factory,
+                                           const WasmHandleCloneFactory &clone_factory,
+                                           bool allow_precompiled);
 // Get an existing ThreadLocal VM matching 'vm_id' or nullptr if there isn't one.
 std::shared_ptr<WasmHandleBase> getThreadLocalWasm(std::string_view vm_id);
 
@@ -371,8 +373,8 @@ using PluginHandleFactory = std::function<std::shared_ptr<PluginHandleBase>(
 // Get an existing ThreadLocal VM matching 'vm_id' or create one using 'base_wavm' by cloning or by
 // using it it as a template.
 std::shared_ptr<PluginHandleBase> getOrCreateThreadLocalPlugin(
-    std::shared_ptr<WasmHandleBase> base_wasm, std::shared_ptr<PluginBase> plugin,
-    WasmHandleCloneFactory clone_factory, PluginHandleFactory plugin_factory);
+    const std::shared_ptr<WasmHandleBase> &base_wasm, const std::shared_ptr<PluginBase> &plugin,
+    const WasmHandleCloneFactory &clone_factory, const PluginHandleFactory &plugin_factory);
 
 // Clear Base Wasm cache and the thread-local Wasm sandbox cache for the calling thread.
 void clearWasmCachesForTesting();
@@ -403,7 +405,7 @@ inline uint64_t WasmBase::copyString(std::string_view s) {
   if (s.empty()) {
     return 0; // nullptr
   }
-  uint64_t pointer;
+  uint64_t pointer = 0;
   uint8_t *m = static_cast<uint8_t *>(allocMemory((s.size() + 1), &pointer));
   memcpy(m, s.data(), s.size());
   m[s.size()] = 0;

include/proxy-wasm/wasm_vm.h

@@ -214,7 +214,7 @@ class WasmVm {
    * @return whether or not the load was successful.
    */
   virtual bool load(std::string_view bytecode, std::string_view precompiled,
-                    const std::unordered_map<uint32_t, std::string> function_names) = 0;
+                    const std::unordered_map<uint32_t, std::string> &function_names) = 0;
 
   /**
    * Link the WASM code to the host-provided functions, e.g. the ABI. Prior to linking, the module

src/bytecode_util.cc

@@ -21,7 +21,7 @@ namespace proxy_wasm {
 bool BytecodeUtil::checkWasmHeader(std::string_view bytecode) {
   // Wasm file header is 8 bytes (magic number + version).
   static const uint8_t wasm_magic_number[4] = {0x00, 0x61, 0x73, 0x6d};
-  return bytecode.size() < 8 || !::memcmp(bytecode.data(), wasm_magic_number, 4);
+  return (bytecode.size() < 8 || ::memcmp(bytecode.data(), wasm_magic_number, 4) == 0);
 }
 
 bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersion &ret) {
@@ -55,7 +55,7 @@ bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersi
         if (!parseVarint(pos, end, export_name_size) || pos + export_name_size > end) {
           return false;
         }
-        const auto name_begin = pos;
+        const auto *const name_begin = pos;
         pos += export_name_size;
         if (pos + 1 > end) {
           return false;
@@ -67,10 +67,12 @@ bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersi
           if (export_name == "proxy_abi_version_0_1_0") {
             ret = AbiVersion::ProxyWasm_0_1_0;
             return true;
-          } else if (export_name == "proxy_abi_version_0_2_0") {
+          }
+          if (export_name == "proxy_abi_version_0_2_0") {
             ret = AbiVersion::ProxyWasm_0_2_0;
             return true;
-          } else if (export_name == "proxy_abi_version_0_2_1") {
+          }
+          if (export_name == "proxy_abi_version_0_2_1") {
             ret = AbiVersion::ProxyWasm_0_2_1;
             return true;
           }
@@ -81,9 +83,8 @@ bool BytecodeUtil::getAbiVersion(std::string_view bytecode, proxy_wasm::AbiVersi
         }
       }
       return true;
-    } else {
-      pos += section_len;
     }
+    pos += section_len;
   }
   return true;
 }
@@ -109,7 +110,7 @@ bool BytecodeUtil::getCustomSection(std::string_view bytecode, std::string_view
     }
     if (section_type == 0) {
       // Custom section.
-      const auto section_data_start = pos;
+      const auto *const section_data_start = pos;
       uint32_t section_name_len = 0;
       if (!BytecodeUtil::parseVarint(pos, end, section_name_len) || pos + section_name_len > end) {
         return false;
@@ -149,7 +150,7 @@ bool BytecodeUtil::getFunctionNameIndex(std::string_view bytecode,
         pos += subsection_size;
       } else {
         // Enters function name subsection.
-        const auto start = pos;
+        const auto *const start = pos;
         uint32_t namemap_vector_size = 0;
         if (!parseVarint(pos, end, namemap_vector_size) || pos + namemap_vector_size > end) {
           return false;
@@ -186,7 +187,7 @@ bool BytecodeUtil::getStrippedSource(std::string_view bytecode, std::string &ret
   const char *pos = bytecode.data() + 8;
   const char *end = bytecode.data() + bytecode.size();
   while (pos < end) {
-    const auto section_start = pos;
+    const auto *const section_start = pos;
     if (pos + 1 > end) {
       return false;
     }
@@ -196,7 +197,7 @@ bool BytecodeUtil::getStrippedSource(std::string_view bytecode, std::string &ret
       return false;
     }
     if (section_type == 0 /* custom section */) {
-      const auto section_data_start = pos;
+      const auto *const section_data_start = pos;
       uint32_t section_name_len = 0;
       if (!parseVarint(pos, end, section_name_len) || pos + section_name_len > end) {
         return false;

src/common/types.h

@@ -16,8 +16,7 @@
 
 #include <memory>
 
-namespace proxy_wasm {
-namespace common {
+namespace proxy_wasm::common {
 
 template <typename T, void (*deleter)(T *)>
 class CSmartPtr : public std::unique_ptr<T, void (*)(T *)> {
@@ -36,5 +35,4 @@ template <typename T, void (*initializer)(T *), void (*deleter)(T *)> class CSma
   T item;
 };
 
-} // namespace common
-} // namespace proxy_wasm
+} // namespace proxy_wasm::common