Skip to content

Add .debug_sup support - #1674

Open
Irene-dev-888 wants to merge 1 commit into
libbpf:mainfrom
expuss2000:debug_sup
Open

Irene-dev-888 wants to merge 1 commit into
libbpf:mainfrom
expuss2000:debug_sup

Conversation

@Irene-dev-888

Copy link
Copy Markdown

Add support for .debug_sup sections, in case dwz is invoked with the --dwarf-5 option.

Signed-off-by: Irene Siboni <irene.siboni.8@gmail.com>
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.54054% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.16%. Comparing base (86f6ccc) to head (f1a3c9b).

Files with missing lines Patch % Lines
src/dwarf/resolver.rs 82.71% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1674      +/-   ##
==========================================
- Coverage   95.24%   95.16%   -0.08%     
==========================================
  Files          58       59       +1     
  Lines       11341    11485     +144     
==========================================
+ Hits        10802    10930     +128     
- Misses        539      555      +16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-e-s-o

d-e-s-o commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Sorry for the delay. I hope to get to taking a look by the end of the week.

Comment thread src/dwarf/debug_sup.rs
@@ -0,0 +1,300 @@
//! Support for reading of GNU .`debug_sup` data as prescribed in DWARF v5

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please mention what this section is actually used for.

Comment thread src/dwarf/debug_sup.rs
/// Read the debug sup section.
pub(crate) fn read_debug_sup(
parser: &ElfParser,
) -> Result<Option<(u16, bool, &Path, BuildId<'_>)>> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Introduce a proper struct type?

Comment thread src/dwarf/debug_sup.rs
Comment on lines +65 to +67
let path = data
.read_cstr()
.ok_or_invalid_data(|| "failed to read .debug_sup filename")?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

File summary seems to indicate that there is only a path if is_supplementary is 1. Yet we read it unconditionally...?

Comment thread src/dwarf/debug_sup.rs
assert!(error.to_string().contains("build ID"));
}

/// Check that we can successfully read an ELF file's debug altlink.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy&pasted comment?

Comment thread src/dwarf/resolver.rs
/// # Notes
/// This function ignores any errors encountered.
fn find_altdebug_file(
fn find_supplementary_file(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Varius error paths still reference "altlink".

Comment thread src/dwarf/debug_sup.rs
//! compilation unit
//! - `is_supplementary`: u8 which is set to 1 if the file in which the
//! section is stored is a supplementary file, 0 o.w.
//! - `sup_filename`: null-terminated supplementary file filename (is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems to be a "path", not a file name? Also see https://dwarfstd.org/issues/260116.1.html

Comment thread src/dwarf/debug_sup.rs
use test_tag::tag;


/// Check that we can correctly read a build id from debug altlink section

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No more altlink?

Comment thread src/dwarf/resolver.rs
Comment on lines +388 to +390
/// If the source file contains a valid debug sup, this parser
/// represents it.
_supee_parser: Option<Rc<ElfParser>>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So wouldn't/shouldn't this be mutually exclusive with the altlink stuff, given that it supposedly standardizes and superseds it? Then we should probably merge the fields. It's even how the logic below is written up...

Comment thread tests/suite/inspect.rs
/// Check that we correctly incorporate debug sup files in the
/// inspection process.
#[test]
fn inspect_debug_sup_honoring() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Parametrize the altlink test instead of duplicating everything please

Comment thread src/dwarf/resolver.rs

/// Check that we resolve debug sup correctly.
#[test]
fn debug_sup_resolution() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Parametrize the altlink test instead of duplicating everything

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.

2 participants