Skip to content

Commit

Permalink
Merge bitcoin#23724: build: add systemtap's sys/sdt.h as depends for …
Browse files Browse the repository at this point in the history
…GUIX builds with USDT tracepoints

6200fbf build: rename --enable-ebpf to --enable-usdt (0xb10c)
e158a2a build: add systemtap's sys/sdt.h as depends (0xb10c)

Pull request description:

  There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don't have to be switched out. This is possible because we don't do [expensive computations](https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#no-expensive-computations-for-tracepoints) only needed for the tracepoints. The tracepoints are NOPs when not used.

  Systemtap's `sys/sdt.h` header is required to build Bitcoin Core with USDT support. The header file defines the `DTRACE_PROBE` macros used in [`src/util/trace.h`](https://github.com/bitcoin/bitcoin/blob/master/src/util/trace.h). This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints.

  Closes bitcoin#23297.

ACKs for top commit:
  fanquake:
    ACK 6200fbf - tested enabling / disabling and with/without SDT from depends. We can follow up with bitcoin#23819, bitcoin#23907 and bitcoin#23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free.

Tree-SHA512: 0263f44892bf8450e8a593e4de7a498243687f8d81269e1c3283fa8354922c7cf93fddef4b92cf5192d33798424aa5812e03e68ef8de31af078a32dd34021382
  • Loading branch information
fanquake committed Jan 10, 2022
2 parents 2e01b69 + 6200fbf commit 542e405
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 13 deletions.
21 changes: 10 additions & 11 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ AC_ARG_WITH([bdb],
[use_bdb=$withval],
[use_bdb=auto])

AC_ARG_ENABLE([ebpf],
[AS_HELP_STRING([--enable-ebpf],
[enable eBPF tracing (default is yes if sys/sdt.h is found)])],
[use_ebpf=$enableval],
[use_ebpf=yes])
AC_ARG_ENABLE([usdt],
[AS_HELP_STRING([--enable-usdt],
[enable tracepoints for Userspace, Statically Defined Tracing (default is yes if sys/sdt.h is found)])],
[use_usdt=$enableval],
[use_usdt=yes])

AC_ARG_WITH([miniupnpc],
[AS_HELP_STRING([--with-miniupnpc],
Expand Down Expand Up @@ -1337,15 +1337,15 @@ if test "$enable_wallet" != "no"; then
fi
fi

if test "$use_ebpf" != "no"; then
AC_MSG_CHECKING([whether eBPF tracepoints are supported])
if test "$use_usdt" != "no"; then
AC_MSG_CHECKING([whether Userspace, Statically Defined Tracing tracepoints are supported])
AC_COMPILE_IFELSE([
AC_LANG_PROGRAM(
[#include <sys/sdt.h>],
[DTRACE_PROBE("context", "event");]
)],
[AC_MSG_RESULT([yes]); have_sdt=yes; AC_DEFINE([ENABLE_TRACING], [1], [Define to 1 to enable eBPF user static defined tracepoints])],
[AC_MSG_RESULT([no]); have_sdt=no;]
[AC_MSG_RESULT([yes]); AC_DEFINE([ENABLE_TRACING], [1], [Define to 1 to enable tracepoints for Userspace, Statically Defined Tracing])],
[AC_MSG_RESULT([no]); use_usdt=no;]
)
fi

Expand Down Expand Up @@ -1759,7 +1759,6 @@ AM_CONDITIONAL([TARGET_WINDOWS], [test "$TARGET_OS" = "windows"])
AM_CONDITIONAL([ENABLE_WALLET], [test "$enable_wallet" = "yes"])
AM_CONDITIONAL([USE_SQLITE], [test "$use_sqlite" = "yes"])
AM_CONDITIONAL([USE_BDB], [test "$use_bdb" = "yes"])
AM_CONDITIONAL([ENABLE_TRACING], [test "$have_sdt" = "yes"])
AM_CONDITIONAL([ENABLE_TESTS], [test "$BUILD_TEST" = "yes"])
AM_CONDITIONAL([ENABLE_FUZZ], [test "$enable_fuzz" = "yes"])
AM_CONDITIONAL([ENABLE_FUZZ_BINARY], [test "$enable_fuzz_binary" = "yes"])
Expand Down Expand Up @@ -1929,7 +1928,7 @@ echo " with bench = $use_bench"
echo " with upnp = $use_upnp"
echo " with natpmp = $use_natpmp"
echo " use asm = $use_asm"
echo " ebpf tracing = $have_sdt"
echo " USDT tracing = $use_usdt"
echo " sanitizers = $use_sanitizers"
echo " debug enabled = $enable_debug"
echo " gprof enabled = $enable_gprof"
Expand Down
5 changes: 4 additions & 1 deletion depends/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ NO_SQLITE ?=
NO_WALLET ?=
NO_ZMQ ?=
NO_UPNP ?=
NO_USDT ?=
NO_NATPMP ?=
MULTIPROCESS ?=
FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
Expand Down Expand Up @@ -149,8 +150,9 @@ natpmp_packages_$(NO_NATPMP) = $(natpmp_packages)

zmq_packages_$(NO_ZMQ) = $(zmq_packages)
multiprocess_packages_$(MULTIPROCESS) = $(multiprocess_packages)
usdt_packages_$(NO_USDT) = $(usdt_$(host_os)_packages)

packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(natpmp_packages_)
packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(natpmp_packages_) $(usdt_packages_)
native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages)

ifneq ($(zmq_packages_),)
Expand Down Expand Up @@ -228,6 +230,7 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_
-e 's|@no_bdb@|$(NO_BDB)|' \
-e 's|@no_sqlite@|$(NO_SQLITE)|' \
-e 's|@no_upnp@|$(NO_UPNP)|' \
-e 's|@no_usdt@|$(NO_USDT)|' \
-e 's|@no_natpmp@|$(NO_NATPMP)|' \
-e 's|@multiprocess@|$(MULTIPROCESS)|' \
-e 's|@debug@|$(DEBUG)|' \
Expand Down
4 changes: 4 additions & 0 deletions depends/config.site.in
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ if test -z "$enable_zmq" && test -n "@no_zmq@"; then
enable_zmq=no
fi

if test -z "$enable_usdt" && test -n "@no_usdt@"; then
enable_usdt=no
fi

if test "@host_os@" = darwin; then
BREW=no
fi
Expand Down
2 changes: 2 additions & 0 deletions depends/packages/packages.mk
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ natpmp_packages=libnatpmp
multiprocess_packages = libmultiprocess capnp
multiprocess_native_packages = native_libmultiprocess native_capnp

usdt_linux_packages=systemtap

darwin_native_packages = native_ds_store native_mac_alias

$(host_arch)_$(host_os)_native_packages += native_b2
Expand Down
12 changes: 12 additions & 0 deletions depends/packages/systemtap.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package=systemtap
$(package)_version=4.5
$(package)_download_path=https://sourceware.org/systemtap/ftp/releases/
$(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=75078ed37e0dd2a769c9d1f9394170b2d9f4d7daa425f43ca80c13bad6cfc925
$(package)_patches=remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch

define $(package)_preprocess_cmds
patch -p1 < $($(package)_patch_dir)/remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch && \
mkdir -p $($(package)_staging_prefix_dir)/include/sys && \
cp includes/sys/sdt.h $($(package)_staging_prefix_dir)/include/sys/sdt.h
endef
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
commit b92d4c121486f3c6e8a2cea537c53eb09894479a
Author: 0xb10c <[email protected]>
Date: Tue Dec 7 11:02:07 2021 +0100

Remove _SDT_ASM_SECTION_AUTOGROUP_SUPPORT check

We assume that the assembler supports "?" in .pushsection directives.
This enables us to skip configure and make.

See https://github.com/bitcoin/bitcoin/issues/23297.

diff --git a/includes/sys/sdt.h b/includes/sys/sdt.h
index 97766e710..352b4ee25 100644
--- a/includes/sys/sdt.h
+++ b/includes/sys/sdt.h
@@ -230,12 +230,10 @@ __extension__ extern unsigned long long __sdt_unsp;
nice with code in COMDAT sections, which comes up in C++ code.
Without that assembler support, some combinations of probe placements
in certain kinds of C++ code may produce link-time errors. */
-#include "sdt-config.h"
-#if _SDT_ASM_SECTION_AUTOGROUP_SUPPORT
+/* PATCH: We assume that the assembler supports the feature. This
+ enables us to skip configure and make. In turn, this means we
+ require fewer dependencies and have shorter depend build times. */
# define _SDT_ASM_AUTOGROUP "?"
-#else
-# define _SDT_ASM_AUTOGROUP ""
-#endif

#define _SDT_ASM_BODY(provider, name, pack_args, args) \
_SDT_ASM_1(990: _SDT_NOP) \
2 changes: 1 addition & 1 deletion doc/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct
| Qt | [5.12.11](https://download.qt.io/official_releases/qt/) | [5.9.5](https://github.com/bitcoin/bitcoin/issues/20104) | No | | |
| SQLite | [3.32.1](https://sqlite.org/download.html) | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | | | |
| XCB | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) (Linux only) |
| systemtap ([tracing](tracing.md))| | | | | |
| systemtap ([tracing](tracing.md))| [4.5](https://sourceware.org/systemtap/ftp/releases/) | | | | |
| xkbcommon | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) (Linux only) |
| ZeroMQ | [4.3.1](https://github.com/zeromq/libzmq/releases) | 4.0.0 | No | | |
| zlib | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) |
Expand Down

0 comments on commit 542e405

Please sign in to comment.