mirror of
https://github.com/roc-lang/roc.git
synced 2025-09-28 14:24:45 +00:00
fix refcount bug
closures were not incremented, but were decremented. This lead to memory corruption
This commit is contained in:
Folkert
parent
30e3f3bd25
commit
f00bd9ba01
5 changed files with 217 additions and 23 deletions
|
|
@ -2173,15 +2173,24 @@ pub fn build_proc_header<'a, 'ctx, 'env>(
|
|||
for (name, layout) in aliases {
|
||||
match layout {
|
||||
Layout::Closure(arguments, closure, result) => {
|
||||
// define closure size and return value size, e.g.
|
||||
//
|
||||
// * roc__mainForHost_1_Update_size() -> i64
|
||||
// * roc__mainForHost_1_Update_result_size() -> i64
|
||||
build_closure_caller(env, &fn_name, *name, arguments, closure, result)
|
||||
}
|
||||
Layout::FunctionPointer(_arguments, _result) => {
|
||||
// TODO should this be considered a closure of size 0?
|
||||
// or do we let the host call it directly?
|
||||
// then we have no RocCallResult wrapping though
|
||||
Layout::FunctionPointer(arguments, result) => {
|
||||
// define function size (equal to pointer size) and return value size, e.g.
|
||||
//
|
||||
// * roc__mainForHost_1_Update_size() -> i64
|
||||
// * roc__mainForHost_1_Update_result_size() -> i64
|
||||
build_function_caller(env, &fn_name, *name, arguments, result)
|
||||
}
|
||||
_ => {
|
||||
// TODO
|
||||
// for anything else we only define the size symbol, e.g.
|
||||
//
|
||||
// * roc__mainForHost_1_Model_size() -> i64
|
||||
build_host_exposed_alias_size(env, &fn_name, *name, layout)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -2301,23 +2310,187 @@ pub fn build_closure_caller<'a, 'ctx, 'env>(
|
|||
|
||||
let closure_data = builder.build_load(closure_data_ptr, "load_closure_data");
|
||||
|
||||
let mut arguments = parameters;
|
||||
arguments.push(closure_data);
|
||||
let mut parameters = parameters;
|
||||
parameters.push(closure_data);
|
||||
|
||||
let result = invoke_and_catch(env, function_value, function_ptr, &arguments, result_type);
|
||||
let call_result = invoke_and_catch(env, function_value, function_ptr, ¶meters, result_type);
|
||||
|
||||
builder.build_store(output, result);
|
||||
builder.build_store(output, call_result);
|
||||
|
||||
builder.build_return(None);
|
||||
|
||||
// STEP 3: build a {} -> u64 function that gives the size of the return type
|
||||
let size_function_type = env.context.i64_type().fn_type(&[], false);
|
||||
let size_function_name: String = format!(
|
||||
"roc_{}_{}_size",
|
||||
build_host_exposed_alias_size_help(
|
||||
env,
|
||||
def_name,
|
||||
alias_symbol,
|
||||
Some("result"),
|
||||
roc_call_result_type.into(),
|
||||
);
|
||||
|
||||
// STEP 4: build a {} -> u64 function that gives the size of the closure
|
||||
let layout = Layout::Closure(arguments, closure.clone(), result);
|
||||
build_host_exposed_alias_size(env, def_name, alias_symbol, &layout);
|
||||
}
|
||||
|
||||
pub fn build_function_caller<'a, 'ctx, 'env>(
|
||||
env: &'a Env<'a, 'ctx, 'env>,
|
||||
def_name: &str,
|
||||
alias_symbol: Symbol,
|
||||
arguments: &[Layout<'a>],
|
||||
result: &Layout<'a>,
|
||||
) {
|
||||
use inkwell::types::BasicType;
|
||||
|
||||
let arena = env.arena;
|
||||
let context = &env.context;
|
||||
let builder = env.builder;
|
||||
|
||||
// STEP 1: build function header
|
||||
|
||||
// e.g. `roc__main_1_Fx_caller`
|
||||
let function_name = format!(
|
||||
"roc_{}_{}_caller",
|
||||
def_name,
|
||||
alias_symbol.ident_string(&env.interns)
|
||||
);
|
||||
|
||||
let mut argument_types = Vec::with_capacity_in(arguments.len() + 3, env.arena);
|
||||
|
||||
for layout in arguments {
|
||||
argument_types.push(basic_type_from_layout(
|
||||
arena,
|
||||
context,
|
||||
layout,
|
||||
env.ptr_bytes,
|
||||
));
|
||||
}
|
||||
|
||||
let function_pointer_type = {
|
||||
let mut args = Vec::new_in(env.arena);
|
||||
args.extend(arguments.iter().cloned());
|
||||
|
||||
// pretend the closure layout is empty
|
||||
args.push(Layout::Struct(&[]));
|
||||
|
||||
let function_layout = Layout::FunctionPointer(&args, result);
|
||||
|
||||
// this is already a (function) pointer type
|
||||
basic_type_from_layout(arena, context, &function_layout, env.ptr_bytes)
|
||||
};
|
||||
argument_types.push(function_pointer_type);
|
||||
|
||||
let closure_argument_type = {
|
||||
let basic_type =
|
||||
basic_type_from_layout(arena, context, &Layout::Struct(&[]), env.ptr_bytes);
|
||||
|
||||
basic_type.ptr_type(AddressSpace::Generic)
|
||||
};
|
||||
argument_types.push(closure_argument_type.into());
|
||||
|
||||
let result_type = basic_type_from_layout(arena, context, result, env.ptr_bytes);
|
||||
|
||||
let roc_call_result_type =
|
||||
context.struct_type(&[context.i64_type().into(), result_type], false);
|
||||
|
||||
let output_type = { roc_call_result_type.ptr_type(AddressSpace::Generic) };
|
||||
argument_types.push(output_type.into());
|
||||
|
||||
let function_type = context.void_type().fn_type(&argument_types, false);
|
||||
|
||||
let function_value = env.module.add_function(
|
||||
function_name.as_str(),
|
||||
function_type,
|
||||
Some(Linkage::External),
|
||||
);
|
||||
|
||||
function_value.set_call_conventions(C_CALL_CONV);
|
||||
|
||||
// STEP 2: build function body
|
||||
|
||||
let entry = context.append_basic_block(function_value, "entry");
|
||||
|
||||
builder.position_at_end(entry);
|
||||
|
||||
let mut parameters = function_value.get_params();
|
||||
let output = parameters.pop().unwrap().into_pointer_value();
|
||||
let _closure_data_ptr = parameters.pop().unwrap().into_pointer_value();
|
||||
let function_ptr = parameters.pop().unwrap().into_pointer_value();
|
||||
|
||||
// let closure_data = context.struct_type(&[], false).const_zero().into();
|
||||
|
||||
let actual_function_type = basic_type_from_layout(
|
||||
arena,
|
||||
context,
|
||||
&Layout::FunctionPointer(arguments, result),
|
||||
env.ptr_bytes,
|
||||
);
|
||||
|
||||
let function_ptr = builder
|
||||
.build_bitcast(function_ptr, actual_function_type, "cast")
|
||||
.into_pointer_value();
|
||||
|
||||
let call_result = invoke_and_catch(env, function_value, function_ptr, ¶meters, result_type);
|
||||
|
||||
builder.build_store(output, call_result);
|
||||
|
||||
builder.build_return(None);
|
||||
|
||||
// STEP 3: build a {} -> u64 function that gives the size of the return type
|
||||
build_host_exposed_alias_size_help(
|
||||
env,
|
||||
def_name,
|
||||
alias_symbol,
|
||||
Some("result"),
|
||||
roc_call_result_type.into(),
|
||||
);
|
||||
|
||||
// STEP 4: build a {} -> u64 function that gives the size of the function
|
||||
let layout = Layout::FunctionPointer(arguments, result);
|
||||
build_host_exposed_alias_size(env, def_name, alias_symbol, &layout);
|
||||
}
|
||||
|
||||
fn build_host_exposed_alias_size<'a, 'ctx, 'env>(
|
||||
env: &'a Env<'a, 'ctx, 'env>,
|
||||
def_name: &str,
|
||||
alias_symbol: Symbol,
|
||||
layout: &Layout<'a>,
|
||||
) {
|
||||
build_host_exposed_alias_size_help(
|
||||
env,
|
||||
def_name,
|
||||
alias_symbol,
|
||||
None,
|
||||
basic_type_from_layout(env.arena, env.context, layout, env.ptr_bytes),
|
||||
)
|
||||
}
|
||||
|
||||
fn build_host_exposed_alias_size_help<'a, 'ctx, 'env>(
|
||||
env: &'a Env<'a, 'ctx, 'env>,
|
||||
def_name: &str,
|
||||
alias_symbol: Symbol,
|
||||
opt_label: Option<&str>,
|
||||
basic_type: BasicTypeEnum<'ctx>,
|
||||
) {
|
||||
let builder = env.builder;
|
||||
let context = env.context;
|
||||
|
||||
let size_function_type = env.context.i64_type().fn_type(&[], false);
|
||||
let size_function_name: String = if let Some(label) = opt_label {
|
||||
format!(
|
||||
"roc_{}_{}_{}_size",
|
||||
def_name,
|
||||
alias_symbol.ident_string(&env.interns),
|
||||
label
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"roc_{}_{}_size",
|
||||
def_name,
|
||||
alias_symbol.ident_string(&env.interns)
|
||||
)
|
||||
};
|
||||
|
||||
let size_function = env.module.add_function(
|
||||
size_function_name.as_str(),
|
||||
size_function_type,
|
||||
|
|
@ -2328,7 +2501,8 @@ pub fn build_closure_caller<'a, 'ctx, 'env>(
|
|||
|
||||
builder.position_at_end(entry);
|
||||
|
||||
let size: BasicValueEnum = roc_call_result_type.size_of().unwrap().into();
|
||||
use inkwell::types::BasicType;
|
||||
let size: BasicValueEnum = basic_type.size_of().unwrap().into();
|
||||
builder.build_return(Some(&size));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2067,7 +2067,7 @@ pub fn allocate_list<'a, 'ctx, 'env>(
|
|||
env: &Env<'a, 'ctx, 'env>,
|
||||
inplace: InPlace,
|
||||
elem_layout: &Layout<'a>,
|
||||
length: IntValue<'ctx>,
|
||||
number_of_elements: IntValue<'ctx>,
|
||||
) -> PointerValue<'ctx> {
|
||||
let builder = env.builder;
|
||||
let ctx = env.context;
|
||||
|
|
@ -2075,10 +2075,11 @@ pub fn allocate_list<'a, 'ctx, 'env>(
|
|||
let len_type = env.ptr_int();
|
||||
let elem_bytes = elem_layout.stack_size(env.ptr_bytes) as u64;
|
||||
let bytes_per_element = len_type.const_int(elem_bytes, false);
|
||||
let number_of_data_bytes = builder.build_int_mul(bytes_per_element, length, "data_length");
|
||||
let number_of_data_bytes =
|
||||
builder.build_int_mul(bytes_per_element, number_of_elements, "data_length");
|
||||
|
||||
let rc1 = match inplace {
|
||||
InPlace::InPlace => length,
|
||||
InPlace::InPlace => number_of_elements,
|
||||
InPlace::Clone => {
|
||||
// the refcount of a new list is initially 1
|
||||
// we assume that the list is indeed used (dead variables are eliminated)
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ use bumpalo::Bump;
|
|||
use inkwell::context::Context;
|
||||
use inkwell::types::BasicTypeEnum::{self, *};
|
||||
use inkwell::types::{ArrayType, BasicType, FunctionType, IntType, PointerType, StructType};
|
||||
use inkwell::values::IntValue;
|
||||
use inkwell::AddressSpace;
|
||||
use roc_mono::layout::Layout;
|
||||
|
||||
|
|
|
|||
|
|
@ -59,12 +59,9 @@ impl<'ctx> PointerToRefcount<'ctx> {
|
|||
let builder = env.builder;
|
||||
// pointer to usize
|
||||
let refcount_type = ptr_int(env.context, env.ptr_bytes);
|
||||
let refcount_ptr_type = refcount_type.ptr_type(AddressSpace::Generic);
|
||||
|
||||
let ptr_as_usize_ptr = cast_basic_basic(
|
||||
builder,
|
||||
data_ptr.into(),
|
||||
refcount_type.ptr_type(AddressSpace::Generic).into(),
|
||||
)
|
||||
let ptr_as_usize_ptr = cast_basic_basic(builder, data_ptr.into(), refcount_ptr_type.into())
|
||||
.into_pointer_value();
|
||||
|
||||
// get a pointer to index -1
|
||||
|
|
@ -431,6 +428,24 @@ pub fn increment_refcount_layout<'a, 'ctx, 'env>(
|
|||
RecursiveUnion(tags) => {
|
||||
build_inc_union(env, layout_ids, tags, value);
|
||||
}
|
||||
Closure(_, closure_layout, _) => {
|
||||
if closure_layout.contains_refcounted() {
|
||||
let wrapper_struct = value.into_struct_value();
|
||||
|
||||
let field_ptr = env
|
||||
.builder
|
||||
.build_extract_value(wrapper_struct, 1, "increment_closure_data")
|
||||
.unwrap();
|
||||
|
||||
increment_refcount_layout(
|
||||
env,
|
||||
parent,
|
||||
layout_ids,
|
||||
field_ptr,
|
||||
&closure_layout.as_block_of_memory_layout(),
|
||||
)
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -587,7 +587,10 @@ impl<'a> Procs<'a> {
|
|||
let symbol = name;
|
||||
|
||||
// TODO should pending_procs hold a Rc<Proc>?
|
||||
let partial_proc = self.partial_procs.get(&symbol).unwrap().clone();
|
||||
let partial_proc = match self.partial_procs.get(&symbol) {
|
||||
Some(p) => p.clone(),
|
||||
None => panic!("no partial_proc for {:?}", symbol),
|
||||
};
|
||||
|
||||
// Mark this proc as in-progress, so if we're dealing with
|
||||
// mutually recursive functions, we don't loop forever.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue