Skip to content

trace: make sys/sdt.h include optional#9886

Open
mhgerlach30 wants to merge 1 commit into
Xilinx:masterfrom
mhgerlach30:trace-optional-sdt-include
Open

trace: make sys/sdt.h include optional#9886
mhgerlach30 wants to merge 1 commit into
Xilinx:masterfrom
mhgerlach30:trace-optional-sdt-include

Conversation

@mhgerlach30

Copy link
Copy Markdown

sys/sdt.h (SystemTap USDT) is not present in all Linux build environments, notably cross-compilation toolchains and embedded distributions that do not ship systemtap-sdt-dev. The unconditional include causes a build failure in those environments.

Use __has_include to make the inclusion conditional. When sys/sdt.h is unavailable, define no-op stubs for the four USDT macros used by XRT (STAP_PROBEV, DTRACE_PROBE, DTRACE_PROBE1, DTRACE_PROBE2) so that the tracing call sites compile without change.

Problem solved by the commit

Compilation errors in environments without sys/sdt.h

How problem was solved, alternative solutions (if any) and why they were rejected

Define no-op stubs for the four USDT macros used by XRT (STAP_PROBEV, DTRACE_PROBE, DTRACE_PROBE1, DTRACE_PROBE2).

What has been tested and how, request additional testing if necessary

Successfully compiled on Ubuntu 24.04 with and without systemtap-sdt-dev installed.

Documentation impact (if any)

@xrt-pr-bot

xrt-pr-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

⚠️ Authorization Failed

@mhgerlach30 is not a repository collaborator.

To proceed:

  • XRT Admins: Add the build label to authorize this PR build
  • OR Add @mhgerlach30 as a repository collaborator

@stsoe stsoe requested a review from maxzhen June 29, 2026 23:20
@stsoe

stsoe commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Couldn't this silently ignore cases where some build infrastructure failed to install systemtap-sdt-dev? That's the point I realize, but silently stubbing out doesn't seem right to me.

sys/sdt.h (SystemTap USDT) is not present in all Linux build
environments, notably cross-compilation toolchains and embedded
distributions that do not ship systemtap-sdt-dev.  The unconditional
include causes a build failure in those environments.

Add a new option, XRT_ENABLE_USDT, that defaults to ON. Generate
an error when XRT_ENABLE_USDT=ON, and the header file, sys/sdt.h,
does not exist.  When XRT_ENABLE_USDT=OFF, define no-op stubs for the
four USDT macros used by XRT (STAP_PROBEV, DTRACE_PROBE, DTRACE_PROBE1,
DTRACE_PROBE2).

Signed-off-by: Matthew Gerlach <matthew@anodize.com>
@mhgerlach30 mhgerlach30 force-pushed the trace-optional-sdt-include branch from 31da04f to 31cf4ef Compare June 30, 2026 19:59
@xrt-pr-bot

xrt-pr-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

⚠️ Authorization Failed

@mhgerlach30 is not a repository collaborator.

To proceed:

  • XRT Admins: Add the build label to authorize this PR build
  • OR Add @mhgerlach30 as a repository collaborator

@mhgerlach30

Copy link
Copy Markdown
Author

Couldn't this silently ignore cases where some build infrastructure failed to install systemtap-sdt-dev? That's the point I realize, but silently stubbing out doesn't seem right to me.

Good point. The build will get a clear error message if systemtap-sdt-dev in not installed when XRT_ENABLE_USDT=ON.

@stsoe stsoe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you. Looks good.
CI is broken currently so merge will be delayed.

@mhgerlach30 mhgerlach30 requested a review from stsoe June 30, 2026 20:08
@stsoe stsoe added the build label Jun 30, 2026
@mhgerlach30

Copy link
Copy Markdown
Author

Thank you. Looks good. CI is broken currently so merge will be delayed.

Thank you for speedy responses.

Is Xilinx/aiebu#314 something you can look at too?

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants