Page MenuHomeFreeBSD

ddb: Add basic CTF support [2/2]
Needs ReviewPublic

Authored by bnovkov on Dec 29 2022, 10:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Feb 10, 3:28 PM
Unknown Object (File)
Jan 19 2024, 2:42 AM
Unknown Object (File)
Jan 16 2024, 7:16 AM
Unknown Object (File)
Jan 3 2024, 4:46 AM
Unknown Object (File)
Dec 20 2023, 7:28 AM
Unknown Object (File)
Dec 10 2023, 12:17 AM
Unknown Object (File)
Nov 24 2023, 6:48 PM
Unknown Object (File)
Nov 24 2023, 5:54 PM
Subscribers

Details

Reviewers
None
Group Reviewers
Contributor Reviews (src)
Summary

This patch adds basic CTF support and a CTF-powered pretty-printer to ddb.
It uses the features added in the first patch to load and use CTF data from ddb.

The db_ctf.* files expose a basic interface for fetching type data for ELF symbols, interacting with the CTF string table, and translating type identifiers to type data.
The db_pprint.c file uses the aforementioned interfaces to implement a pretty-printer for all kernel ELF symbols.
Additionally, the pretty printing command takes an optional 'depth' parameter, making it easy to print out specific parts of complex data types.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Set default depth before printing.

The ddb(4) manual page should be updated as well. I and others can help with formatting if needed, i.e., you can just document it here in the review somewhere.

sys/ddb/db_command.c
169

The MEMSAFE flag shouldn't be set on this command, for the same reason it isn't set on the "x" command: it allows arbitrary kernel memory to be dumped to the console.

sys/ddb/db_ctf.c
6

Isn't this new code? The copyright attribution seems wrong.

sys/ddb/db_ctf.h
2

Missing copyright.

sys/ddb/db_pprint.c
6

Same here.

The ddb(4) manual page should be updated as well. I and others can help with formatting if needed, i.e., you can just document it here in the review somewhere.

Thank you! Do you think that this should be in a separate patch or should I squeeze it in here?

sys/ddb/db_command.c
169

Thank you for pointing this out, I was only aware of the write part of the flag.

sys/ddb/db_ctf.c
6

I completely forgot to check the copyright stuff before submitting the patch, should be fixed now.

I've changed the way the db_ctf subsystem registers kernel CTF data - it will now use preload_search_by_type during ddb initialization check if the loader loaded the file containing raw kernel CTF data. I've also added an extra code to copy kernel CTF data info from ddb into linker_kernel_file upon KLD initialization.

The db_ctf subsystem is now also able to register CTF data from kernel modules. Actually loading and using kernel module CTF data is planned for another patch, but I lay down the foundations for this here.

I spent some time testing this. For the purpose of loading CTF, I think it's best to simply punt on the problem for now and load CTF info after mountroot, like subr_firmware does. I wrote a patch here to demonstrate what I mean: https://github.com/markjdb/freebsd/commit/3985ddda16c0d0a7eb076a55ed33349090ad472c

In particular, I think the kernel linker should be the source of truth whenever possible. kern_linker.c already has a number of interfaces for use by kernel debuggers; I think it's best if we add a new one which can be used to look up (cached) CTF info for a particular linker file.

Regarding the functionality:

  • I think it would be quite useful to be able to pretty print an arbitrary address. Most of the time I have some pointer printed by another DDB command, and I want to examine the memory pointed to by that. OpenBSD has implemented this as "show struct" - would it be difficult to do the same?
  • pprint would be much more convenient if it resolved symbol names.
  • pprint (specifically db_ctf_find_symbol()) only looks up symbols in the kernel, it doesn't look up anything in kernel modules. I think we want a kernel linker routine akin to linker_debug_lookup() which can return the CTF container corresponding to a given address (or symbol name).
  • The formatting of "pprint" is kind of wonky. For example: https://reviews.freebsd.org/P580 . Can we indent nested struct fields?
  • There is no documentation of the new pprint command in ddb.4. I'm happy to do this part ahead of committing the patch.
sys/ddb/db_ctf.h
5

Do you want to include your name here?

sys/ddb/db_main.c
50 ↗(On Diff #123221)

Why did you move this include? It's a prerequisite for other DDB headers.

sys/ddb/db_pprint.c
29

No need for cdefs.h or types.h here.

40

ddb.h should come before other ddb includes.

Address @markj 's comments.

List of changes:

  • Applied @markj 's mountroot loading patch
  • Output is now properly indented
  • pprint now takes either a symbol name or a struct name + address
  • All pprint commands now scan both the kernel and all loaded kernel modules for CTF info or symbol info
  • Added relevant docs in ddb.4
bnovkov added inline comments.
sys/ddb/db_pprint.c
40

That was caused by clang-format, fixed.