Skip to content

Conversation

@krisukox
Copy link
Contributor

@krisukox krisukox commented Dec 7, 2022

This PR adds clang-tidy rule which is written using action API.
I had to introduce two tags (LIBRARY and BINARY) for swift_cc_library and swift_cc_binary in order to distinguish them from swift_cc_tool_library and swift_cc_tool.

Similar to the clang-format rule, I added a script that chooses the correct version of clang-tidy.

Run clang-tidy check with the default .clang-tidy config file:
bazel build --aspects @rules_swiftnav//clang_tidy:clang_tidy.bzl%clang_tidy_aspect --output_groups=report //...

In order to run clang-tidy check with a custom config we have to define it in a BUILD file and pass path to the --@rules_swiftnav//clang_tidy:clang_tidy_config flag:
bazel build --aspects @rules_swiftnav//clang_tidy:clang_tidy.bzl%clang_tidy_aspect --output_groups=report --@rules_swiftnav//clang_tidy:clang_tidy_config=//:clang_tidy_config //...

@krisukox krisukox force-pushed the krisukox/clang-tidy branch 2 times, most recently from cd83ddd to 0a53ff2 Compare December 8, 2022 19:13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to replace GENDIR variable manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compilation_contexts from implementation_deps must be manually added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround for gflag that I described in the ticket.

@krisukox krisukox marked this pull request as ready for review December 9, 2022 08:25
- update .clang-tidy config
- fix implementation_deps
- fix gflag
- add LIBRARY and BINARY tags to cc/defs
- use valid .clang-tidy config
- fix (GENDIR)
OUTPUT=$1
shift

# .clang-tidy config file has to be placed in the current working directory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be the piece that was missing in the clang_format implementation that was preventing it from working unless the project provided the .clang_format file.

Copy link
Contributor

@jungleraptor jungleraptor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @krisukox. I'm going to go ahead and merge this.

@jungleraptor jungleraptor merged commit f28ca34 into main Dec 23, 2022
@jungleraptor jungleraptor deleted the krisukox/clang-tidy branch December 23, 2022 21:37
reimerix added a commit that referenced this pull request Feb 3, 2025
With this change, UBSAN failures in CI should now have symbol
information, facilitating hunting done these issues:

```
[----------] 5 tests from ValidatorTest
[ RUN      ] ValidatorTest.FailsWithSanityChecker
external/starling-core/public_types/include/public_types/optional/optional_object.h:904:46: runtime error: load of value 48, which is not a valid value for type 'bool'
error: failed to decompress '.debug_aranges', zlib is not available
error: failed to decompress '.debug_info', zlib is not available
error: failed to decompress '.debug_abbrev', zlib is not available
error: failed to decompress '.debug_line', zlib is not available
error: failed to decompress '.debug_str', zlib is not available
error: failed to decompress '.debug_loc', zlib is not available
error: failed to decompress '.debug_ranges', zlib is not available
    #0 0x1576c5c in swift::optional<double>::operator=(swift::optional<double>&&) /proc/self/cwd/external/starling-core/public_types/include/public_types/optional/optional_object.h
    #1 0x185142c in sensorfusion::InertialNavigationSystem::CorrectedInertialData::operator=(sensorfusion::InertialNavigationSystem::CorrectedInertialData&&) /proc/self/cwd/sensorfusion/include/sensorfusion/deadreckoning/inertial_navigation_system.h:49:10
    #2 0x1851386 in sensorfusion::InertialNavigationSystem::InertialState::operator=(sensorfusion::InertialNavigationSystem::InertialState&&) /proc/self/cwd/sensorfusion/include/sensorfusion/deadreckoning/inertial_navigation_system.h:70:10
    #3 0x1ac4f8d in sensorfusion::InertialNavigationSystem::reset() /proc/self/cwd/sensorfusion/src/deadreckoning/inertial_navigation_system.cc:37:19
    #4 0x1a0a2fd in sensorfusion::ErrorEstimator::reset_filter() /proc/self/cwd/sensorfusion/src/deadreckoning/error_ekf.cc:130:9
    #5 0x1a09919 in sensorfusion::ErrorEstimator::ErrorEstimator(sensorfusion::configuration::Configuration const&, sensorfusion::InertialNavigationSystem*) /proc/self/cwd/sensorfusion/src/deadreckoning/error_ekf.cc:51:3
    #6 0x1916708 in (anonymous namespace)::ValidatorTest::ValidatorTest() /proc/self/cwd/sensorfusion/test/unit/test_sanity_checkers.cc:48:9
    #7 0x19165c0 in (anonymous namespace)::ValidatorTest_FailsWithSanityChecker_Test::ValidatorTest_FailsWithSanityChecker_Test() /proc/self/cwd/sensorfusion/test/unit/test_sanity_checkers.cc:256:1
    #8 0x19165c0 in testing::internal::TestFactoryImpl<(anonymous namespace)::ValidatorTest_FailsWithSanityChecker_Test>::CreateTest() /proc/self/cwd/external/gtest/googletest/include/gtest/internal/gtest-internal.h:472:44
    #9 0x1de353c in testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) /proc/self/cwd/external/gtest/googletest/src/gtest.cc:2607:10
    #10 0x1de353c in testing::Test* testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) /proc/self/cwd/external/gtest/googletest/src/gtest.cc:2643:14
    #11 0x1dd068d in testing::TestInfo::Run() /proc/self/cwd/external/gtest/googletest/src/gtest.cc:2851:22
    #12 0x1dd1106 in testing::TestSuite::Run() /proc/self/cwd/external/gtest/googletest/src/gtest.cc:3015:28
    #13 0x1ddd5f8 in testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/gtest/googletest/src/gtest.cc:5855:44
    #14 0x1de458c in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/gtest/googletest/src/gtest.cc:2607:10
    #15 0x1de458c in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/gtest/googletest/src/gtest.cc:2643:14
    #16 0x1ddcfd2 in testing::UnitTest::Run() /proc/self/cwd/external/gtest/googletest/src/gtest.cc:5438:10
    #17 0x1cf80f8 in RUN_ALL_TESTS() /proc/self/cwd/external/gtest/googletest/include/gtest/gtest.h:2490:46
    #18 0x1cf80f8 in (anonymous namespace)::test_runner(void*) /proc/self/cwd/starling_test_support/src/main.cc:43:18
    #19 0x1cf80f8 in (anonymous namespace)::run_tests() /proc/self/cwd/starling_test_support/src/main.cc:80:5
    #20 0x1cf8065 in main /proc/self/cwd/starling_test_support/src/main.cc:95:16
    #21 0x7fffff488d09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09) (BuildId: 2b86a1968781038c0766b17c1ea11a2a71d7d907)
    #22 0x1541e89 in _start (/home/jenkins/.cache/bazel/_bazel_jenkins/4535647fbb1b448ea04035fc06e4558a/execroot/starling/bazel-out/k8-dbg-ubsan/bin/sensorfusion/sensorfusion_unit_test+0x1541e89) (BuildId: 84737abdebf3f7092985f88abda709e0)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants