Skip to content
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.

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

fix: prevent no_predicates from removing predicates in calling function #5452

Merged
merged 3 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
19 changes: 19 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ struct PerFunctionContext<'function> {

/// True if we're currently working on the entry point function.
inlining_entry: bool,

side_effects_enabled: Option<ValueId>,
}

/// Utility function to find out the direct calls of a function.
Expand Down Expand Up @@ -325,6 +327,7 @@ impl<'function> PerFunctionContext<'function> {
blocks: HashMap::default(),
values: HashMap::default(),
inlining_entry: false,
side_effects_enabled: None,
}
}

Expand Down Expand Up @@ -482,12 +485,28 @@ impl<'function> PerFunctionContext<'function> {
Some(func_id) => {
if self.should_inline_call(ssa, func_id) {
self.inline_function(ssa, *id, func_id, arguments);

// This is only relevant during handling functions with `InlineType::NoPredicates` as these
// can pollute the function they're being inlined into with `Instruction::EnabledSideEffects`,
// resulting in predicates not being applied properly.
//
// Note that this doesn't cover the case in which there exists an `Instruction::EnabledSideEffects`
// within the function being inlined whilst the source function has not encountered one yet.
// In practice this isn't an issue as the last `Instruction::EnabledSideEffects` in the
// function being inlined will be to turn off predicates rather than to create one.
if let Some(condition) = self.side_effects_enabled {
self.context.builder.insert_enable_side_effects_if(condition);
}
} else {
self.push_instruction(*id);
}
}
None => self.push_instruction(*id),
},
Instruction::EnableSideEffects { condition } => {
self.side_effects_enabled = Some(self.translate_value(*condition));
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
self.push_instruction(*id);
}
_ => self.push_instruction(*id),
}
}
Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_failure/regression_5435/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_5435"
type = "bin"
authors = [""]
compiler_version = ">=0.31.0"

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_failure/regression_5435/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
input = "0"
enable = false
18 changes: 18 additions & 0 deletions test_programs/execution_failure/regression_5435/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
fn main(input: Field, enable: bool) {
if enable {
let hash = no_predicate_function(input);
// `EnableSideEffects` instruction from above instruction leaks out and removes the predicate from this call,
// resulting in execution failure.
fail(hash);
}
}

#[no_predicates]
fn no_predicate_function(enable: Field) -> Field {
// if-statement ensures that an `EnableSideEffects` instruction is emitted.
if enable == 0 { 1 } else { 0 }
}

unconstrained fn fail(_: Field) {
assert(false);
}
Loading