Skip to content

Conversation

fanquake
Copy link
Member

Just checking for the sys/sdt.h header isn't enough, as systems like macOS have the header, but it doesn't actually have the DTRACE_PROBE* probes, which leads to compile failures. The contents of sys/sdt.h in the macOS SDK is:

#ifndef _SYS_SDT_H
#define _SYS_SDT_H

/*
 * This is a wrapper header that wraps the mach visible sdt.h header so that
 * the header file ends up visible where software expects it to be.  We also
 * do the C/C++ symbol wrapping here, since Mach headers are technically C
 * interfaces.
 *
 * Note:  The process of adding USDT probes to code is slightly different
 * than documented in the "Solaris Dynamic Tracing Guide".
 * The DTRACE_PROBE*() macros are not supported on Mac OS X -- instead see
 * "BUILDING CODE CONTAINING USDT PROBES" in the dtrace(1) manpage
 *
 */
#include <sys/cdefs.h>
__BEGIN_DECLS
#include <mach/sdt.h>
__END_DECLS

#endif  /* _SYS_SDT_H */

The BUILDING CODE CONTAINING USDT PROBES section from the dtrace manpage is available here, and outlines the more involved process of using USDT probes on macOS.

@theStack
Copy link
Contributor

Concept ACK

1 similar comment
@practicalswift
Copy link
Contributor

Concept ACK

Just checking for the `sys/sdt.h` header isn't enough, as systems like
macOS have the header, but it doesn't actually have the dtrace probes,
which leads to compile failures.
@fanquake fanquake force-pushed the no_probes_darwin_for_now branch from ae46c96 to 8f7704d Compare June 16, 2021 02:16
@jb55
Copy link
Contributor

jb55 commented Jun 17, 2021

utACK 8f7704d

@practicalswift
Copy link
Contributor

cr ACK 8f7704d

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 8f7704d, tested on macOS Big Sur 11.4 (20F71) and on Linux Mint 20.1 (x86_64) with depends.

It would be nice to have the default value of --enable-ebpf equal to auto, and emit an error message when --enable-ebpf=yes but its conditions are not met.

@fanquake fanquake merged commit ad0c8f3 into bitcoin:master Jun 18, 2021
@fanquake fanquake deleted the no_probes_darwin_for_now branch June 18, 2021 07:17
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 18, 2021
8f7704d build: improve detection of eBPF support (fanquake)

Pull request description:

  Just checking for the `sys/sdt.h` header isn't enough, as systems like macOS have the header, but it doesn't actually have the `DTRACE_PROBE*` probes, which leads to [compile failures](bitcoin#22006 (comment)). The contents of `sys/sdt.h` in the macOS SDK is:
  ```bash
  #ifndef _SYS_SDT_H
  #define _SYS_SDT_H

  /*
   * This is a wrapper header that wraps the mach visible sdt.h header so that
   * the header file ends up visible where software expects it to be.  We also
   * do the C/C++ symbol wrapping here, since Mach headers are technically C
   * interfaces.
   *
   * Note:  The process of adding USDT probes to code is slightly different
   * than documented in the "Solaris Dynamic Tracing Guide".
   * The DTRACE_PROBE*() macros are not supported on Mac OS X -- instead see
   * "BUILDING CODE CONTAINING USDT PROBES" in the dtrace(1) manpage
   *
   */
  #include <sys/cdefs.h>
  __BEGIN_DECLS
  #include <mach/sdt.h>
  __END_DECLS

  #endif  /* _SYS_SDT_H */
  ```

  The `BUILDING CODE CONTAINING USDT PROBES` section from the dtrace manpage is available [here](https://gist.github.com/fanquake/e56c9866d53b326646d04ab43a8df9e2), and outlines the more involved process of using USDT probes on macOS.

ACKs for top commit:
  jb55:
    utACK 8f7704d
  practicalswift:
    cr ACK 8f7704d
  hebasto:
    ACK 8f7704d, tested on macOS Big Sur 11.4 (20F71) and on Linux Mint 20.1 (x86_64) with depends.

Tree-SHA512: 5f1351d0ac2e655fccb22a5454f415906404fdaa336fd89b54ef49ca50a442c44ab92d063cba3f161cb8ea0679c92ae3cd6cfbbcb19728cac21116247a017df5
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 4, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 4, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 4, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 4, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 4, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 6, 2022
After the macOS USDT support, just checking for the `sys/sdt.h` header
is enough. This partially reverts bitcoin#22238.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 6, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 6, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 7, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 9, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Jul 9, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
kouloumos added a commit to kouloumos/bitcoin that referenced this pull request Aug 17, 2022
- Partially revert bitcoin#22238 as after the macOS USDT support, just checking
for the `sys/sdt.h` header is enough.
- Generate `util/probes.h` during build time when supported.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants