Skip to content

Commit

Permalink
Do not GC DW_AT_declaration DW_TAG_subprograms (bytecodealliance#…
Browse files Browse the repository at this point in the history
…9578)

* Add a test

* Log section-relative offsets of DIEs

For easier correlation to llvm-dwarfdump's output.

* Do not GC function declarations

Declarations must be consistent across compilation units.
  • Loading branch information
SingleAccretion authored Nov 6, 2024
1 parent 185f7a8 commit a126152
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 14 deletions.
10 changes: 6 additions & 4 deletions crates/cranelift/src/debug/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ fn build_unit_dependencies(
Ok(())
}

fn has_die_back_edge(die: &read::DebuggingInformationEntry<Reader<'_>>) -> bool {
match die.tag() {
fn has_die_back_edge(die: &read::DebuggingInformationEntry<Reader<'_>>) -> read::Result<bool> {
let result = match die.tag() {
constants::DW_TAG_variable
| constants::DW_TAG_constant
| constants::DW_TAG_inlined_subroutine
Expand All @@ -124,8 +124,10 @@ fn has_die_back_edge(die: &read::DebuggingInformationEntry<Reader<'_>>) -> bool
| constants::DW_TAG_variant_part
| constants::DW_TAG_variant
| constants::DW_TAG_formal_parameter => true,
constants::DW_TAG_subprogram => die.attr(constants::DW_AT_declaration)?.is_some(),
_ => false,
}
};
Ok(result)
}

fn has_valid_code_range(
Expand Down Expand Up @@ -223,7 +225,7 @@ fn build_die_dependencies(
let child_entry = child.entry();
let child_offset = child_entry.offset().to_unit_section_offset(unit);
deps.add_edge(child_offset, offset);
if has_die_back_edge(child_entry) {
if has_die_back_edge(child_entry)? {
deps.add_edge(offset, child_offset);
}
if has_valid_code_range(child_entry, dwarf, unit, at)? {
Expand Down
21 changes: 16 additions & 5 deletions crates/cranelift/src/debug/transform/debug_transform_logging.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::debug::Reader;
use core::fmt;
use gimli::{write, AttributeValue, DebuggingInformationEntry, Dwarf, LittleEndian, Unit};
use gimli::{
write, AttributeValue, DebuggingInformationEntry, Dwarf, LittleEndian, Unit, UnitSectionOffset,
};

macro_rules! dbi_log {
($($tt:tt)*) => {
Expand All @@ -18,7 +20,7 @@ pub struct CompileUnitSummary<'a> {
impl<'a> fmt::Debug for CompileUnitSummary<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let unit = self.unit;
let offs: usize = unit.header.offset().as_debug_info_offset().unwrap().0;
let offs = get_offset_value(unit.header.offset());
write!(f, "0x{offs:08x} [")?;
let comp_dir = match unit.comp_dir {
Some(dir) => &dir.to_string_lossy(),
Expand Down Expand Up @@ -51,7 +53,7 @@ pub fn log_begin_input_die(
) {
dbi_log!(
"=== Begin DIE at 0x{:08x} (depth = {}):\n{:?}",
die.offset().0,
get_offset_value(die.offset().to_unit_section_offset(unit)),
depth,
DieDetailedSummary { dwarf, unit, die }
);
Expand Down Expand Up @@ -210,14 +212,15 @@ impl<'a> fmt::Debug for OutDieDetailedSummary<'a> {

pub fn log_end_output_die(
input_die: &DebuggingInformationEntry<Reader<'_>>,
input_unit: &Unit<Reader<'_>, usize>,
die_id: write::UnitEntryId,
unit: &write::Unit,
strings: &write::StringTable,
depth: isize,
) {
dbi_log!(
"=== End DIE at 0x{:08x} (depth = {}):\n{:?}",
input_die.offset().0,
get_offset_value(input_die.offset().to_unit_section_offset(input_unit)),
depth,
OutDieDetailedSummary {
die_id,
Expand All @@ -229,13 +232,21 @@ pub fn log_end_output_die(

pub fn log_end_output_die_skipped(
input_die: &DebuggingInformationEntry<Reader<'_>>,
input_unit: &Unit<Reader<'_>, usize>,
reason: &str,
depth: isize,
) {
dbi_log!(
"=== End DIE at 0x{:08x} (depth = {}):\n Skipped as {}\n",
input_die.offset().0,
get_offset_value(input_die.offset().to_unit_section_offset(input_unit)),
depth,
reason
);
}

fn get_offset_value(offset: UnitSectionOffset) -> usize {
match offset {
UnitSectionOffset::DebugInfoOffset(offs) => offs.0,
UnitSectionOffset::DebugTypesOffset(offs) => offs.0,
}
}
17 changes: 12 additions & 5 deletions crates/cranelift/src/debug/transform/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ pub(crate) fn clone_unit(
let (wp_die_id, vmctx_die_id) =
add_internal_types(out_unit, out_root_id, out_strings, memory_offset);

log_end_output_die(entry, out_root_id, out_unit, out_strings, 0);
log_end_output_die(entry, unit, out_root_id, out_unit, out_strings, 0);
stack.push(out_root_id);
(
out_unit,
Expand Down Expand Up @@ -376,7 +376,7 @@ pub(crate) fn clone_unit(
// if D is below B continue to skip
if new_depth > 0 {
skip_at_depth = Some((new_depth, cached));
log_end_output_die_skipped(entry, "unreachable", current_depth);
log_end_output_die_skipped(entry, unit, "unreachable", current_depth);
continue;
}
// otherwise process D with `depth_delta` being the difference from A to D
Expand All @@ -395,7 +395,7 @@ pub(crate) fn clone_unit(
// Here B = C so `depth` is 0. A is the previous node so `cached` =
// `depth_delta`.
skip_at_depth = Some((0, depth_delta));
log_end_output_die_skipped(entry, "unreachable", current_depth);
log_end_output_die_skipped(entry, unit, "unreachable", current_depth);
continue;
}

Expand Down Expand Up @@ -467,7 +467,7 @@ pub(crate) fn clone_unit(
stack.push(die_id);
assert_eq!(stack.len(), new_stack_len);
die_ref_map.insert(entry.offset(), die_id);
log_end_output_die(entry, die_id, out_unit, out_strings, current_depth);
log_end_output_die(entry, unit, die_id, out_unit, out_strings, current_depth);
continue;
}

Expand Down Expand Up @@ -524,7 +524,14 @@ pub(crate) fn clone_unit(
)?;
}

log_end_output_die(entry, out_die_id, out_unit, out_strings, current_depth);
log_end_output_die(
entry,
unit,
out_die_id,
out_unit,
out_strings,
current_depth,
);
}
die_ref_map.patch(pending_die_refs, out_unit);
Ok(Some((out_unit_id, die_ref_map, pending_di_refs)))
Expand Down
34 changes: 34 additions & 0 deletions tests/all/debug/lldb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,40 @@ check: exited with status
Ok(())
}

#[test]
#[ignore]
pub fn test_debug_dwarf_generic_lldb() -> Result<()> {
let output = lldb_with_script(
&[
"-Ccache=n",
"-Ddebug-info",
"tests/all/debug/testsuite/generic.wasm",
],
r#"b MainDefinedFunction
r
p __vmctx->set()
n
p (x + x)
b SatelliteFunction
c
n
p (x + x)
c"#,
)?;

check_lldb_output(
&output,
r#"
check: stop reason = breakpoint 1.1
check: 2
check: stop reason = breakpoint 2.1
check: 4
check: exited with status = 0
"#,
)?;
Ok(())
}

#[test]
#[ignore]
pub fn test_debug_dwarf_ref() -> Result<()> {
Expand Down
6 changes: 6 additions & 0 deletions tests/all/debug/testsuite/generic-satellite.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "generic.h"

int SomeClass::SatelliteFunction(int x) {
x *= 2;
return x;
}
21 changes: 21 additions & 0 deletions tests/all/debug/testsuite/generic.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// clang generic.cpp generic-satellite.cpp -o generic.wasm -g -target
// wasm32-unknown-wasip1
//
#include "generic.h"

int SomeClass::MainDefinedFunction() {
int x = HIDE_FROM_CHECKER(1);
int y = SatelliteFunction(x);
return x + y;
}

int TestClassDefinitionSpreadAcrossCompileUnits() {
int result = SomeClass::MainDefinedFunction();
return result != 3 ? 1 : 0;
}

int main() {
int exitCode = 0;
exitCode += TestClassDefinitionSpreadAcrossCompileUnits();
return exitCode;
}
7 changes: 7 additions & 0 deletions tests/all/debug/testsuite/generic.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#define HIDE_FROM_CHECKER(x) x

class SomeClass {
public:
static int MainDefinedFunction();
static int SatelliteFunction(int x);
};
Binary file added tests/all/debug/testsuite/generic.wasm
Binary file not shown.

0 comments on commit a126152

Please sign in to comment.