From d62ae9ded7589440ec2d9491b52782e98c36e926 Mon Sep 17 00:00:00 2001 From: Michael Platings Date: Fri, 19 Jan 2024 18:02:07 +0000 Subject: [PATCH 1/2] Fix uninitialised member variable --- intrinsiccv/include/utils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/intrinsiccv/include/utils.h b/intrinsiccv/include/utils.h index 74f2f2d11..fdb6d46fd 100644 --- a/intrinsiccv/include/utils.h +++ b/intrinsiccv/include/utils.h @@ -80,6 +80,7 @@ class LoopUnroll final { size_t step) INTRINSICCV_STREAMING_COMPATIBLE : length_(length), step_(step), + index_(0), can_avoid_tail_(length >= step) {} // Loop unrolled four times. -- GitLab From 7da2895fad19cfc864891b7964a33e389ad581a5 Mon Sep 17 00:00:00 2001 From: Michael Platings Date: Fri, 19 Jan 2024 18:06:16 +0000 Subject: [PATCH 2/2] Run clang-tidy in CI Also make some tweaks to test code to either selectively disable checks or keep the checks happy. --- .clang-tidy | 16 ++++++++++++++++ intrinsiccv/include/utils.h | 4 ++++ scripts/ci.sh | 4 ++++ test/api/test_yuv_to_rgb.cpp | 2 ++ test/framework/array.h | 2 ++ test/framework/generator.h | 2 +- test/framework/test_array2d.cpp | 4 ++++ test/framework/test_kernel.cpp | 4 ++++ 8 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..2e81039e1 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,16 @@ +# SPDX-FileCopyrightText: 2024 Arm Limited and/or its affiliates +# +# SPDX-License-Identifier: Apache-2.0 + +--- +Checks: >- + -*, + cert-*, + clang-analyzer-core*, + clang-analyzer-cplusplus*, + clang-analyzer-unix.MismatchedDeallocator, + misc-new-delete-overloads +WarningsAsErrors: '*' +HeaderFilterRegex: '(intrinsiccv|test)/.*' +FormatStyle: google +... diff --git a/intrinsiccv/include/utils.h b/intrinsiccv/include/utils.h index fdb6d46fd..cdb7b8695 100644 --- a/intrinsiccv/include/utils.h +++ b/intrinsiccv/include/utils.h @@ -173,7 +173,11 @@ class LoopUnroll final { LoopUnroll &unroll_n_times(CallbackType callback) INTRINSICCV_STREAMING_COMPATIBLE { const size_t step = UnrollFactor * step_; + // In practice step will never be zero and we don't want to spend + // instructions on checking that. + // NOLINTBEGIN(clang-analyzer-core.DivideZero) const size_t max_index = remaining_length() / step; + // NOLINTEND(clang-analyzer-core.DivideZero) for (index_ = 0; index_ < max_index; ++index_) { callback(step); diff --git a/scripts/ci.sh b/scripts/ci.sh index 1fbb28faa..bd2bb544d 100755 --- a/scripts/ci.sh +++ b/scripts/ci.sh @@ -20,8 +20,12 @@ reuse lint # Build and run tests cmake -S . -B build -G Ninja \ + -DCMAKE_CXX_CLANG_TIDY=clang-tidy \ -DCMAKE_CXX_FLAGS="--coverage -g -O0" +# Workaround to avoid applying clang-tidy to files in build directory +echo '{"Checks": "-*,cppcoreguidelines-avoid-goto"}'>build/.clang-tidy + GTEST_OUTPUT=xml:$(pwd)/build/test-results/ \ ninja -C build check-intrinsiccv diff --git a/test/api/test_yuv_to_rgb.cpp b/test/api/test_yuv_to_rgb.cpp index 03de4d4bf..876e4dce5 100644 --- a/test/api/test_yuv_to_rgb.cpp +++ b/test/api/test_yuv_to_rgb.cpp @@ -58,7 +58,9 @@ class YuvTest final { for (size_t vindex = 0; vindex < exp_arr.height(); vindex++) { for (size_t hindex = 0; hindex < exp_arr.width() / channel_number_; hindex++) { + // NOLINTBEGIN(clang-analyzer-core.uninitialized.Assign) int32_t y = *y_arr.at(vindex, hindex); + // NOLINTEND(clang-analyzer-core.uninitialized.Assign) y = std::max(0, y - 16); int32_t u = *uv_arr.at(vindex / 2, (hindex - hindex % 2) + (is_nv21 ? 1 : 0)); diff --git a/test/framework/array.h b/test/framework/array.h index e5f9e4bd9..1b392ad65 100644 --- a/test/framework/array.h +++ b/test/framework/array.h @@ -57,6 +57,7 @@ class Array2D : public TwoDimensional { /// Copy assignment operator. Array2D &operator=(const Array2D &other) { + if (this == &other) return *this; width_ = other.width_; height_ = other.height_; channels_ = other.channels_; @@ -74,6 +75,7 @@ class Array2D : public TwoDimensional { /// Move assignment operator. Array2D &operator=(Array2D &&other) { + if (this == &other) return *this; data_ = std::move(other.data_); width_ = other.width_; height_ = other.height_; diff --git a/test/framework/generator.h b/test/framework/generator.h index 34c56af53..42eb5c9df 100644 --- a/test/framework/generator.h +++ b/test/framework/generator.h @@ -16,7 +16,7 @@ namespace test { template class PseudoRandomNumberGenerator : public Generator { public: - PseudoRandomNumberGenerator() : seed_{Options::seed()} { reset(); } + PseudoRandomNumberGenerator() : seed_{Options::seed()}, rng_{seed_} {} /// Resets the generator to the initial state. void reset() override { rng_.seed(seed_); } diff --git a/test/framework/test_array2d.cpp b/test/framework/test_array2d.cpp index 7aed9667a..a26c25bd1 100644 --- a/test/framework/test_array2d.cpp +++ b/test/framework/test_array2d.cpp @@ -75,11 +75,15 @@ TEST(Array2D, MoveAssignment) { EXPECT_EQ(array_2.stride(), width * sizeof(uint32_t)); EXPECT_TRUE(array_2.valid()); + // This test is specifically to find out what happens to an object after it is + // moved so disable this static analysis check. + // NOLINTBEGIN(clang-analyzer-cplusplus.Move) EXPECT_EQ(array_1.width(), 0); EXPECT_EQ(array_1.height(), 0); EXPECT_EQ(array_1.channels(), 0); EXPECT_EQ(array_1.stride(), 0); EXPECT_FALSE(array_1.valid()); + // NOLINTEND(clang-analyzer-cplusplus.Move) } /// Tests that test::Array2D.at() works for set/get. diff --git a/test/framework/test_kernel.cpp b/test/framework/test_kernel.cpp index 2818859a7..30b5f5018 100644 --- a/test/framework/test_kernel.cpp +++ b/test/framework/test_kernel.cpp @@ -150,6 +150,9 @@ class ExampleKernelTest : public test::KernelTest { size_t border_count_{0}; }; // end of class class ExampleKernelTest +// Disable check for possible exceptions thrown outside main. +// The check isn't important in a test program. +// NOLINTBEGIN(cert-err58-cpp) template const std::array, 2> ExampleKernelTest::kKernels = { @@ -158,6 +161,7 @@ const std::array, 2> test::Kernel{ test::Array2D{4, 4}}, }; +// NOLINTEND(cert-err58-cpp) /// Tests that KernelTest::test() works. TEST(KernelTest, Test) { -- GitLab