Skip to content
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.

Sign up for GitHub

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

Support for built-in ORC information #460

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+435 −26
Open

Support for built-in ORC information #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e72870
orc_info: don't use ELF_Data when processing orc header
brenns10 Dec 13, 2024
278f004
module: add object
brenns10 Dec 13, 2024
b49d37d
orc_info: enable the use of built-in ORC
brenns10 Dec 14, 2024
2026435
orc_info: apply debug file bias to pc_base at load
brenns10 Dec 16, 2024
44e10fd
tests: linux_kernel: test built-in ORC unwinding
brenns10 Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions _drgn.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,14 @@ class Module:
module, it is set to the file's build ID if it is not already set. It can
also be set manually.
"""
object: Object
"""
The object this module was created from.

For Linux kernel loadable modules, this is a reference to the ``struct
module`` this was created from. For other kinds, this is currently an absent
Comment on lines +1506 to +1507
Copy link
Owner

Choose a reason for hiding this comment

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

A pointer (i.e. struct module *) feels slightly more natural to me (following the precedent of drgn.Thread.object). I'd be fine restricting drgn.Program.linux_kernel_loadable_module() to only accept struct module * and not struct module to make that simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about that as well. I get that value pointers vs reference objects are always confusing to users, so I agree it would probably be better to stick with the convention that's already established.

I'd actually love to try to keep linux_kernel_loadable_module() accepting struct module. If we can't create a pointer to the object, then we can just leave the Module.object field absent as before. If you're ok with that.

The reason being that I really enjoyed the possibility of somebody constructing a struct module value Object, and then passing it in. I don't actually know how feasible that would be, but it's kinda fun :)

object.
"""
loaded_file_status: ModuleFileStatus
"""Status of the module's :ref:`loaded file <module-loaded-file>`."""
loaded_file_path: Optional[str]
Expand Down
25 changes: 25 additions & 0 deletions libdrgn/debug_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ struct drgn_error *drgn_module_find_or_create(struct drgn_program *prog,

module->prog = prog;
module->kind = key->kind;
drgn_object_init(&module->object, prog);
// Linux userspace core dumps usually filter out file-backed mappings
// (see coredump_filter in core(5)), so we need the loaded file to read
// the text. Additionally, .eh_frame is in the loaded file and not the
Expand Down Expand Up @@ -414,6 +415,7 @@ struct drgn_error *drgn_module_find_or_create(struct drgn_program *prog,
err_name:
free(module->name);
err_module:
drgn_object_deinit(&module->object);
free(module);
return err;
}
Expand Down Expand Up @@ -515,6 +517,7 @@ static void drgn_module_destroy(struct drgn_module *module)
drgn_elf_file_destroy(module->loaded_file);
free(module->build_id);
free(module->name);
drgn_object_deinit(&module->object);
free(module);
}

Expand Down Expand Up @@ -987,6 +990,12 @@ drgn_module_wanted_supplementary_debug_file(struct drgn_module *module,
: DRGN_SUPPLEMENTARY_FILE_NONE;
}

LIBDRGN_PUBLIC struct drgn_error *
drgn_module_object(struct drgn_module *module, struct drgn_object *ret)
{
return drgn_object_copy(ret, &module->object);
}

static struct drgn_error *
drgn_program_register_debug_info_finder_impl(struct drgn_program *prog,
struct drgn_debug_info_finder *finder,
Expand Down Expand Up @@ -5528,5 +5537,21 @@ drgn_module_find_cfi(struct drgn_program *prog, struct drgn_module *module,
if (err != &drgn_not_found)
return err;
}

if (!can_use_debug_file) {
if (!module->parsed_orc) {
err = drgn_module_parse_orc(module);
if (err)
return err;
module->parsed_orc = true;
}

err = drgn_module_find_orc_cfi(module, pc, row_ret,
interrupted_ret,
ret_addr_regno_ret);
if (err != &drgn_not_found)
return err;
}

return &drgn_not_found;
}
2 changes: 2 additions & 0 deletions libdrgn/debug_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ struct drgn_module {
struct drgn_module_wanted_supplementary_file *wanted_supplementary_debug_file;
/** Node in @ref drgn_debug_info::modules_pending_indexing. */
struct drgn_module *pending_indexing_next;
/** Object the module was created from */
struct drgn_object object;
};

struct drgn_error *drgn_module_find_or_create(struct drgn_program *prog,
Expand Down
11 changes: 11 additions & 0 deletions libdrgn/drgn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,17 @@ drgn_module_wanted_supplementary_debug_file(struct drgn_module *module,
const void **checksum_ret,
size_t *checksum_len_ret);

/**
* Return the object associated with this module.
*
* For Linux kernel loadable modules, this is a reference to the `struct module`
* they represent. Other kinds of modules may have other objects.
*
* @param[out] ret Initialized object where the module object is placed
*/
struct drgn_error *
drgn_module_object(struct drgn_module *module, struct drgn_object *ret);
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please also add a drgn_module_set_object() (and the corresponding Python binding setter)? The module API in general lets the user say it knows better than drgn, so we should have a way to override this (for any kind of module).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It strikes me that this opens up the possibility for a user to assign a completely irrelevant object to the module, thus making it much more likely for an error to occur when we're using it. I don't have a problem with that, of course: good error handling is important regardless.

But in my experience, sometimes errors that come from too deep within the bowels of drgn are no longer helpful, because they don't come with a stack trace like Python errors. For example here: if a user ran prog.thread(1).stack_trace(), and got an error related to the ORC unwinder, it may not be obvious where it came from.

I know we can catch errors that we expect to be likely, and replace them with more informative ones. But I wonder if it wouldn't be worthwhile to add some sort of error-chaining functionality for drgn? That way, we could implement it so that any error returned by drgn_read_builtin_orc() might get wrapped with the context "encountered error while loading built-in ORC for module foo", without eliminating the specifics of the error that is wrapped.

That's just an oddball idea which doesn't have to be part of this PR, in any case. And I could make the case against it as well: maybe we should rely on logging to give us the context for the error! Just an idea to ponder over the holidays.


/** Debugging information finder callback table. */
struct drgn_debug_info_finder_ops {
/**
Expand Down
26 changes: 21 additions & 5 deletions libdrgn/linux_kernel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ kernel_module_set_section_addresses(struct drgn_module *module,
static struct drgn_error *
kernel_module_find_or_create_internal(const struct drgn_object *module_obj,
struct drgn_module **ret, bool *new_ret,
bool create, bool log)
bool create, bool log, uint64_t address)
{
struct drgn_error *err;
struct drgn_program *prog = drgn_object_program(module_obj);
Expand Down Expand Up @@ -1336,7 +1336,17 @@ kernel_module_find_or_create_internal(const struct drgn_object *module_obj,
&module, &new);
if (err)
return err;
if (!new) {
if (new) {
// We take a struct module value object. But to expose to users,
// we need to create a reference object of the same type, so the
// users won't get stale data.
if (address)
err = drgn_object_set_reference(&module->object,
drgn_object_qualified_type(module_obj),
address, 0, 0);
if (err)
return err;
} else {
*ret = no_cleanup_ptr(module);
if (new_ret)
*new_ret = new;
Expand Down Expand Up @@ -1392,6 +1402,7 @@ drgn_module_find_or_create_linux_kernel_loadable_internal(const struct drgn_obje
bool create)
{
struct drgn_error *err;
uint64_t module_address = 0;

// kernel_module_find_or_create_internal() expects a `struct module`
// value.
Expand All @@ -1400,21 +1411,25 @@ drgn_module_find_or_create_linux_kernel_loadable_internal(const struct drgn_obje
== DRGN_TYPE_POINTER) {
drgn_object_init(&mod, drgn_object_program(module_obj));
err = drgn_object_dereference(&mod, module_obj);
if (!err)
if (!err) {
module_address = mod.address;
err = drgn_object_read(&mod, &mod);
}
module_obj = &mod;
if (err)
goto out;
} else if (module_obj->kind != DRGN_OBJECT_VALUE) {
drgn_object_init(&mod, drgn_object_program(module_obj));
err = drgn_object_read(&mod, module_obj);
if (!err)
module_address = module_obj->address;
module_obj = &mod;
if (err)
goto out;
}

err = kernel_module_find_or_create_internal(module_obj, ret, new_ret,
create, false);
create, false, module_address);
out:
if (module_obj == &mod)
drgn_object_deinit(&mod);
Expand Down Expand Up @@ -1493,6 +1508,7 @@ yield_kernel_module(struct linux_kernel_loaded_module_iterator *it,
// for /proc/kcore, it is faster to read the entire structure
// (which is <2kB as of Linux 6.5) from the core dump all at
// once than it is to read each field individually.
uint64_t address = mod.address;
err = drgn_object_read(&mod, &mod);
if (err)
goto list_walk_err;
Expand All @@ -1505,7 +1521,7 @@ yield_kernel_module(struct linux_kernel_loaded_module_iterator *it,
goto list_walk_err;

err = kernel_module_find_or_create_internal(&mod, ret, new_ret,
true, true);
true, true, address);
if (err && !drgn_error_is_fatal(err)) {
drgn_error_log_warning(prog, err, "ignoring module: ");
drgn_error_destroy(err);
Expand Down
Loading
Loading