Rollup merge of rust-lang#73460 - tmandry:variant-lineinfo, r=oli-obk

Manishearth · web-flow · commit 673d7caae110 · 2020-06-25T17:58:13.000-07:00
Emit line info for generator variants Debuggers should be able to read a generator / async fn state machine and show the line it's suspended at. Eventually, this could grow into an "async stack trace" feature of sorts. While no debugger support this for Rust today, this PR adds the debuginfo necessary for that support to exist. [This gist](https://gist.github.com/tmandry/6d7004fa008684f76809208847459f9b) shows the resulting debuginfo for a simple example. Here's a snippet: ``` 0x00000986: DW_TAG_variant DW_AT_discr_value (0x03) 0x00000988: DW_TAG_member DW_AT_name ("3") DW_AT_type (0x000009bc "Suspend0") DW_AT_decl_file ("/home/tmandry/code/playground/generator-simple.rs") DW_AT_decl_line (6) DW_AT_alignment (8) DW_AT_data_member_location (0x00) ``` The file and line have been added here. The line currently points to the beginning of the statement containing the yield (or await), because that's what the MIR source info points to for the yield terminator. (We may want to point to the yield or await line specifically, but that can be done independently of this change.) Debuggers don't know how to use this kind of info yet. However, we're hoping to experiment with adding such support to Fuchsia's debugger. It would be exciting if someone were interested in adding similar to support to gdb/lldb. r? @oli-obk cc @eddyb @jonas-schievink Part of rust-lang#73524.
diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs
diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs
@@ -70,10 +70,10 @@ fn uncached_llvm_type<'a, 'tcx>(
                     write!(&mut name, "::{}", def.variants[index].ident).unwrap();
                 }
             }
-            if let (&ty::Generator(_, substs, _), &Variants::Single { index })
+            if let (&ty::Generator(_, _, _), &Variants::Single { index })
                  = (&layout.ty.kind, &layout.variants)
             {
-                write!(&mut name, "::{}", substs.as_generator().variant_name(index)).unwrap();
+                write!(&mut name, "::{}", ty::GeneratorSubsts::variant_name(index)).unwrap();
             }
             Some(name)
         }
diff --git a/src/librustc_index/bit_set.rs b/src/librustc_index/bit_set.rs
@@ -700,7 +700,7 @@ impl<T: Idx> GrowableBitSet<T> {
 ///
 /// All operations that involve a row and/or column index will panic if the
 /// index exceeds the relevant bound.
-#[derive(Clone, Debug, Eq, PartialEq, RustcDecodable, RustcEncodable)]
+#[derive(Clone, Eq, PartialEq, RustcDecodable, RustcEncodable)]
 pub struct BitMatrix<R: Idx, C: Idx> {
     num_rows: usize,
     num_columns: usize,
@@ -876,6 +876,22 @@ impl<R: Idx, C: Idx> BitMatrix<R, C> {
     }
 }
 
+impl<R: Idx, C: Idx> fmt::Debug for BitMatrix<R, C> {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        /// Forces its contents to print in regular mode instead of alternate mode.
+        struct OneLinePrinter<T>(T);
+        impl<T: fmt::Debug> fmt::Debug for OneLinePrinter<T> {
+            fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+                write!(fmt, "{:?}", self.0)
+            }
+        }
+
+        write!(fmt, "BitMatrix({}x{}) ", self.num_rows, self.num_columns)?;
+        let items = self.rows().flat_map(|r| self.iter(r).map(move |c| (r, c)));
+        fmt.debug_set().entries(items.map(OneLinePrinter)).finish()
+    }
+}
+
 /// A fixed-column-size, variable-row-size 2D bit matrix with a moderately
 /// sparse representation.
 ///
diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs
@@ -10,6 +10,8 @@ use rustc_index::vec::IndexVec;
 use rustc_span::{Span, Symbol};
 use rustc_target::abi::VariantIdx;
 use smallvec::SmallVec;
+use std::cell::Cell;
+use std::fmt::{self, Debug};
 
 use super::{Field, SourceInfo};
 
@@ -58,7 +60,7 @@ rustc_index::newtype_index! {
 }
 
 /// The layout of generator state.
-#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
+#[derive(Clone, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
 pub struct GeneratorLayout<'tcx> {
     /// The type of every local stored inside the generator.
     pub field_tys: IndexVec<GeneratorSavedLocal, Ty<'tcx>>,
@@ -67,12 +69,72 @@ pub struct GeneratorLayout<'tcx> {
     /// be stored in multiple variants.
     pub variant_fields: IndexVec<VariantIdx, IndexVec<Field, GeneratorSavedLocal>>,
 
+    /// The source that led to each variant being created (usually, a yield or
+    /// await).
+    pub variant_source_info: IndexVec<VariantIdx, SourceInfo>,
+
     /// Which saved locals are storage-live at the same time. Locals that do not
     /// have conflicts with each other are allowed to overlap in the computed
     /// layout.
     pub storage_conflicts: BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal>,
 }
 
+impl Debug for GeneratorLayout<'_> {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        /// Prints an iterator of (key, value) tuples as a map.
+        struct MapPrinter<'a, K, V>(Cell<Option<Box<dyn Iterator<Item = (K, V)> + 'a>>>);
+        impl<'a, K, V> MapPrinter<'a, K, V> {
+            fn new(iter: impl Iterator<Item = (K, V)> + 'a) -> Self {
+                Self(Cell::new(Some(Box::new(iter))))
+            }
+        }
+        impl<'a, K: Debug, V: Debug> Debug for MapPrinter<'a, K, V> {
+            fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+                fmt.debug_map().entries(self.0.take().unwrap()).finish()
+            }
+        }
+
+        /// Prints the generator variant name.
+        struct GenVariantPrinter(VariantIdx);
+        impl From<VariantIdx> for GenVariantPrinter {
+            fn from(idx: VariantIdx) -> Self {
+                GenVariantPrinter(idx)
+            }
+        }
+        impl Debug for GenVariantPrinter {
+            fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+                let variant_name = ty::GeneratorSubsts::variant_name(self.0);
+                if fmt.alternate() {
+                    write!(fmt, "{:9}({:?})", variant_name, self.0)
+                } else {
+                    write!(fmt, "{}", variant_name)
+                }
+            }
+        }
+
+        /// Forces its contents to print in regular mode instead of alternate mode.
+        struct OneLinePrinter<T>(T);
+        impl<T: Debug> Debug for OneLinePrinter<T> {
+            fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+                write!(fmt, "{:?}", self.0)
+            }
+        }
+
+        fmt.debug_struct("GeneratorLayout")
+            .field("field_tys", &MapPrinter::new(self.field_tys.iter_enumerated()))
+            .field(
+                "variant_fields",
+                &MapPrinter::new(
+                    self.variant_fields
+                        .iter_enumerated()
+                        .map(|(k, v)| (GenVariantPrinter(k), OneLinePrinter(v))),
+                ),
+            )
+            .field("storage_conflicts", &self.storage_conflicts)
+            .finish()
+    }
+}
+
 #[derive(Debug, RustcEncodable, RustcDecodable, HashStable)]
 pub struct BorrowCheckResult<'tcx> {
     /// All the opaque types that are restricted to concrete types
diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs
@@ -522,8 +522,7 @@ impl<'tcx> GeneratorSubsts<'tcx> {
 
     /// Calls `f` with a reference to the name of the enumerator for the given
     /// variant `v`.
-    #[inline]
-    pub fn variant_name(self, v: VariantIdx) -> Cow<'static, str> {
+    pub fn variant_name(v: VariantIdx) -> Cow<'static, str> {
         match v.as_usize() {
             Self::UNRESUMED => Cow::from(Self::UNRESUMED_NAME),
             Self::RETURNED => Cow::from(Self::RETURNED_NAME),
diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs
@@ -422,6 +422,9 @@ struct LivenessInfo {
     /// The set of saved locals live at each suspension point.
     live_locals_at_suspension_points: Vec<BitSet<GeneratorSavedLocal>>,
 
+    /// Parallel vec to the above with SourceInfo for each yield terminator.
+    source_info_at_suspension_points: Vec<SourceInfo>,
+
     /// For every saved local, the set of other saved locals that are
     /// storage-live at the same time as this local. We cannot overlap locals in
     /// the layout which have conflicting storage.
@@ -473,6 +476,7 @@ fn locals_live_across_suspend_points(
 
     let mut storage_liveness_map = IndexVec::from_elem(None, body.basic_blocks());
     let mut live_locals_at_suspension_points = Vec::new();
+    let mut source_info_at_suspension_points = Vec::new();
     let mut live_locals_at_any_suspension_point = BitSet::new_empty(body.local_decls.len());
 
     for (block, data) in body.basic_blocks().iter_enumerated() {
@@ -518,6 +522,7 @@ fn locals_live_across_suspend_points(
             live_locals_at_any_suspension_point.union(&live_locals);
 
             live_locals_at_suspension_points.push(live_locals);
+            source_info_at_suspension_points.push(data.terminator().source_info);
         }
     }
 
@@ -541,6 +546,7 @@ fn locals_live_across_suspend_points(
     LivenessInfo {
         saved_locals,
         live_locals_at_suspension_points,
+        source_info_at_suspension_points,
         storage_conflicts,
         storage_liveness: storage_liveness_map,
     }
@@ -754,6 +760,7 @@ fn compute_layout<'tcx>(
     let LivenessInfo {
         saved_locals,
         live_locals_at_suspension_points,
+        source_info_at_suspension_points,
         storage_conflicts,
         storage_liveness,
     } = liveness;
@@ -768,7 +775,18 @@ fn compute_layout<'tcx>(
     }
 
     // Leave empty variants for the UNRESUMED, RETURNED, and POISONED states.
+    // In debuginfo, these will correspond to the beginning (UNRESUMED) or end
+    // (RETURNED, POISONED) of the function.
     const RESERVED_VARIANTS: usize = 3;
+    let body_span = body.source_scopes[OUTERMOST_SOURCE_SCOPE].span;
+    let mut variant_source_info: IndexVec<VariantIdx, SourceInfo> = [
+        SourceInfo::outermost(body_span.shrink_to_lo()),
+        SourceInfo::outermost(body_span.shrink_to_hi()),
+        SourceInfo::outermost(body_span.shrink_to_hi()),
+    ]
+    .iter()
+    .copied()
+    .collect();
 
     // Build the generator variant field list.
     // Create a map from local indices to generator struct indices.
@@ -787,11 +805,13 @@ fn compute_layout<'tcx>(
             remap.entry(locals[saved_local]).or_insert((tys[saved_local], variant_index, idx));
         }
         variant_fields.push(fields);
+        variant_source_info.push(source_info_at_suspension_points[suspension_point_idx]);
     }
     debug!("generator variant_fields = {:?}", variant_fields);
     debug!("generator storage_conflicts = {:#?}", storage_conflicts);
 
-    let layout = GeneratorLayout { field_tys: tys, variant_fields, storage_conflicts };
+    let layout =
+        GeneratorLayout { field_tys: tys, variant_fields, variant_source_info, storage_conflicts };
 
     (remap, layout, storage_liveness)
 }
diff --git a/src/librustc_mir/util/pretty.rs b/src/librustc_mir/util/pretty.rs
@@ -131,7 +131,7 @@ fn dump_matched_mir_node<'tcx, F>(
         }
         writeln!(file, " {} {}", disambiguator, pass_name)?;
         if let Some(ref layout) = body.generator_layout {
-            writeln!(file, "// generator_layout = {:?}", layout)?;
+            writeln!(file, "/* generator_layout = {:#?} */", layout)?;
         }
         writeln!(file)?;
         extra_data(PassWhere::BeforeCFG, &mut file)?;
diff --git a/src/test/codegen/async-fn-debug-msvc.rs b/src/test/codegen/async-fn-debug-msvc.rs
@@ -0,0 +1,48 @@
+// Verify debuginfo for generators:
+//  - Each variant points to the file and line of its yield point
+//  - The generator types and variants are marked artificial
+//  - Captured vars from the source are not marked artificial
+//
+// ignore-tidy-linelength
+// compile-flags: -C debuginfo=2 --edition=2018
+// only-msvc
+
+async fn foo() {}
+async fn async_fn_test() {
+    foo().await;
+    let s = String::from("foo");
+    foo().await;
+}
+
+// FIXME: No way to reliably check the filename.
+
+// CHECK-DAG:  [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test"
+// CHECK-DAG:  [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[ASYNC_FN]], {{.*}}flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// For brevity, we only check the struct name and members of the last variant.
+// CHECK-SAME: file: [[FILE:![0-9]*]], line: 11,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 15,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 15,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 12,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 14,
+// CHECK-SAME: baseType: [[VARIANT:![0-9]*]]
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[ASYNC_FN]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]]
+// CHECK-NOT:  flags: DIFlagArtificial
+// CHECK-SAME: )
+
+fn main() {
+    let _dummy = async_fn_test();
+}
diff --git a/src/test/codegen/async-fn-debug.rs b/src/test/codegen/async-fn-debug.rs
@@ -0,0 +1,51 @@
+// Verify debuginfo for async fn:
+//  - Each variant points to the file and line of its yield point
+//  - The generator types and variants are marked artificial
+//  - Captured vars from the source are not marked artificial
+//
+// ignore-tidy-linelength
+// compile-flags: -C debuginfo=2 --edition=2018
+// ignore-msvc
+
+async fn foo() {}
+async fn async_fn_test() {
+    foo().await;
+    let s = String::from("foo");
+    foo().await;
+}
+
+// FIXME: No way to reliably check the filename.
+
+// CHECK-DAG:  [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test"
+// CHECK-DAG:  [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[ASYNC_FN]], {{.*}}flags: DIFlagArtificial
+// CHECK:      [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[ASYNC_FN]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK-SAME: discriminator: [[DISC:![0-9]*]]
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]],
+// CHECK-SAME: file: [[FILE:![0-9]*]], line: 11,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "Unresumed", scope: [[GEN]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]],
+// CHECK-SAME: file: [[FILE]], line: 15,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]],
+// CHECK-SAME: file: [[FILE]], line: 15,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]],
+// CHECK-SAME: file: [[FILE]], line: 12,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]],
+// CHECK-SAME: file: [[FILE]], line: 14,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]]
+// CHECK-NOT:  flags: DIFlagArtificial
+// CHECK-SAME: )
+// CHECK:      [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[ASYNC_FN]],
+// CHECK-SAME: flags: DIFlagArtificial
+
+fn main() {
+    let _dummy = async_fn_test();
+}
diff --git a/src/test/codegen/generator-debug-msvc.rs b/src/test/codegen/generator-debug-msvc.rs
@@ -0,0 +1,52 @@
+// Verify debuginfo for generators:
+//  - Each variant points to the file and line of its yield point
+//  - The generator types and variants are marked artificial
+//  - Captured vars from the source are not marked artificial
+//
+// ignore-tidy-linelength
+// compile-flags: -C debuginfo=2
+// only-msvc
+
+#![feature(generators, generator_trait)]
+use std::ops::Generator;
+
+fn generator_test() -> impl Generator<Yield = i32, Return = ()> {
+    || {
+        yield 0;
+        let s = String::from("foo");
+        yield 1;
+    }
+}
+
+// FIXME: No way to reliably check the filename.
+
+// CHECK-DAG:  [[GEN_FN:!.*]] = !DINamespace(name: "generator_test"
+// CHECK-DAG:  [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]], {{.*}}flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// For brevity, we only check the struct name and members of the last variant.
+// CHECK-SAME: file: [[FILE:![0-9]*]], line: 14,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 18,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 18,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 15,
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]],
+// CHECK-SAME: file: [[FILE]], line: 17,
+// CHECK-SAME: baseType: [[VARIANT:![0-9]*]]
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN_FN]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]],
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK:      {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]]
+// CHECK-NOT:  flags: DIFlagArtificial
+// CHECK-SAME: )
+
+fn main() {
+    let _dummy = generator_test();
+}
diff --git a/src/test/codegen/generator-debug.rs b/src/test/codegen/generator-debug.rs
diff --git a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir
diff --git a/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir b/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir

Original file line number	Diff line number	Diff line change
`@@ -70,10 +70,10 @@ fn uncached_llvm_type<'a, 'tcx>(`
`70`	`70`	`write!(&mut name, "::{}", def.variants[index].ident).unwrap();`
`71`	`71`	`}`
`72`	`72`	`}`
`73`		`- if let (&ty::Generator(_, substs, _), &Variants::Single { index })`
	`73`	`+ if let (&ty::Generator(_, _, _), &Variants::Single { index })`
`74`	`74`	`= (&layout.ty.kind, &layout.variants)`
`75`	`75`	`{`
`76`		`- write!(&mut name, "::{}", substs.as_generator().variant_name(index)).unwrap();`
	`76`	`+ write!(&mut name, "::{}", ty::GeneratorSubsts::variant_name(index)).unwrap();`
`77`	`77`	`}`
`78`	`78`	`Some(name)`
`79`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ fn dump_matched_mir_node<'tcx, F>(`
`131`	`131`	`}`
`132`	`132`	`writeln!(file, " {} {}", disambiguator, pass_name)?;`
`133`	`133`	`if let Some(ref layout) = body.generator_layout {`
`134`		`- writeln!(file, "// generator_layout = {:?}", layout)?;`
	`134`	`+ writeln!(file, "/* generator_layout = {:#?} */", layout)?;`
`135`	`135`	`}`
`136`	`136`	`writeln!(file)?;`
`137`	`137`	`extra_data(PassWhere::BeforeCFG, &mut file)?;`