Page MenuHomeFreeBSD

libdtrace: implement inline function tracing for kinst
Needs ReviewPublic

Authored by christos on Feb 28 2023, 5:10 PM.
Tags
Referenced Files
F62327445: D38825.diff
Mon, Jun 5, 6:29 AM
Unknown Object (File)
Sat, Jun 3, 5:06 PM
Unknown Object (File)
Fri, May 26, 1:49 PM
Unknown Object (File)
Tue, May 23, 1:41 PM
Unknown Object (File)
Tue, May 23, 3:08 AM
Unknown Object (File)
Wed, May 17, 12:28 PM
Unknown Object (File)
Mon, May 15, 5:35 AM
Unknown Object (File)
Mon, May 15, 5:35 AM

Details

Reviewers
markj
gnn
Summary

The following code implements inline tracing for the kinst DTrace provider.

Briefly explained, what the code does is, for each probe of the form:

kinst::<inline_func>:<entry/return>

libdtrace checks to see if <inline_func> is indeed an inline function, where
in this case it finds all inline copies of it and creates new probes for each
one of them. Otherwise, it converts the probe to an FBT one so that we don't
duplicate FBT's functionality in kinst.

Notes:

Unlike FBT, leaving the probe name empty won't trace both entry and return.
kinst goes into "inline-tracing mode" only when it sees entry/return in the
probe name, otherwise it thinks it's a regular kinst probe, in which case the
dtrace(1) invocation will fail if we try to trace an inline function with an
empty probe name.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
cddl/contrib/opensolaris/lib/libdtrace/common/dt_sugar.c
245

Any reason not to use dt_alloc()?

305

Instead of duplicating all of these fields, why not embed the GElf_Shdr directly into the section structure?

cddl/lib/libdtrace/Makefile
124

Add these to the LIBADD= line below instead.

christos marked 3 inline comments as done.

Fix a recent lowpc/highpc bug I made accidentally, embed GElf_Shdr into
the section struct, replace emalloc() with dt_alloc(), fix libraries in Makefiles.

christos retitled this revision from libdtrace: implement inline tracing for kinst to libdtrace: implement inline function tracing for kinst.Feb 28 2023, 6:09 PM
cddl/contrib/opensolaris/lib/libdtrace/common/dt_sugar.c
146

You should avoid adding global variables to libdtrace, all state should be accessed via the dtrace_hdl_t structure.

378

What's the purpose of the else-if?

752

This should use the kern.bootfile sysctl instead of hard-coding the path. Then we have three cases: debug info is embedded in the kernel executable itself (should be rare), kernel.debug is in the same directory as the kernel, or kernel.debug is in /usr/lib/debug/$(dirname $(sysctl -n kern.bootfile)).

It might make sense to keep the bootfile path in the dtrace_hdl_t since there's some other code in libdtrace that uses it.

(Since the current code should work fine, you could postpone fixing this until later, just be sure to add a TODO comment or so.)

875

Don't we want to check the probe name as well? This code shouldn't modify regular kinst::<func>:<offset> probes.

share/mk/src.libnames.mk
387

Please keep these alphabetically sorted, also elf is already listed.

christos added inline comments.
cddl/contrib/opensolaris/lib/libdtrace/common/dt_sugar.c
146

Since this is dt_sugar-only code, do you think I could embed those globals
in dt_sugar_parse_t instead?

378

No real use other than to make it obvious that I'm handling both entry
and return probes.

875

This doesn't affect kinst::<func>:<offset> probes at all. The
flag is tested in dt_sugar_do_kinst_inline() and will exit right away
if the probe is neither entry nor return.

share/mk/src.libnames.mk
387

Didn't notice it.

christos marked 4 inline comments as done.
christos edited the summary of this revision. (Show Details)

Address Mark's comments.

diff against main, not last patch

  • use dt_free() instead of free()
  • add error checks after calls to dt_alloc() and dt_node_xalloc()

use dtp->bootfile in dt_sugar_do_kinst_inline() instead of hardcoding the path

fix dtrace_disx86() fail in dt_sugar_kinst_find_caller_func()

update dtrace_kinst.4 man page to mention inline tracing

remove call to dwarf_next_types_section()

Handle leaf functions with their push %rbp ommitted.

christos edited the summary of this revision. (Show Details)

Got rid of deep copying, appending to the main clause is simpler.

Handle multiple dn_pdescs properly, move handling away from
dt_sugar_visit_all().

christos edited the summary of this revision. (Show Details)

Do not always go back one instruction.

Improve example in man page.

Get rid of push %rbp check (depends on D39166).

christos edited the summary of this revision. (Show Details)

Parse all loaded modules and their debug files instead of just kernel.
Mention new bug in summary.

christos edited the summary of this revision. (Show Details)

Fix bugs, surround amd64-specific code in an #ifdef, merge flags into
dtsp_flags, handle dt_sugar_elf_init() and dwarf_elf_init() errors, use
longjmp() for dt_alloc() failures.

Fix per-module parsing. Create an additional FBT probe if a non-inline definition is
found along an inline definition in the same module.

More bug fixes. I always keep finding them after I submit the patch.

Do not exclude inline copies that have DW_AT_subprogram instead of
DW_AT_inlined_subroutine.

Add tests. kinst_test_inline() needs modification so that it can be splitted
into DW_AT_ranges.

Implement dt_sugar_elf_verify_debuglink() to make sure the debug files we're
reading are up to date, and if they're not, skip them. Also stop leaking
elf_info structs in dt_sugar_do_kinst_inline().

Fix accidental dangling brace.

Surround amd64-specific code in an ifdef.

Use dtrace_instr_size() and get rid of machine-dependent code (depends on
D39489).

Fix Makefile conflict, check if dtracetest is already loaded before attempting to load it.

Add libdwarf in Makefile.inc1 (Thanks Mark).

Bail out if kinst doesn't exist (i.e the architecture doesn't support it).