Skip to content

std.dwarf: Append the compile dir to relative include dirs #13540

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

Conversation

micahswitzer
Copy link
Contributor

This should fix missing source code lines in debug stack traces.

I had originally placed the code to do this immediately after an include_dir was read out of the debug info, but I would have had to either write the code twice (once for DWARFv5, and once for v4 and below), or iterate over all of the include_directories once they were created. I chose the method in this PR because it doesn't require any additional allocations, and because the same logic will presumably apply to any future methods of getting include directories.

Thanks to @kubkon for helping me find and fix this!

// path separators than what generated the debug info. This was deemed
// acceptable as this code is really only designed to be used in
// combination with std.debug
const parts = if (file_entry.dir_index > 0 and !fs.path.isAbsolute(dir_name))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if it would be better stylistically to create two separate arrays and choose between them, or to just create one and pass a different slice of it to path.join depending on what behavior we want. I chose this because I think it's a bit more readable, but I'd be fine with the other option as well.

The DWARF spec states that relative include directories are relative to
the current compilation unit directory.
@Vexu
Copy link
Member

Vexu commented Nov 29, 2022

Applying this and reverting the relevant changes from my commit still doesn't make source lines appear when calling from a different working directory. I'd say to add an absolute path to the compile unit but there is this big comment about not doing that:

// For macOS stack traces, we want to avoid having to parse the compilation unit debug
// info. As long as each debug info file has a path independent of the compilation unit
// directory (DW_AT_comp_dir), then we never have to look at the compilation unit debug
// info. If we provide an absolute path to LLVM here for the compilation unit debug
// info, LLVM will emit DWARF info that depends on DW_AT_comp_dir. To avoid this, we
// pass "." for the compilation unit directory. This forces each debug file to have a
// directory rather than be relative to DW_AT_comp_dir. According to DWARF 5, debug
// files will no longer reference DW_AT_comp_dir, for the purpose of being able to
// support the common practice of stripping all but the line number sections from an
// executable.

@kubkon
Copy link
Member

kubkon commented Nov 29, 2022

Applying this and reverting the relevant changes from my commit still doesn't make source lines appear when calling from a different working directory. I'd say to add an absolute path to the compile unit but there is this big comment about not doing that:

// For macOS stack traces, we want to avoid having to parse the compilation unit debug
// info. As long as each debug info file has a path independent of the compilation unit
// directory (DW_AT_comp_dir), then we never have to look at the compilation unit debug
// info. If we provide an absolute path to LLVM here for the compilation unit debug
// info, LLVM will emit DWARF info that depends on DW_AT_comp_dir. To avoid this, we
// pass "." for the compilation unit directory. This forces each debug file to have a
// directory rather than be relative to DW_AT_comp_dir. According to DWARF 5, debug
// files will no longer reference DW_AT_comp_dir, for the purpose of being able to
// support the common practice of stripping all but the line number sections from an
// executable.

Absolute paths are required by PDB on Win tho (we currently rely on a postmortem hack where we instruct the linker what the full prefix was), so perhaps we should revisit this idea after all? I can re-investigate why we think it's a problem for macos stack traces.

@kubkon
Copy link
Member

kubkon commented Dec 9, 2022

As #13843 has been merged which changed how we emit DWARF (i.e., we now emit absolute paths rather than relative ones like we did in stage1), I will close this PR as it's now obsolete.

@kubkon kubkon closed this Dec 9, 2022
@micahswitzer
Copy link
Contributor Author

I realize that this code only has utility when it comes to a Zig binary inspecting its own debug information, but theoretically this change still fixes a legitimate issue in the DWARF parsing code.

@kubkon
Copy link
Member

kubkon commented Dec 12, 2022

I would argue that this code is specifically tailored to printing our stack traces since it parses all of debug info upfront and allocates the vast majority of it which is usually unnecessary. For that very reason, MachO linker uses its own custom DWARF parser rather than relying on this one (it did but this implementation is simply too slow for the reasons mentioned above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants