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 declarations of blackbox functions outside of the stdlib #4177

Merged
merged 10 commits into from
Jan 30, 2024
10 changes: 10 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
let mut errors = vec![];

let module = ModuleId { krate, local_id: self.module_id };
let in_stdlib = *context.stdlib_crate_id() == krate;

for function in functions {
// check if optional field attribute is compatible with native field
Expand All @@ -218,6 +219,15 @@
}
}

let is_low_level_function =
function.attributes().function.as_ref().map_or(false, |func| func.is_low_level());
if !in_stdlib && is_low_level_function {
let error = DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib {
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
span: function.span(),
};
errors.push((error.into(), self.file_id));
}

let name = function.name_ident().clone();
let func_id = context.def_interner.push_empty_fn();

Expand Down Expand Up @@ -455,7 +465,7 @@
}
}
TraitItem::Type { name } => {
// TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()

Check warning on line 468 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (nickysn)

Check warning on line 468 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (alexvitkov)
if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[trait_id.0.local_id.0]
.declare_type_alias(name.clone(), TypeAliasId::dummy_id())
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub enum DefCollectorErrorKind {
"Either the type or the trait must be from the same crate as the trait implementation"
)]
TraitImplOrphaned { span: Span },
#[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")]
LowLevelFunctionOutsideOfStdlib { span: Span },
#[error("macro error : {0:?}")]
MacroError(MacroError),
}
Expand Down Expand Up @@ -246,6 +248,11 @@ impl From<DefCollectorErrorKind> for Diagnostic {
"Either the type or the trait must be from the same crate as the trait implementation".into(),
span,
),
DefCollectorErrorKind::LowLevelFunctionOutsideOfStdlib { span } => Diagnostic::simple_error(
"Definition of low-level function outside of standard library".into(),
"Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(),
span,
),
DefCollectorErrorKind::MacroError(macro_error) => {
Diagnostic::simple_error(macro_error.primary_message, macro_error.secondary_message.unwrap_or_default(), macro_error.span.unwrap_or_default())
},
Expand Down
13 changes: 11 additions & 2 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod test {

use core::panic;
use std::collections::BTreeMap;
use std::path::Path;

use fm::FileId;

Expand Down Expand Up @@ -51,11 +52,19 @@ mod test {
src: &str,
) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) {
let root = std::path::Path::new("/");
let fm = FileManager::new(root);
let mut fm = FileManager::new(root);
let entrypoint = Path::new("main.nr");
let root_file_id = fm.add_file_with_source(entrypoint, src.to_string()).unwrap();
let path_to_std_lib_file = Path::new("std").join("lib.nr");
let std_file_id =
fm.add_file_with_source_canonical_path(&path_to_std_lib_file, "".to_string()).unwrap();

let mut context = Context::new(fm, Default::default());
context.def_interner.populate_dummy_operator_traits();
let root_file_id = FileId::dummy();

let root_crate_id = context.crate_graph.add_crate_root(root_file_id);
context.crate_graph.add_stdlib(std_file_id);

TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
let (program, parser_errors) = parse_program(src);
let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id));
remove_experimental_warnings(&mut errors);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "builtin_function_declaration"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// This test prevents users from trying to create their own builtin functions as these should only exist in the stdlib.

// This would otherwise be a perfectly valid declaration of the `to_le_bits` builtin function
#[builtin(to_le_bits)]
fn to_le_bits(_x: Field, _bit_size: u32) -> [u1] {}

fn main(x: Field) -> pub u1 {
let bits = to_le_bits(x, 100);
bits[0]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "foreign_function_declaration"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// This test prevents users from trying to create their own blackbox functions as these should only exist in the stdlib.

// This would otherwise be a perfectly valid definition of the `pedersen_hash` black box function,
// however executing the circuit results in an unhelpful ICE.
#[foreign(pedersen_hash)]
fn my_pedersen_hash<N>(_input: [Field; N]) -> Field {}

fn main() -> pub Field {
my_pedersen_hash([1])
}
Loading