#125 compiles LLVM from source with Bazel. This is achieved through a fork of the llvm-bazel repo that supports the 12.x release branch. This is a temporary solution until Bazel support is natively available in LLVM.

https://reviews.llvm.org/D90352

I might miss some context for the project, but do we want to add "what" and "how to use" in this project readme? Usually that's the first place I will look for github projects.

flush_queue_mock.h has been temporarily removed from Clang Tidy. I believe that it needs an accompanying binary in order for the Google Mock dependency to work.

Introduced in #38.

trace_writer_mock.h has been temporarily removed from Clang Tidy. I believe that it needs an accompanying binary in order for the Google Mock dependency to work.

Introduced in #35.

Investigate using a Bazel Aspect for Clang Format to leverage Bazel's caching infrastructure when formatting files.

Discussion: #2 (comment)

There are open compliance tasks that need to be reviewed for your spoor repo.

Action required: 4 compliance tasks

To bring this repository to the standard required for 2021, we require administrators of this and all Microsoft GitHub repositories to complete a small set of tasks within the next 60 days. This is critical work to ensure the compliance and security of your microsoft GitHub organization.

Please take a few minutes to complete the tasks at: https://repos.opensource.microsoft.com/orgs/microsoft/repos/spoor/compliance

• The GitHub AE (GitHub inside Microsoft) migration survey has not been completed for this private repository
• No Service Tree mapping has been set for this repo. If this team does not use Service Tree, they can also opt-out of providing Service Tree data in the Compliance tab.
• No repository maintainers are set. The Open Source Maintainers are the decision-makers and actionable owners of the repository, irrespective of administrator permission grants on GitHub.
• Classification of the repository as production/non-production is missing in the Compliance tab.

You can close this work item once you have completed the compliance tasks, or it will automatically close within a day of taking action.

If you no longer need this repository, it might be quickest to delete the repo, too.

GitHub inside Microsoft program information

More information about GitHub inside Microsoft and the new GitHub AE product can be found at https://aka.ms/gim or by contacting [email protected]

FYI: current admins at Microsoft include @zhuorantan, @Jon-Schneider, @imWildCat, @zetasq, @li-bei, @hruizhang, @thomasameisel, @corbinreynolds, @aaroncrespo, @Petr-2019, @dalemyers, @lelandjansen, @icodesign

Reintroduce watchOS and macOS runtime support when rules_apple supports those platforms in its apple_xcframework rule.

Introduced in #255.

event_logger_notifier_mock.h has been temporarily removed from Clang Tidy. I believe that it needs an accompanying binary in order for the Google Mock dependency to work.

Introduced in #43.

It's possible for Spoor's instrumentation pass to fail, for example, if the output function map file cannot be created. Right now this is handled by emitting an error message and calling std::exit which is not thread-safe. Verify that this is a concern for Spoor's instrumentation pass, and if so, develop of a better error handling strategy.

Clang Tidy spuriously reports "error: invalid case style for template parameter 'expr-type'". We have temporarily disabled the error via a clang-tidy config and should remove the exception when the bug is fixed.

- key: readability-identifier-naming.TypeTemplateParameterIgnoredRegexp
  value: expr-type

Run Clang Tidy against multiple files in parallel to speed up the linter.

Use the default xctestrunner when the following PR has been checked in - google/xctestrunner#29

#59 disabled Linux support in CI to unblock our MVP. Getting Linux back up and running should be a priority after the MVP. This will also allow us to reenable UBSAN support.

Abseil 20210324.1 (introduced in #120) fails to link in #125 and was downgraded to the previous release. This linker issue has come up a few times in Abseil's history.

We should eventually upgrade Abseil to a newer release that fixes the problem when one is available.

ld: illegal thread local variable reference to regular symbol __ZN4absl12lts_2021032413base_internal19thread_identity_ptrE for architecture x86_64

One solution that works today is to remove the dynamic linker feature, however, this is not ideal since it complicates the build and we don't have a need for any of the new features introduced.

--features=-supports_dynamic_linker

The README's CI status badge displays "set up now" instead of reporting the status after moving to the new (private) pipeline. Fix this when transitioning back to a public build.

Generate a compilation database entry for header-only libraries.

Header-only libraries such as trace_mock do not generate a compilation database entry because they have no source files and therefore will not emit a CppCompile action during the Bazel build. This means that clang-tidy does not work on these files and must be disabled.

One (suboptimal) solution could hack around the problem by adding a dummy source file to trigger the CppCompile action.

This ticket consolidates #34, #37, #42, and #49.

The bazel_llvm dependency points to a fork with a patch supporting LLVM 11. Migrate the dependency to the original repo when the change is contributed upstream.

Introduced in #55.

Spoor logging can be enabled and disabled at runtime. Therefore, before logging events we need to check if logging is enabled. This is currently implemented via a shared mutex to make the library thread-safe. However, this approach has significant performance implications (~30s or 21% of user time in the benchmark below). The informal benchmark below shows that replacing the mutex locks with an atomic bool (which is lock-free on the architectures we care about) has similar performance characteristics to not locking at all.

// spoor/runtime/runtime_manager/runtime_manager.cc

auto RuntimeManager::Enabled() const -> bool {
  std::shared_lock lock{lock_};
  return enabled_;
}

Informal benchmark

System

Model Name:                 MacBook Pro
Model Identifier:        MacBookPro15,1
Processor Name:    6-Core Intel Core i7

Processor Speed:                2.6 GHz
Number of Processors:             1
Total Number of Cores:            6
L2 Cache (per Core):            256 KB
L3 Cache:                         9 MB
Memory:                          16 GB

Source code

#include <cstdint>

auto Fibonacci(const uint64_t n) -> uint64_t {
  if (n < 2) return n;
  return Fibonacci(n - 1) + Fibonacci(n - 2);
}

// The return value overflows but it doesn't matter. We just need to return
// some value to ensure that the call isn't optimized out.
auto main() -> int { return Fibonacci(40); }

Build

#!/usr/bin/env bash

clang++ \
  fibonacci.cc \
  -O3 \
  -g \
  -emit-llvm \
  -S \
  -o fibonacci.ll

opt \
  fibonacci.ll \
  -load-pass-plugin=libspoor_instrumentation.dylib \
  -passes=inject-spoor-runtime \
  -S \
  -o fibonacci_instrumented.ll

clang++ \
  fibonacci_instrumented.ll \
  -O3 \
  -lspoor_runtime \
  -o fibonacci_instrumented

time ./fibonacci_instrumented

Spoor Config

Guessing an optimal config:
My L3 cache is 9 MB
9 MB / (16 bytes / event) = 562,500 events
Round down to 500k

# Guessing optimal config:
# My L3 cache is 9 MB
# 9 MB / (16 bytes / event) = 562,500 events
# Round down to 500k
SPOOR_RUNTIME_THEAD_EVENT_BUFFER_CAPACITY="500000"
SPOOR_RUNTIME_MAX_RESERVED_EVENT_BUFFER_SLICE_CAPACITY="500000"
SPOOR_RUNTIME_MAX_DYNAMIC_EVENT_BUFFER_SLICE_CAPACITY="0"
SPOOR_RUNTIME_RESERVED_EVENT_POOL_CAPACITY="10000000"
SPOOR_RUNTIME_DYNAMIC_EVENT_POOL_CAPACITY="0"

Perfetto CL 1799542 introduces a configurable installation path for the trace_processor. Use the official repo's trace_processor when it is checked in.

Clang Tidy mysteriously started incorrectly reporting readability-identifier-naming violations for most source files. Right now this warning is not treated as an error in CI to unblock development, however, it should be re-enabled.

I'm unable to reproduce the issue locally (on either macOS or Ubuntu).

e.g.

Tidying /home/vsts/work/1/s/util/result.h
3731 warnings generated.
error: invalid case style for template parameter 'expr-type' [readability-identifier-naming,-warnings-as-errors]
Suppressed 3734 warnings (3730 in non-user code, 4 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error

The error doesn't make sense because expr-type is not a valid template parameter name.

rules_apple's apple_xcframework rule does not support embedding bitcode (yet). #266 works around this limitation by stripping the -fembed-bitcode and -fembed-bitcode-marker linker flags. We should reintroduce the stripped flags when bitcode support is available.

Introduced in #266.

Clang Format does not apply changes if the applied change is undone. This could be a problem with Bazel's caching? Note that any other modifications to the source file will return Clang Tidy to the expected behavior.

Reproduction

1. Edit a source file with a style violation.
2. Run Clang Format (Bazel Aspect).
3. Observe that Clang Format fixed the violation.
4. Undo Clang Format's change by modifying the file back to what it was in step 1 (exactly).
5. Run Clang Format.
6. The style violation is not fixed.

#125 compiles LLVM from source and mysteriously introduces a bug where LLVM command line options are registered with libLLVM instead of in within own shared library. This causes duplicate command line options when loading the pass plugin and crashes opt.

After a few days of investigation digging into the build process and dynamic linking, we understand the problem but are at a loss with how to fix it. We're (reluctantly) introducing the regression anyway because the toolchain will use spoor_opt (a standalone executable) and this PR is blocking other work. The dynamic pass plugin is still something we should support.

Thanks @dawidk-msft for working with me on the investigation.

trace_reader_mock.h has been temporarily removed from Clang Tidy.

Introduced in #50.

Include the spoor directory in clang-tidy checks when the buffer system is complete. The directory is temporarily removed from checks because we're incrementally publishing the buffers (e.g. just header definitions) but clang-tidy won't work until we build the whole library.

watchOS unit tests fail with an empty execution log and are commented out to unblock other work.

$ cat /private/var/tmp/_bazel_lelandjansen/ff5a70af79385ca234cc756daa86068e/execroot/spoor/bazel-out/darwin_arm64-fastbuild/testlogs/spoor/runtime/wrappers/apple/watchos_runtime_test/test.log
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //spoor/runtime/wrappers/apple:watchos_runtime_test
-----------------------------------------------------------------------------

Experimental watchOS test support was added to rules_apple in bazelbuild/rules_apple#1177.

Introduced in #236.

There are several advantages in using#pragma once in header files (https://en.wikipedia.org/wiki/Pragma_once). It's also widely supported on modern compilers.

Spoor's instrumentation pass crashes when instrumenting IR that uses the C++ STL's iostream (using either libstdc++ or libc++ on my Mac).

Note: We encountered a similar crash when using Abseil's StrFormat util so being specific to iostream might be red herring.

// hello.cc

#include <iostream>

auto main() -> int {
  std::cout << "Hello, world!\n";
}

$ clang++ \
    hello.cc \
    -emit-llvm \
    -S \
    -o hello.ll

$ opt \
    hello.ll \
    -load-pass-plugin=libspoor_instrumentation.dylib \
    -passes=inject-spoor-runtime \
    -S \
    -o hello_instrumented.ll

Stack dump:
0.      Program arguments: opt fibonacci.ll -load-pass-plugin=/Users/lelandjansen/projects/spoor/bazel-bin/spoor/instrumentation/libspoor_instrumentation.dylib -passes=inject-spoor-runtime -S -o fibonacci_instrumented.ll
0  libLLVM.dylib                  0x0000000106eef485 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  libLLVM.dylib                  0x0000000106eef92c SignalHandler(int) + 200
2  libsystem_platform.dylib       0x00007fff72e455fd _sigtramp + 29
3  libsystem_platform.dylib       0x0000000000000001 _sigtramp + 18446603338588596769
4  libspoor_instrumentation.dylib 0x000000010290109e spoor::instrumentation::inject_runtime::InjectRuntime::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 206
5  libspoor_instrumentation.dylib 0x00000001028ec225 llvm::detail::PassModel<llvm::Module, spoor::instrumentation::inject_runtime::InjectRuntime, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 69
6  libLLVM.dylib                  0x0000000106fda11a llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) + 692
7  opt                            0x000000010284a26f llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::StringRef>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool) + 6460
8  opt                            0x0000000102856af7 main + 4644
9  libdyld.dylib                  0x00007fff72c4ccc9 start + 1
10 libdyld.dylib                  0x0000000000000007 start + 18446603338590663487
[1]    48864 segmentation fault  opt fibonacci.ll  -passes=inject-spoor-runtime -S -o fibonacci_instrumented.ll

$ opt --version
LLVM (http://llvm.org/):
  LLVM version 11.0.0
  Optimized build.
  Default target: x86_64-apple-darwin19.6.0
  Host CPU: skylake

The (TOML) runtime configuration file only supports signed 64-bit integers, however, many configuration types are unsigned 64-bit integers. Introduced in #218.

Generate a compilation database entry for header-only libraries.

Header-only libraries such as trace_mock do not generate a compilation database entry because they have no source files and therefore will not emit a CppCompile action during the Bazel build. This means that clang-tidy does not work on these files and must be disabled.

One (suboptimal) solution could hack around the problem by adding a dummy source file to trigger the CppCompile action.

This ticket consolidates #34, #37, #42, and #49.