-
Notifications
You must be signed in to change notification settings - Fork 150
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
Add some methods useful for locating debug symbols #39
Conversation
…files. This is intended to return information that can be used to locate debug info stored in a separate file. For Mach-O files, the UUID can be used in a Spotlight query to locate the matching dSYM. This will be used in the forthcoming `moria` crate.
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.
LGTM, but a couple things:
-
@philipc should take a look too
-
Thoughts on the associated type approach below?
|
||
#[inline] | ||
fn has_debug_symbols(&self) -> bool { | ||
// TODO: look at what the mingw toolchain does with DWARF-in-PE, and also |
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.
File a follow-up investigation issue and link it here?
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.
Filed #40
#[derive(Debug, Clone, Copy)] | ||
pub enum DebugFileInfo { | ||
/// The UUID from a Mach-O `LC_UUID` load command. | ||
MachOUuid(Uuid), |
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.
I wonder if it makes sense to have this be an associated type of the trait, something like:
impl Object for MachOFile {
type DebugFileInfo = Uuid;
fn get_debug_file_info(&self) -> Option<Uuid> { ... }
}
And then when we add support for GnuDebugLink
for elf files it could have its own associated type as well.
For object files that don't have separate debug info, or we haven't implemented it yet, then we can use ()
as the associated type.
Thoughts on this approach?
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.
I'm not sure that returning an associated type is enough. The goal of the Object
trait is to allow you to write cross-platform code that optionally allows you to only include support for the native platform. For debug_file_info
, using an associated type for the return value is more useful if the return value can also be handled in cross-platform code, and the way we've handled that for other associated types is for each of them to satisfy a trait too. So the question is, what trait should this return value satisfy? And I don't have any good ideas for that.
Maybe the API should instead be fn debug_file_path(&self) -> Option<PathBuf>
, and the implementations of this would use the moria
crate (so moria
would need to become independent of object
).
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.
It gets a little twisty, certainly. I wasn't sure which way the dependency ought to go, just that I wanted a straightforward API for users. Having the APIs in object
continue to be platform-agnostic from the outside seems sensible. One thing to consider is that moria
is going to wind up dragging in a bunch of other dependencies because of what it wants to accomplish. In the future I might want to add APIs to locate debug symbols by fetching them from symbol servers over HTTP, which is going to pull in a whole world of things. Do we want all of that as dependencies on object
, or would it be better to keep object
simpler and have moria
as an optional crate people can pull in to get that functionality?
The other way we can go with this is to provide a method on File
that returns what's currently the FileInternal
enum, to allow callers to delve into format-specific implementation details. Then we could just have methods like Mach::get_uuid
and Elf::get_build_id
which would be very straightforward.
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.
I don't think that exposing FileInternal
would be any better for cross-platform code than the DebugFileInfo
enum, and enum would be easier to use.
We do want to keep object dependencies simple. Even if we added debug_file_path
, we would still need to expose the UUID for the more complex uses. So let's leave it how it is.
@@ -148,6 +149,12 @@ where | |||
fn is_little_endian(&self) -> bool { | |||
self.elf.little_endian | |||
} | |||
|
|||
fn has_debug_symbols(&self) -> bool { | |||
self.section_data_by_name(".debug_info").is_some() |
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.
I'm not confident that this is correct. What if the file was generated with .debug_line
only? What's the intended use for this? Should calllers only look for .gnu_debuglink
if this returns false? I think it might be better to always use .gnu_debuglink
if it exists.
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.
Actually, .debug_info
exists even with -C debuginfo=1
. Still might be better to always check for .gnu_debuglink
though.
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.
I'm not 100% sure, honestly, except that .debug_info
is a pretty strong signal. Calling strip
on a binary will definitely remove .debug_info
by default. This is mostly intended as a check for "can I load debug info from this binary, or do I need to look for an external symbol file?"
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.
This is mostly intended as a check for "can I load debug info from this binary, or do I need to look for an external symbol file?"
There's also gdb "mini debug info" to consider: https://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html#MiniDebugInfo
With this an executable might contain some minimal info (line table) but more data might be available elsewhere. gdb looks for direct debug info first; then tries separate debuginfo; then falls back to minidebug.
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.
Following whatever gdb does here is probably the best bet for compatibility's sake.
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.
This looks fine then.
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.
Thanks!
Add some methods useful for locating debug symbols
I have another PR forthcoming to start the
moria
crate, so I've added some useful methods toObject
in support of that.Object::has_debug_symbols
is pretty straightforward, andObject::debug_file_info
exposes the platform-specific info necessary to locate debug info stored in a separate file.