Rollup merge of rust-lang#59446 - Aaron1011:fix/debuginfo-overflow, r…

…=oli-obk

Fix stack overflow when generating debuginfo for 'recursive' type

By using 'impl trait', it's possible to create a self-referential
type as follows:

fn foo() -> impl Copy { foo }

This is a function which returns itself.
Normally, the signature of this function would be impossible
to write - it would look like 'fn foo() -> fn() -> fn() ...'
e.g. a function which returns a function, which returns a function...

Using 'impl trait' allows us to avoid writing this infinitely long
type. While it's useless for practical purposes, it does compile and run

However, issues arise when we try to generate llvm debuginfo for such a
type. All 'impl trait' types (e.g. ty::Opaque) are resolved when we
generate debuginfo, which can lead to us recursing back to the original
'fn' type when we try to process its return type.

To resolve this, I've modified debuginfo generation to account for these
kinds of weird types. Unfortunately, there's no 'correct' debuginfo that
we can generate - 'impl trait' does not exist in debuginfo, and this
kind of recursive type is impossible to directly represent.

To ensure that we emit *something*, this commit emits dummy
debuginfo/type names whenever it encounters a self-reference. In
practice, this should never happen - it's just to ensure that we can
emit some kind of debuginfo, even if it's not particularly meaningful

Fixes rust-lang#58463

src/librustc_codegen_llvm/debuginfo/metadata.rs

@@ -117,6 +117,32 @@ impl TypeMap<'ll, 'tcx> {
         }
     }
 
+    // Removes a Ty to metadata mapping
+    // This is useful when computing the metadata for a potentially
+    // recursive type (e.g. a function ptr of the form:
+    //
+    // fn foo() -> impl Copy { foo }
+    //
+    // This kind of type cannot be properly represented
+    // via LLVM debuginfo. As a workaround,
+    // we register a temporary Ty to metadata mapping
+    // for the function before we compute its actual metadata.
+    // If the metadata computation ends up recursing back to the
+    // original function, it will use the temporary mapping
+    // for the inner self-reference, preventing us from
+    // recursing forever.
+    //
+    // This function is used to remove the temporary metadata
+    // mapping after we've computed the actual metadata
+    fn remove_type(
+        &mut self,
+        type_: Ty<'tcx>,
+    ) {
+        if self.type_to_metadata.remove(type_).is_none() {
+            bug!("Type metadata Ty '{}' is not in the TypeMap!", type_);
+        }
+    }
+
     // Adds a UniqueTypeId to metadata mapping to the TypeMap. The method will
     // fail if the mapping already exists.
     fn register_unique_id_with_metadata(
@@ -608,17 +634,49 @@ pub fn type_metadata(
             }
         }
         ty::FnDef(..) | ty::FnPtr(_) => {
-            let fn_metadata = subroutine_type_metadata(cx,
-                                                       unique_type_id,
-                                                       t.fn_sig(cx.tcx),
-                                                       usage_site_span).metadata;
+
             if let Some(metadata) = debug_context(cx).type_map
                .borrow()
                .find_metadata_for_unique_id(unique_type_id)
             {
                 return metadata;
             }
 
+            // It's possible to create a self-referential
+            // type in Rust by using 'impl trait':
+            //
+            // fn foo() -> impl Copy { foo }
+            //
+            // See TypeMap::remove_type for more detals
+            // about the workaround
+
+            let temp_type = {
+                unsafe {
+                    // The choice of type here is pretty arbitrary -
+                    // anything reading the debuginfo for a recursive
+                    // type is going to see *somthing* weird - the only
+                    // question is what exactly it will see
+                    let (size, align) = cx.size_and_align_of(t);
+                    llvm::LLVMRustDIBuilderCreateBasicType(
+                        DIB(cx),
+                        SmallCStr::new("<recur_type>").as_ptr(),
+                        size.bits(),
+                        align.bits() as u32,
+                        DW_ATE_unsigned)
+                }
+            };
+
+            let type_map = &debug_context(cx).type_map;
+            type_map.borrow_mut().register_type_with_metadata(t, temp_type);
+
+            let fn_metadata = subroutine_type_metadata(cx,
+                                                       unique_type_id,
+                                                       t.fn_sig(cx.tcx),
+                                                       usage_site_span).metadata;
+
+            type_map.borrow_mut().remove_type(t);
+
+
             // This is actually a function pointer, so wrap it in pointer DI
             MetadataCreationResult::new(pointer_type_metadata(cx, t, fn_metadata), false)
 

src/librustc_codegen_llvm/debuginfo/type_names.rs

@@ -5,6 +5,7 @@ use rustc::hir::def_id::DefId;
 use rustc::ty::subst::SubstsRef;
 use rustc::ty::{self, Ty};
 use rustc_codegen_ssa::traits::*;
+use rustc_data_structures::fx::FxHashSet;
 
 use rustc::hir;
 
@@ -17,7 +18,8 @@ pub fn compute_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                                              qualified: bool)
                                              -> String {
     let mut result = String::with_capacity(64);
-    push_debuginfo_type_name(cx, t, qualified, &mut result);
+    let mut visited = FxHashSet::default();
+    push_debuginfo_type_name(cx, t, qualified, &mut result, &mut visited);
     result
 }
 
@@ -26,7 +28,9 @@ pub fn compute_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
 pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                                           t: Ty<'tcx>,
                                           qualified: bool,
-                                          output: &mut String) {
+                                          output: &mut String,
+                                          visited: &mut FxHashSet<Ty<'tcx>>) {
+
     // When targeting MSVC, emit C++ style type names for compatibility with
     // .natvis visualizers (and perhaps other existing native debuggers?)
     let cpp_like_names = cx.sess().target.target.options.is_like_msvc;
@@ -42,12 +46,12 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
         ty::Foreign(def_id) => push_item_name(cx, def_id, qualified, output),
         ty::Adt(def, substs) => {
             push_item_name(cx, def.did, qualified, output);
-            push_type_params(cx, substs, output);
+            push_type_params(cx, substs, output, visited);
         },
         ty::Tuple(component_types) => {
             output.push('(');
             for &component_type in component_types {
-                push_debuginfo_type_name(cx, component_type, true, output);
+                push_debuginfo_type_name(cx, component_type, true, output, visited);
                 output.push_str(", ");
             }
             if !component_types.is_empty() {
@@ -65,7 +69,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                 hir::MutMutable => output.push_str("mut "),
             }
 
-            push_debuginfo_type_name(cx, inner_type, true, output);
+            push_debuginfo_type_name(cx, inner_type, true, output, visited);
 
             if cpp_like_names {
                 output.push('*');
@@ -79,15 +83,15 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                 output.push_str("mut ");
             }
 
-            push_debuginfo_type_name(cx, inner_type, true, output);
+            push_debuginfo_type_name(cx, inner_type, true, output, visited);
 
             if cpp_like_names {
                 output.push('*');
             }
         },
         ty::Array(inner_type, len) => {
             output.push('[');
-            push_debuginfo_type_name(cx, inner_type, true, output);
+            push_debuginfo_type_name(cx, inner_type, true, output, visited);
             output.push_str(&format!("; {}", len.unwrap_usize(cx.tcx)));
             output.push(']');
         },
@@ -98,7 +102,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                 output.push('[');
             }
 
-            push_debuginfo_type_name(cx, inner_type, true, output);
+            push_debuginfo_type_name(cx, inner_type, true, output, visited);
 
             if cpp_like_names {
                 output.push('>');
@@ -113,12 +117,31 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                     &principal,
                 );
                 push_item_name(cx, principal.def_id, false, output);
-                push_type_params(cx, principal.substs, output);
+                push_type_params(cx, principal.substs, output, visited);
             } else {
                 output.push_str("dyn '_");
             }
         },
         ty::FnDef(..) | ty::FnPtr(_) => {
+            // We've encountered a weird 'recursive type'
+            // Currently, the only way to generate such a type
+            // is by using 'impl trait':
+            //
+            // fn foo() -> impl Copy { foo }
+            //
+            // There's not really a sensible name we can generate,
+            // since we don't include 'impl trait' types (e.g. ty::Opaque)
+            // in the output
+            //
+            // Since we need to generate *something*, we just
+            // use a dummy string that should make it clear
+            // that something unusual is going on
+            if !visited.insert(t) {
+                output.push_str("<recursive_type>");
+                return;
+            }
+
+
             let sig = t.fn_sig(cx.tcx);
             if sig.unsafety() == hir::Unsafety::Unsafe {
                 output.push_str("unsafe ");
@@ -136,7 +159,7 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
             let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
             if !sig.inputs().is_empty() {
                 for &parameter_type in sig.inputs() {
-                    push_debuginfo_type_name(cx, parameter_type, true, output);
+                    push_debuginfo_type_name(cx, parameter_type, true, output, visited);
                     output.push_str(", ");
                 }
                 output.pop();
@@ -155,8 +178,20 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
 
             if !sig.output().is_unit() {
                 output.push_str(" -> ");
-                push_debuginfo_type_name(cx, sig.output(), true, output);
+                push_debuginfo_type_name(cx, sig.output(), true, output, visited);
             }
+
+
+            // We only keep the type in 'visited'
+            // for the duration of the body of this method.
+            // It's fine for a particular function type
+            // to show up multiple times in one overall type
+            // (e.g. MyType<fn() -> u8, fn() -> u8>
+            //
+            // We only care about avoiding recursing
+            // directly back to the type we're currently
+            // processing
+            visited.remove(t);
         },
         ty::Closure(..) => {
             output.push_str("closure");
@@ -200,15 +235,16 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
     // common denominator - otherwise we would run into conflicts.
     fn push_type_params<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
                                   substs: SubstsRef<'tcx>,
-                                  output: &mut String) {
+                                  output: &mut String,
+                                  visited: &mut FxHashSet<Ty<'tcx>>) {
         if substs.types().next().is_none() {
             return;
         }
 
         output.push('<');
 
         for type_parameter in substs.types() {
-            push_debuginfo_type_name(cx, type_parameter, true, output);
+            push_debuginfo_type_name(cx, type_parameter, true, output, visited);
             output.push_str(", ");
         }
 

src/test/codegen/fn-impl-trait-self.rs

@@ -0,0 +1,15 @@
+// compile-flags: -g
+//
+// CHECK-LABEL: @main
+// CHECK: {{.*}}DIDerivedType(tag: DW_TAG_pointer_type, name: "fn() -> <recursive_type>",{{.*}}
+//
+// CHECK: {{.*}}DISubroutineType{{.*}}
+// CHECK: {{.*}}DIBasicType(name: "<recur_type>", encoding: DW_ATE_unsigned)
+
+pub fn foo() -> impl Copy {
+    foo
+}
+
+fn main() {
+    let my_res = foo();
+}

src/test/run-pass/issues/issue-58463.rs

@@ -0,0 +1,8 @@
+// run-pass
+// compile-flags:-C debuginfo=2
+fn foo() -> impl Copy {
+    foo
+}
+fn main() {
+    foo();
+}