Skip to content

Fix stack overflow when generating debuginfo for 'recursive' type #59446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 2, 2019
Merged

Fix stack overflow when generating debuginfo for 'recursive' type #59446

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
512069f
Fix stack overflow when generating debuginfo for 'recursive' type
Aaron1011 Mar 26, 2019
e1837a0
Fix inverted panic check
Aaron1011 Mar 26, 2019
aed7ec4
Add codegen test
Aaron1011 Mar 27, 2019
c4556a5
Fix typos
Aaron1011 Mar 28, 2019
d2584e3
Only track 'visited' state for function types
Aaron1011 Apr 1, 2019
c13daeb
Fix typo
Aaron1011 Apr 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 31 additions & 18 deletions src/librustc_codegen_llvm/debuginfo/type_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,6 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
output: &mut String,
visited: &mut FxHashSet<Ty<'tcx>>) {

// 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;
}

// 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;
Expand Down Expand Up @@ -141,6 +123,25 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
}
},
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 ");
Expand Down Expand Up @@ -179,6 +180,18 @@ pub fn push_debuginfo_type_name<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
output.push_str(" -> ");
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 currentlu
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: "currentlu" should be "currently"

// processing
visited.remove(t);
},
ty::Closure(..) => {
output.push_str("closure");
Expand Down