From a2cb6365ccd3db4f5b5c855eb3bc364d785d2245 Mon Sep 17 00:00:00 2001 From: Will Song Date: Thu, 4 Apr 2024 01:14:23 -0400 Subject: [PATCH 1/4] add library search functionality --- cli/src/main.rs | 6 +- parser/src/include_logic.rs | 73 +++++++++++++++++++++---- parser/src/lib.rs | 4 +- program_analysis/src/analysis_runner.rs | 8 ++- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index ba44fc9..4af428f 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -19,6 +19,10 @@ struct Cli { #[clap(name = "INPUT")] input_files: Vec, + /// Library file paths + #[clap(short = 'L', long = "library", name = "LIBRARIES")] + libraries: Vec, + /// Output level (INFO, WARNING, or ERROR) #[clap(short = 'l', long = "level", name = "LEVEL", default_value = config::DEFAULT_LEVEL)] output_level: MessageCategory, @@ -69,7 +73,7 @@ fn main() -> ExitCode { } // Set up analysis runner. - let (mut runner, reports) = AnalysisRunner::new(options.curve).with_files(&options.input_files); + let (mut runner, reports) = AnalysisRunner::new(options.curve).with_libraries(&options.libraries).with_files(&options.input_files); // Set up writer and write reports to `stdout`. let allow_list = options.allow_list.clone(); diff --git a/parser/src/include_logic.rs b/parser/src/include_logic.rs index 42d2c4c..178ac97 100644 --- a/parser/src/include_logic.rs +++ b/parser/src/include_logic.rs @@ -4,6 +4,7 @@ use super::errors::IncludeError; use program_structure::ast::Include; use program_structure::report::{Report, ReportCollection}; use std::collections::HashSet; +use std::ffi::OsString; use std::fs; use std::path::PathBuf; @@ -11,34 +12,60 @@ pub struct FileStack { current_location: Option, black_paths: HashSet, user_inputs: HashSet, + libraries: Vec, stack: Vec, } +#[derive(Debug)] +struct Library { + dir: bool, + path: PathBuf, +} + impl FileStack { - pub fn new(paths: &[PathBuf], reports: &mut ReportCollection) -> FileStack { + pub fn new(paths: &[PathBuf], libs: &[PathBuf], reports: &mut ReportCollection) -> FileStack { let mut result = FileStack { current_location: None, black_paths: HashSet::new(), user_inputs: HashSet::new(), + libraries: Vec::new(), stack: Vec::new(), }; + result.add_libraries(libs, reports); result.add_files(paths, reports); result.user_inputs = result.stack.iter().cloned().collect::>(); result } + fn add_libraries(&mut self, libs: &[PathBuf], reports: &mut ReportCollection) { + for path in libs { + if path.is_dir() { + self.libraries.push(Library { dir: true, path: path.clone() }); + } else if let Some(extension) = path.extension() { + // Add Circom files to file stack. + if extension == "circom" { + match fs::canonicalize(path) { + Ok(path) => self.libraries.push(Library { dir: false, path: path.clone() }), + Err(_) => { + reports.push( + FileOsError { path: path.display().to_string() }.into_report(), + ); + } + } + } + } + } + } + fn add_files(&mut self, paths: &[PathBuf], reports: &mut ReportCollection) { for path in paths { if path.is_dir() { // Handle directories on a best effort basis only. - let mut paths = Vec::new(); if let Ok(entries) = fs::read_dir(path) { - for entry in entries.flatten() { - paths.push(entry.path()) - } + let paths: Vec<_> = entries.flatten().map(|x| x.path()).collect(); + self.add_files(&paths, reports); } - self.add_files(&paths, reports); } else if let Some(extension) = path.extension() { // Add Circom files to file stack. if extension == "circom" { @@ -66,16 +93,38 @@ impl FileStack { Ok(()) } Err(_) => { - let error = IncludeError { - path: include.path.clone(), - file_id: include.meta.file_id, - file_location: include.meta.file_location(), - }; - Err(Box::new(error.into_report())) + self.include_library(include) } } } + fn include_library(&mut self, include: &Include) -> Result<(), Box> { + // try and perform library resolution on the include + let pathos = OsString::from(include.path.clone()); + for lib in &self.libraries { + if lib.dir { + let libpath = lib.path.join(&include.path); + if fs::canonicalize(&libpath).is_ok() { + self.stack.push(libpath); + return Ok(()); + } + } else { + // only match include paths with a single component i.e. lib.circom and not dir/lib.circom + if lib.path.file_name().expect("good library file") == pathos { + self.stack.push(lib.path.clone()); + return Ok(()); + } + } + } + + let error = IncludeError { + path: include.path.clone(), + file_id: include.meta.file_id, + file_location: include.meta.file_location(), + }; + Err(Box::new(error.into_report())) + } + pub fn take_next(&mut self) -> Option { loop { match self.stack.pop() { diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 51ba636..76e8a4d 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -36,9 +36,9 @@ pub enum ParseResult { Library(Box, ReportCollection), } -pub fn parse_files(file_paths: &[PathBuf], compiler_version: &Version) -> ParseResult { +pub fn parse_files(file_paths: &[PathBuf], libraries: &[PathBuf], compiler_version: &Version) -> ParseResult { let mut reports = ReportCollection::new(); - let mut file_stack = FileStack::new(file_paths, &mut reports); + let mut file_stack = FileStack::new(file_paths, libraries, &mut reports); let mut file_library = FileLibrary::new(); let mut definitions = HashMap::new(); let mut main_components = Vec::new(); diff --git a/program_analysis/src/analysis_runner.rs b/program_analysis/src/analysis_runner.rs index 3aa128a..0f27a01 100644 --- a/program_analysis/src/analysis_runner.rs +++ b/program_analysis/src/analysis_runner.rs @@ -30,6 +30,7 @@ type ReportCache = HashMap; #[derive(Default)] pub struct AnalysisRunner { curve: Curve, + libraries: Vec, /// The corresponding file library including file includes. file_library: FileLibrary, /// Template ASTs generated by the parser. @@ -51,8 +52,13 @@ impl AnalysisRunner { AnalysisRunner { curve, ..Default::default() } } + pub fn with_libraries(mut self, libraries: &[PathBuf]) -> Self { + self.libraries.extend_from_slice(libraries); + self + } + pub fn with_files(mut self, input_files: &[PathBuf]) -> (Self, ReportCollection) { - let reports = match parser::parse_files(input_files, &config::COMPILER_VERSION) { + let reports = match parser::parse_files(input_files, &self.libraries, &config::COMPILER_VERSION) { ParseResult::Program(program, warnings) => { self.template_asts = program.templates; self.function_asts = program.functions; From b34ac2444b0a393ae64b40fbb9ba69a8694a275b Mon Sep 17 00:00:00 2001 From: Will Song Date: Thu, 4 Apr 2024 01:40:03 -0400 Subject: [PATCH 2/4] fix local includes --- parser/src/include_logic.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/parser/src/include_logic.rs b/parser/src/include_logic.rs index 178ac97..80cb28c 100644 --- a/parser/src/include_logic.rs +++ b/parser/src/include_logic.rs @@ -100,17 +100,24 @@ impl FileStack { fn include_library(&mut self, include: &Include) -> Result<(), Box> { // try and perform library resolution on the include + // at this point any absolute path has been handled by the push in add_include let pathos = OsString::from(include.path.clone()); for lib in &self.libraries { if lib.dir { + // only match relative paths that do not start with . + if include.path.find('.') == Some(0) { + continue; + } + let libpath = lib.path.join(&include.path); if fs::canonicalize(&libpath).is_ok() { self.stack.push(libpath); return Ok(()); } } else { - // only match include paths with a single component i.e. lib.circom and not dir/lib.circom - if lib.path.file_name().expect("good library file") == pathos { + // only match include paths with a single component i.e. lib.circom and not dir/lib.circom or + // ./lib.circom + if include.path.find(std::path::MAIN_SEPARATOR) == None && lib.path.file_name().expect("good library file") == pathos { self.stack.push(lib.path.clone()); return Ok(()); } From c4bb707cdca5c325e46f95c62cfe39a2fb6dd7c3 Mon Sep 17 00:00:00 2001 From: Will Song Date: Thu, 4 Apr 2024 01:49:12 -0400 Subject: [PATCH 3/4] cargo fmt --- cli/src/main.rs | 4 ++- parser/src/include_logic.rs | 18 ++++++------ parser/src/lib.rs | 6 +++- program_analysis/src/analysis_runner.rs | 37 +++++++++++++------------ 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 4af428f..a6cb65e 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -73,7 +73,9 @@ fn main() -> ExitCode { } // Set up analysis runner. - let (mut runner, reports) = AnalysisRunner::new(options.curve).with_libraries(&options.libraries).with_files(&options.input_files); + let (mut runner, reports) = AnalysisRunner::new(options.curve) + .with_libraries(&options.libraries) + .with_files(&options.input_files); // Set up writer and write reports to `stdout`. let allow_list = options.allow_list.clone(); diff --git a/parser/src/include_logic.rs b/parser/src/include_logic.rs index 80cb28c..2879043 100644 --- a/parser/src/include_logic.rs +++ b/parser/src/include_logic.rs @@ -18,8 +18,8 @@ pub struct FileStack { #[derive(Debug)] struct Library { - dir: bool, - path: PathBuf, + dir: bool, + path: PathBuf, } impl FileStack { @@ -92,9 +92,7 @@ impl FileStack { } Ok(()) } - Err(_) => { - self.include_library(include) - } + Err(_) => self.include_library(include), } } @@ -117,7 +115,9 @@ impl FileStack { } else { // only match include paths with a single component i.e. lib.circom and not dir/lib.circom or // ./lib.circom - if include.path.find(std::path::MAIN_SEPARATOR) == None && lib.path.file_name().expect("good library file") == pathos { + if include.path.find(std::path::MAIN_SEPARATOR) == None + && lib.path.file_name().expect("good library file") == pathos + { self.stack.push(lib.path.clone()); return Ok(()); } @@ -125,9 +125,9 @@ impl FileStack { } let error = IncludeError { - path: include.path.clone(), - file_id: include.meta.file_id, - file_location: include.meta.file_location(), + path: include.path.clone(), + file_id: include.meta.file_id, + file_location: include.meta.file_location(), }; Err(Box::new(error.into_report())) } diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 76e8a4d..fdd4070 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -36,7 +36,11 @@ pub enum ParseResult { Library(Box, ReportCollection), } -pub fn parse_files(file_paths: &[PathBuf], libraries: &[PathBuf], compiler_version: &Version) -> ParseResult { +pub fn parse_files( + file_paths: &[PathBuf], + libraries: &[PathBuf], + compiler_version: &Version, +) -> ParseResult { let mut reports = ReportCollection::new(); let mut file_stack = FileStack::new(file_paths, libraries, &mut reports); let mut file_library = FileLibrary::new(); diff --git a/program_analysis/src/analysis_runner.rs b/program_analysis/src/analysis_runner.rs index 0f27a01..1abb656 100644 --- a/program_analysis/src/analysis_runner.rs +++ b/program_analysis/src/analysis_runner.rs @@ -53,25 +53,26 @@ impl AnalysisRunner { } pub fn with_libraries(mut self, libraries: &[PathBuf]) -> Self { - self.libraries.extend_from_slice(libraries); - self + self.libraries.extend_from_slice(libraries); + self } pub fn with_files(mut self, input_files: &[PathBuf]) -> (Self, ReportCollection) { - let reports = match parser::parse_files(input_files, &self.libraries, &config::COMPILER_VERSION) { - ParseResult::Program(program, warnings) => { - self.template_asts = program.templates; - self.function_asts = program.functions; - self.file_library = program.file_library; - warnings - } - ParseResult::Library(library, warnings) => { - self.template_asts = library.templates; - self.function_asts = library.functions; - self.file_library = library.file_library; - warnings - } - }; + let reports = + match parser::parse_files(input_files, &self.libraries, &config::COMPILER_VERSION) { + ParseResult::Program(program, warnings) => { + self.template_asts = program.templates; + self.function_asts = program.functions; + self.file_library = program.file_library; + warnings + } + ParseResult::Library(library, warnings) => { + self.template_asts = library.templates; + self.function_asts = library.functions; + self.file_library = library.file_library; + warnings + } + }; (self, reports) } @@ -223,7 +224,7 @@ impl AnalysisRunner { // Get the AST corresponding to the template. let Some(ast) = self.template_asts.get(name) else { trace!("failed to lift unknown template `{name}`"); - return Err(AnalysisError::UnknownTemplate { name: name.to_string() }) + return Err(AnalysisError::UnknownTemplate { name: name.to_string() }); }; // Generate the template CFG from the AST. Cache any reports. let mut reports = ReportCollection::new(); @@ -249,7 +250,7 @@ impl AnalysisRunner { // Get the AST corresponding to the function. let Some(ast) = self.function_asts.get(name) else { trace!("failed to lift unknown function `{name}`"); - return Err(AnalysisError::UnknownFunction { name: name.to_string() }) + return Err(AnalysisError::UnknownFunction { name: name.to_string() }); }; // Generate the function CFG from the AST. Cache any reports. let mut reports = ReportCollection::new(); From c946bf33bce69a33af49cf7b46ed16f64753b7e9 Mon Sep 17 00:00:00 2001 From: Will Song Date: Thu, 4 Apr 2024 14:20:12 -0400 Subject: [PATCH 4/4] add some debug info for includes and library resolution --- parser/src/include_logic.rs | 19 +++++++++++++------ parser/src/lib.rs | 5 +++++ .../src/abstract_syntax_tree/ast.rs | 9 +++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/parser/src/include_logic.rs b/parser/src/include_logic.rs index 2879043..ac28318 100644 --- a/parser/src/include_logic.rs +++ b/parser/src/include_logic.rs @@ -1,5 +1,7 @@ use crate::errors::FileOsError; +use log::debug; + use super::errors::IncludeError; use program_structure::ast::Include; use program_structure::report::{Report, ReportCollection}; @@ -85,9 +87,10 @@ impl FileStack { pub fn add_include(&mut self, include: &Include) -> Result<(), Box> { let mut location = self.current_location.clone().expect("parsing file"); location.push(include.path.clone()); - match fs::canonicalize(location) { + match fs::canonicalize(&location) { Ok(path) => { if !self.black_paths.contains(&path) { + debug!("adding local or absolute include `{}`", location.display()); self.stack.push(path); } Ok(()) @@ -108,18 +111,22 @@ impl FileStack { } let libpath = lib.path.join(&include.path); + debug!("searching for `{}` in `{}`", include.path, lib.path.display()); if fs::canonicalize(&libpath).is_ok() { + debug!("adding include `{}` from directory", libpath.display()); self.stack.push(libpath); return Ok(()); } } else { // only match include paths with a single component i.e. lib.circom and not dir/lib.circom or // ./lib.circom - if include.path.find(std::path::MAIN_SEPARATOR) == None - && lib.path.file_name().expect("good library file") == pathos - { - self.stack.push(lib.path.clone()); - return Ok(()); + if include.path.find(std::path::MAIN_SEPARATOR) == None { + debug!("checking if `{}` matches `{}`", include.path, lib.path.display()); + if lib.path.file_name().expect("good library file") == pathos { + debug!("adding include `{}` from file", lib.path.display()); + self.stack.push(lib.path.clone()); + return Ok(()); + } } } } diff --git a/parser/src/lib.rs b/parser/src/lib.rs index fdd4070..6aca1f7 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -52,6 +52,11 @@ pub fn parse_files( if let Some(main_component) = program.main_component { main_components.push((file_id, main_component, program.custom_gates)); } + debug!( + "adding {} definitions from `{}`", + program.definitions.iter().map(|x| x.name()).collect::>().join(", "), + file_path.display(), + ); definitions.insert(file_id, program.definitions); reports.append(&mut warnings); } diff --git a/program_structure/src/abstract_syntax_tree/ast.rs b/program_structure/src/abstract_syntax_tree/ast.rs index 5907892..2c71eac 100644 --- a/program_structure/src/abstract_syntax_tree/ast.rs +++ b/program_structure/src/abstract_syntax_tree/ast.rs @@ -161,6 +161,15 @@ pub fn build_function( Definition::Function { meta, name, args, arg_location, body } } +impl Definition { + pub fn name(&self) -> String { + match self { + Self::Template { name, .. } => name.clone(), + Self::Function { name, .. } => name.clone(), + } + } +} + #[derive(Clone)] pub enum Statement { IfThenElse {