Skip to content

Commit

Permalink
Fix stack overflow in llvm::Verifier::visitMDNode()
Browse files Browse the repository at this point in the history
This code is a proof-of-concept more than anything else. It is not ready
to be merged in its current state. Any version of this fix must somehow
measure the depth of the metadata nodes (or a rough upper bound), and it
must somehow send that depth to the LLVM codegen entry point. There is
likely a tradeoff to be made between performance and readability here.
  • Loading branch information
LegionMammal978 committed Apr 28, 2022
1 parent b347aa5 commit a68b894
Show file tree
Hide file tree
Showing 12 changed files with 219 additions and 104 deletions.
15 changes: 12 additions & 3 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_codegen_ssa::traits::*;
use rustc_codegen_ssa::{CompiledModule, ModuleCodegen};
use rustc_data_structures::profiling::SelfProfilerRef;
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_data_structures::stack::ensure_recursive_stack;
use rustc_errors::{FatalError, Handler, Level};
use rustc_fs_util::{link_or_copy, path_to_c_string};
use rustc_middle::bug;
Expand Down Expand Up @@ -757,6 +758,13 @@ pub(crate) unsafe fn codegen(
create_msvc_imps(cgcx, llcx, llmod);
}

let stack_size = crate::debuginfo::LLMOD_DEPTHS
.lock()
.unwrap()
.get(&(llmod as *const _ as usize))
.unwrap_or(&0)
* 256;

// A codegen-specific pass manager is used to generate object
// files for an LLVM module.
//
Expand All @@ -769,6 +777,7 @@ pub(crate) unsafe fn codegen(
tm: &'ll llvm::TargetMachine,
llmod: &'ll llvm::Module,
no_builtins: bool,
stack_size: usize,
f: F,
) -> R
where
Expand All @@ -777,7 +786,7 @@ pub(crate) unsafe fn codegen(
let cpm = llvm::LLVMCreatePassManager();
llvm::LLVMAddAnalysisPasses(tm, cpm);
llvm::LLVMRustAddLibraryInfo(cpm, llmod, no_builtins);
f(cpm)
ensure_recursive_stack(stack_size, || f(cpm))
}

// Two things to note:
Expand Down Expand Up @@ -881,7 +890,7 @@ pub(crate) unsafe fn codegen(
} else {
llmod
};
with_codegen(tm, llmod, config.no_builtins, |cpm| {
with_codegen(tm, llmod, config.no_builtins, stack_size, |cpm| {
write_output_file(
diag_handler,
tm,
Expand Down Expand Up @@ -916,7 +925,7 @@ pub(crate) unsafe fn codegen(
(_, SplitDwarfKind::Split) => Some(dwo_out.as_path()),
};

with_codegen(tm, llmod, config.no_builtins, |cpm| {
with_codegen(tm, llmod, config.no_builtins, stack_size, |cpm| {
write_output_file(
diag_handler,
tm,
Expand Down
24 changes: 14 additions & 10 deletions compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::metadata::file_metadata;
use super::utils::DIB;
use super::utils::{self, DIB};
use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext};
use rustc_codegen_ssa::traits::*;

Expand Down Expand Up @@ -97,15 +97,19 @@ fn make_mir_scope<'ll, 'tcx>(
let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty());
cx.dbg_scope_fn(callee, callee_fn_abi, None)
}
None => unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlock(
DIB(cx),
parent_scope.dbg_scope.unwrap(),
file_metadata,
loc.line,
loc.col,
)
},
None => {
let dbg_scope = unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlock(
DIB(cx),
parent_scope.dbg_scope.unwrap(),
file_metadata,
loc.line,
loc.col,
)
};
utils::debug_context(cx).add_di_node(dbg_scope);
dbg_scope
}
};

let inlined_at = scope_data.inlined.map(|(_, callsite_span)| {
Expand Down
79 changes: 50 additions & 29 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ fn build_fixed_size_array_di_node<'ll, 'tcx>(

let upper_bound = len.eval_usize(cx.tcx, ty::ParamEnv::reveal_all()) as c_longlong;

let subrange =
unsafe { Some(llvm::LLVMRustDIBuilderGetOrCreateSubrange(DIB(cx), 0, upper_bound)) };
let subrange = unsafe { llvm::LLVMRustDIBuilderGetOrCreateSubrange(DIB(cx), 0, upper_bound) };
debug_context(cx).add_di_node(subrange);

let subscripts = create_DIArray(DIB(cx), &[subrange]);
let subscripts = create_DIArray(cx, &[Some(subrange)]);
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateArrayType(
DIB(cx),
Expand All @@ -149,6 +149,7 @@ fn build_fixed_size_array_di_node<'ll, 'tcx>(
subscripts,
)
};
debug_context(cx).add_di_node(di_node);

DINodeCreationResult::new(di_node, false)
}
Expand Down Expand Up @@ -204,6 +205,7 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
ptr_type_debuginfo_name.len(),
)
};
debug_context(cx).add_di_node(di_node);

DINodeCreationResult { di_node, already_stored_in_typemap: false }
}
Expand Down Expand Up @@ -255,6 +257,7 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
0,
)
};
debug_context(cx).add_di_node(data_ptr_type_di_node);

smallvec![
build_field_di_node(
Expand Down Expand Up @@ -332,9 +335,10 @@ fn build_subroutine_type_di_node<'ll, 'tcx>(
let fn_di_node = unsafe {
llvm::LLVMRustDIBuilderCreateSubroutineType(
DIB(cx),
create_DIArray(DIB(cx), &signature_di_nodes[..]),
create_DIArray(cx, &signature_di_nodes[..]),
)
};
debug_context(cx).add_nested_di_node(fn_di_node, &[3]);

// This is actually a function pointer, so wrap it in pointer DI.
let name = compute_debuginfo_type_name(cx.tcx, fn_ty, false);
Expand All @@ -349,6 +353,7 @@ fn build_subroutine_type_di_node<'ll, 'tcx>(
name.len(),
)
};
debug_context(cx).add_di_node(di_node);

DINodeCreationResult::new(di_node, false)
}
Expand Down Expand Up @@ -498,7 +503,7 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
// FIXME(mw): Cache this via a regular UniqueTypeId instead of an extra field in the debug context.
fn recursion_marker_type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) -> &'ll DIType {
*debug_context(cx).recursion_marker_type.get_or_init(move || {
unsafe {
let di_node = unsafe {
// The choice of type here is pretty arbitrary -
// anything reading the debuginfo for a recursive
// type is going to see *something* weird - the only
Expand All @@ -517,7 +522,9 @@ fn recursion_marker_type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) -> &'ll D
cx.tcx.data_layout.pointer_size.bits(),
DW_ATE_unsigned,
)
}
};
debug_context(cx).add_di_node(di_node);
di_node
})
}

Expand Down Expand Up @@ -595,6 +602,7 @@ fn file_metadata_raw<'ll>(
hash_value.len(),
)
};
debug_context(cx).add_di_node(file_metadata);

v.insert(file_metadata);
file_metadata
Expand Down Expand Up @@ -680,6 +688,7 @@ fn build_basic_type_di_node<'ll, 'tcx>(
encoding,
)
};
debug_context(cx).add_di_node(ty_di_node);

if !cpp_like_debuginfo {
return DINodeCreationResult::new(ty_di_node, false);
Expand All @@ -703,6 +712,7 @@ fn build_basic_type_di_node<'ll, 'tcx>(
None,
)
};
debug_context(cx).add_di_node(typedef_di_node);

DINodeCreationResult::new(typedef_di_node, false)
}
Expand Down Expand Up @@ -740,18 +750,17 @@ fn build_param_type_di_node<'ll, 'tcx>(
) -> DINodeCreationResult<'ll> {
debug!("build_param_type_di_node: {:?}", t);
let name = format!("{:?}", t);
DINodeCreationResult {
di_node: unsafe {
llvm::LLVMRustDIBuilderCreateBasicType(
DIB(cx),
name.as_ptr().cast(),
name.len(),
Size::ZERO.bits(),
DW_ATE_unsigned,
)
},
already_stored_in_typemap: false,
}
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateBasicType(
DIB(cx),
name.as_ptr().cast(),
name.len(),
Size::ZERO.bits(),
DW_ATE_unsigned,
)
};
debug_context(cx).add_di_node(di_node);
DINodeCreationResult { di_node, already_stored_in_typemap: false }
}

pub fn build_compile_unit_di_node<'ll, 'tcx>(
Expand Down Expand Up @@ -838,6 +847,7 @@ pub fn build_compile_unit_di_node<'ll, 'tcx>(
ptr::null(),
0,
);
debug_context.add_di_node(compile_unit_file);

let unit_metadata = llvm::LLVMRustDIBuilderCreateCompileUnit(
debug_context.builder,
Expand All @@ -857,6 +867,7 @@ pub fn build_compile_unit_di_node<'ll, 'tcx>(
0,
tcx.sess.opts.debugging_opts.split_dwarf_inlining,
);
debug_context.add_di_node(unit_metadata);

if tcx.sess.opts.debugging_opts.profile {
let cu_desc_metadata =
Expand Down Expand Up @@ -924,7 +935,7 @@ fn build_field_di_node<'ll, 'tcx>(
flags: DIFlags,
type_di_node: &'ll DIType,
) -> &'ll DIType {
unsafe {
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateMemberType(
DIB(cx),
owner,
Expand All @@ -938,7 +949,9 @@ fn build_field_di_node<'ll, 'tcx>(
flags,
type_di_node,
)
}
};
debug_context(cx).add_di_node(di_node);
di_node
}

/// Creates the debuginfo node for a Rust struct type. Maybe be a regular struct or a tuple-struct.
Expand Down Expand Up @@ -1257,15 +1270,17 @@ fn build_generic_type_param_di_nodes<'ll, 'tcx>(
cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), ty);
let actual_type_di_node = type_di_node(cx, actual_type);
let name = name.as_str();
Some(unsafe {
let template_param = unsafe {
llvm::LLVMRustDIBuilderCreateTemplateTypeParameter(
DIB(cx),
None,
name.as_ptr().cast(),
name.len(),
actual_type_di_node,
)
})
};
debug_context(cx).add_di_node(template_param);
Some(template_param)
} else {
None
}
Expand Down Expand Up @@ -1326,7 +1341,7 @@ pub fn build_global_var_di_node<'ll>(cx: &CodegenCx<'ll, '_>, def_id: DefId, glo

let global_align = cx.align_of(variable_type);

unsafe {
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateStaticVariable(
DIB(cx),
Some(var_scope),
Expand All @@ -1341,8 +1356,9 @@ pub fn build_global_var_di_node<'ll>(cx: &CodegenCx<'ll, '_>, def_id: DefId, glo
global,
None,
global_align.bytes() as u32,
);
}
)
};
debug_context(cx).add_nested_di_node(di_node, &[0, 1]);
}

/// Generates LLVM debuginfo for a vtable.
Expand Down Expand Up @@ -1467,7 +1483,7 @@ pub fn create_vtable_di_node<'ll, 'tcx>(
let vtable_type_di_node = build_vtable_type_di_node(cx, ty, poly_trait_ref);
let linkage_name = "";

unsafe {
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateStaticVariable(
DIB(cx),
NO_SCOPE_METADATA,
Expand All @@ -1482,8 +1498,9 @@ pub fn create_vtable_di_node<'ll, 'tcx>(
vtable,
None,
0,
);
}
)
};
debug_context(cx).add_nested_di_node(di_node, &[0, 1]);
}

/// Creates an "extension" of an existing `DIScope` into another file.
Expand All @@ -1493,7 +1510,11 @@ pub fn extend_scope_to_file<'ll>(
file: &SourceFile,
) -> &'ll DILexicalBlock {
let file_metadata = file_metadata(cx, file);
unsafe { llvm::LLVMRustDIBuilderCreateLexicalBlockFile(DIB(cx), scope_metadata, file_metadata) }
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlockFile(DIB(cx), scope_metadata, file_metadata)
};
debug_context(cx).add_di_node(di_node);
di_node
}

pub fn tuple_field_name(field_index: usize) -> Cow<'static, str> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{
unknown_file_metadata, DINodeCreationResult, SmallVec, NO_GENERICS, NO_SCOPE_METADATA,
UNKNOWN_LINE_NUMBER,
},
utils::DIB,
utils::{debug_context, DIB},
},
llvm::{
self,
Expand Down Expand Up @@ -441,7 +441,7 @@ fn build_union_fields_for_direct_tag_enum_or_generator<'ll, 'tcx>(
// We use LLVMRustDIBuilderCreateMemberType() member type directly because
// the build_field_di_node() function does not support specifying a source location,
// which is something that we don't do anywhere else.
unsafe {
let member_type_di_node = unsafe {
llvm::LLVMRustDIBuilderCreateMemberType(
DIB(cx),
enum_type_di_node,
Expand All @@ -458,7 +458,9 @@ fn build_union_fields_for_direct_tag_enum_or_generator<'ll, 'tcx>(
DIFlags::FlagZero,
variant_member_info.variant_struct_type_di_node,
)
}
};
debug_context(cx).add_di_node(member_type_di_node);
member_type_di_node
}));

debug_assert_eq!(
Expand Down
Loading

0 comments on commit a68b894

Please sign in to comment.