Do not GC DW_AT_declaration DW_TAG_subprograms (#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.
bytecodealliance · Nov 6, 2024 · a126152 · a126152
1 parent 185f7a8
commit a126152
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 14 deletions.
diff --git a/crates/cranelift/src/debug/gc.rs b/crates/cranelift/src/debug/gc.rs
@@ -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
@@ -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(
@@ -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)? {

diff --git a/crates/cranelift/src/debug/transform/debug_transform_logging.rs b/crates/cranelift/src/debug/transform/debug_transform_logging.rs
@@ -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)*) => {
@@ -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(),
@@ -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 }
     );
@@ -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,
@@ -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,
+    }
+}
diff --git a/crates/cranelift/src/debug/transform/unit.rs b/crates/cranelift/src/debug/transform/unit.rs
@@ -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,
@@ -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
@@ -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;
         }
 
@@ -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;
         }
 
@@ -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)))

diff --git a/tests/all/debug/lldb.rs b/tests/all/debug/lldb.rs
@@ -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<()> {

diff --git a/tests/all/debug/testsuite/generic-satellite.cpp b/tests/all/debug/testsuite/generic-satellite.cpp
@@ -0,0 +1,6 @@
+#include "generic.h"
+
+int SomeClass::SatelliteFunction(int x) {
+  x *= 2;
+  return x;
+}
diff --git a/tests/all/debug/testsuite/generic.cpp b/tests/all/debug/testsuite/generic.cpp
@@ -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;
+}
diff --git a/tests/all/debug/testsuite/generic.h b/tests/all/debug/testsuite/generic.h
@@ -0,0 +1,7 @@
+#define HIDE_FROM_CHECKER(x) x
+
+class SomeClass {
+public:
+  static int MainDefinedFunction();
+  static int SatelliteFunction(int x);
+};
diff --git a/tests/all/debug/testsuite/generic.wasm b/tests/all/debug/testsuite/generic.wasm