-
Notifications
You must be signed in to change notification settings - Fork 37.7k
build: Detect USDT the same way how it is used in the code #27458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Thanks for looking into this, however, I'm not sure if it's the right time for this. At the moment, we only really support the tracepoints on Linux.
The suggested commit enables USDT detection on FreeBSD, something that (somewhat unintentionally) wasn't enabled before. Not sure if it's worth enabling them at the moment for FreeBSD. It would be awesome to have them on more OS's (e.g. macOS see #25541), but AFAIK no one has looked into using them on FreeBSD. @kouloumos suggested to use dtrace to generate the tracepoint macros in #26593 (review) which would unify the process for macOS and Linux. I just saw that there's a dtrace port for FreeBSD too. It would be nice if we could get FreeBSD, macOS and Linux with one unified approach. Until then, maybe leave them disabled? |
I agree with @0xB10C here. Not sure about making this change, given the tracepoints still wont work anyways. |
Besides the FreeBSD case, shouldn't tokens be passed to the tested macro rather than string literals? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 468cf43
The actual source code does not pass strings and it is better to test for whatever the source code will use. Maybe adjust the PR title/OP and the commit message.
AFAIK no one has looked into using them on FreeBSD
I would use them, please :)
I just saw that there's a dtrace port for FreeBSD too
DTrace has been part of the FreeBSD base system since 2009. This is why I found it strange that Bitcoin Core's src/util/trace.h
uses e.g. DTRACE_PROBE3()
, but that is not supported on FreeBSD. The man page: STD(9).
What are we doing here? If we aren't doing this yet, I think it can just be closed and/or combined in #26593. Otherwise,
? |
Reworked. The PR description has been updated. |
ACK b53cab0 Tracepoints for FreeBSD can happen at a later point, if needed. |
…n the code b53cab0 build: Detect USDT the same way how it is used in the code (Hennadii Stepanov) Pull request description: In the code we do not use string literals. Also a check for `DTRACE_PROBE7` macro has been added as not all systems define`DTRACE_PROBE{6,7,8,9,10,11,12}` macros (e.g., FreeBSD). ACKs for top commit: 0xB10C: ACK b53cab0 Tree-SHA512: 74f49424d57bf1929f2b09edba1449cef5a1a2448161952da35302343f3003d5bedeab1417e166b656c5f629303e2de888550b1219e886a1b991b12b9c880794
In the code we do not use string literals.
Also a check for
DTRACE_PROBE7
macro has been added as not all systems defineDTRACE_PROBE{6,7,8,9,10,11,12}
macros (e.g., FreeBSD).