Skip to content

Commit

Permalink
Fix a bug in the IR parsing. (FuelLabs#2442)
Browse files Browse the repository at this point in the history
When resolving calls after recreating the IR from parsed input as a
second pass the old code would inject a NOP where the CALL should go and
replace it with a resolved CALL.

This was a problem when the call value was used as an argument to another
call, as the NOP was taken rather than the CALL.  Attempting to replace
the NOP everywhere was very tricky (I couldn't actually manage it in the
end).

A better (though still a bit hacky) approach now is to inject a CALL
from the start, but to a dummy function and replace that function within
the CALL in the resolving pass.  This way any references to the call
value are valid from the start.
  • Loading branch information
otrho authored Aug 2, 2022
1 parent c2aaa0f commit 51a304e
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 34 deletions.
4 changes: 1 addition & 3 deletions sway-ir/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ pub enum Kind {
}

impl Module {
/// Return a new named module of a specific kind.
///
/// NOTE: the name is redundant and will be removed in the future.
/// Return a new module of a specific kind.
pub fn new(context: &mut Context, kind: Kind) -> Module {
let content = ModuleContent {
kind,
Expand Down
61 changes: 30 additions & 31 deletions sway-ir/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,11 +792,8 @@ mod ir_builder {
}

struct PendingCall {
block: Block,
call_value: Value,
call_val: Value,
callee: String,
args: Vec<Value>,
metadata: Option<MetadataIndex>,
}

impl IrBuilder {
Expand Down Expand Up @@ -945,21 +942,24 @@ mod ir_builder {
}
IrAstOperation::Call(callee, args) => {
// We can't resolve calls to other functions until we've done a first pass and
// created them first. So we can insert a NOP here, save the call params and
// replace it with a CALL in a second pass.
let nop = block.ins(context).nop();
self.unresolved_calls.push(PendingCall {
block: *block,
call_value: nop,
callee,
args: args
.iter()
.map(|arg_name| val_map.get(arg_name).unwrap())
.cloned()
.collect::<Vec<Value>>(),
metadata: opt_metadata,
});
nop
// created them first. So we can insert a dummy call here, save the call
// params and update it with the proper callee function in a second pass.
//
// The dummy function we'll use for now is just the current function.
let dummy_func = block.get_function(context);
let call_val = block
.ins(context)
.call(
dummy_func,
&args
.iter()
.map(|arg_name| val_map.get(arg_name).unwrap())
.cloned()
.collect::<Vec<Value>>(),
)
.add_metadatum(context, opt_metadata);
self.unresolved_calls.push(PendingCall { call_val, callee });
call_val
}
IrAstOperation::Cbr(cond_val_name, true_block_name, false_block_name) => block
.ins(context)
Expand Down Expand Up @@ -1147,12 +1147,13 @@ mod ir_builder {
}

fn resolve_calls(self, context: &mut Context) -> Result<(), IrError> {
// All of the call instructions are currently NOPs which need to be replaced with actual
// calls. We couldn't do it above until we'd gone and created all the functions first.
// All of the call instructions are currently invalid (recursive) CALLs to their own
// function, which need to be replaced with the proper callee function. We couldn't do
// it above until we'd gone and created all the functions first.
//
// Now we can loop and find the callee function for each call and replace the NOPs.
// Now we can loop and find the callee function for each call and update them.
for pending_call in self.unresolved_calls {
let function = context
let call_func = context
.functions
.iter()
.find_map(|(idx, content)| {
Expand All @@ -1163,14 +1164,12 @@ mod ir_builder {
}
})
.unwrap();
let call_val =
Value::new_instruction(context, Instruction::Call(function, pending_call.args))
.add_metadatum(context, pending_call.metadata);
pending_call.block.replace_instruction(
context,
pending_call.call_value,
call_val,
)?;

if let Some(Instruction::Call(dummy_func, _args)) =
pending_call.call_val.get_instruction_mut(context)
{
*dummy_func = call_func;
}
}
Ok(())
}
Expand Down
10 changes: 10 additions & 0 deletions sway-ir/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ impl Value {
}
}

pub fn get_instruction_mut<'a>(&self, context: &'a mut Context) -> Option<&'a mut Instruction> {
if let ValueDatum::Instruction(instruction) =
&mut context.values.get_mut(self.0).unwrap().value
{
Some(instruction)
} else {
None
}
}

/// Get the type for this value, if found.
///
/// Arguments and constants always have a type, but only some instructions do.
Expand Down
20 changes: 20 additions & 0 deletions sway-ir/tests/serialize/untyped_arg_to_call.ir
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// This script was causing a verification error:
// An untyped/void value has been passed to a function call.
// Based on the `v2 = call a(v1)` line

script {
// check: fn main
fn main() -> bool {
entry:
v0 = const bool true
v1 = call a(v0)
v2 = call a(v1)
ret bool v2
}

fn a(x: bool) -> bool {
entry:
v0 = const bool true
ret bool v0
}
}

0 comments on commit 51a304e

Please sign in to comment.