Skip to content

Commit

Permalink
fix: prevent declarations of blackbox functions outside of the stdlib (
Browse files Browse the repository at this point in the history
…#4177)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

I found this [semaphore lib which is redeclaring pedersen for some
reason](https://github.com/siosw/semaphore-noir/blob/main/circuits/src/identity.nr).
Let's ~break it!~ give a nice error message to tell users that they're
doing something wrong here.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Jan 30, 2024
1 parent 8f70e57 commit 9fb6b09
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 0 deletions.
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub enum ResolverError {
InvalidTypeForEntryPoint { span: Span },
#[error("Nested slices are not supported")]
NestedSlices { span: Span },
#[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")]
LowLevelFunctionOutsideOfStdlib { ident: Ident },
}

impl ResolverError {
Expand Down Expand Up @@ -311,6 +313,11 @@ impl From<ResolverError> for Diagnostic {
"Try to use a constant sized array instead".into(),
span,
),
ResolverError::LowLevelFunctionOutsideOfStdlib { ident } => 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(),
ident.span(),
),
}
}
}
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,13 @@ impl<'a> Resolver<'a> {
position: PubPosition::ReturnType,
});
}
let is_low_level_function =
func.attributes().function.as_ref().map_or(false, |func| func.is_low_level());
if !self.path_resolver.module_id().krate.is_stdlib() && is_low_level_function {
let error =
ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() };
self.push_err(error);
}

// 'pub' is required on return types for entry point functions
if self.is_entry_point_function(func)
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ mod test {
) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) {
let root = std::path::Path::new("/");
let fm = FileManager::new(root);

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);

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]
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])
}

0 comments on commit 9fb6b09

Please sign in to comment.