From 5579da90f4e35d6e41ad517ca58bfe5983b697e2 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Thu, 24 Aug 2023 03:29:19 +0800 Subject: [PATCH 1/7] fix: restrict duplicate macros --- huff_parser/src/lib.rs | 7 ++++ huff_parser/tests/macro.rs | 66 ++++++++++++++++++++++++++++++++++++++ huff_utils/src/error.rs | 10 ++++++ 3 files changed, 83 insertions(+) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index 48e88c0e..709bc691 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -104,6 +104,13 @@ impl Parser { TokenKind::Macro | TokenKind::Fn | TokenKind::Test => { let m = self.parse_macro()?; tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name); + if contract.macros.iter().any(|existing| existing.name == m.name) { + return Err(ParserError { + kind: ParserErrorKind::DuplicateMacro(m.name), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![self.spans[2].clone()]), + }) + } contract.macros.push(m); } TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => { diff --git a/huff_parser/tests/macro.rs b/huff_parser/tests/macro.rs index bc34e903..412d2136 100644 --- a/huff_parser/tests/macro.rs +++ b/huff_parser/tests/macro.rs @@ -1232,3 +1232,69 @@ fn empty_test_with_multi_flag_decorator() { assert_eq!(macro_definition, expected); assert_eq!(parser.current_token.kind, TokenKind::Eof); } + +#[test] +fn test_duplicate_macro_error() { + let source = r#" + #define macro CONSTRUCTOR() = takes(0) returns (0) {} + + #define macro MINT() = takes(0) returns (0) { + 0x04 calldataload // [to] + 0x00 // [from (0x00), to] + 0x24 calldataload // [value, from, to] + } + + #define macro MINT() = takes(0) returns (0) { + 0x04 calldataload // [to] + 0x00 // [from (0x00), to] + 0x24 calldataload // [value, from, to] + } + + #define macro MAIN() = takes(0) returns (0) { + 0x00 calldataload 0xE0 shr + dup1 0x40c10f19 eq mints jumpi + + mints: + MINT() + } + "#; + + //let const_start = source.find("MINT()").unwrap_or(0); + //let const_end = const_start + "MINT()".len() - 1; + let mut start = 0; + let target = "MINT"; + let target_len = target.len(); + let mut occurrences = Vec::new(); + + while let Some(pos) = source[start..].find(target) { + let adjusted_start = start + pos; + let adjusted_end = adjusted_start + target_len - 1; + + occurrences.push((adjusted_start, adjusted_end)); + start = adjusted_end + 1; + } + + let full_source = FullFileSource { source, file: None, spans: vec![] }; + let lexer = Lexer::new(full_source.source); + let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::>(); + let mut parser = Parser::new(tokens, Some("".to_string())); + + // This should be caught before codegen invalid macro statement + match parser.parse() { + Ok(_) => panic!("moose"), + Err(e) => { + assert_eq!( + e, + ParserError { + kind: ParserErrorKind::DuplicateMacro("MINT".to_string()), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![Span { + start: occurrences[1].0.clone(), + end: occurrences[1].1.clone(), + file: None + }]), + } + ) + } + } +} diff --git a/huff_utils/src/error.rs b/huff_utils/src/error.rs index 9ecb2631..b1d1af81 100644 --- a/huff_utils/src/error.rs +++ b/huff_utils/src/error.rs @@ -63,6 +63,8 @@ pub enum ParserErrorKind { InvalidDecoratorFlag(String), /// Invalid decorator flag argument InvalidDecoratorFlagArg(TokenKind), + /// Duplicate MACRO + DuplicateMacro(String), } /// A Lexing Error @@ -488,6 +490,14 @@ impl fmt::Display for CompilerError { pe.spans.error(pe.hint.as_ref()) ) } + ParserErrorKind::DuplicateMacro(mn) => { + write!( + f, + "\nError: Duplicate MACRO name found: \"{}\" \n{}\n", + mn, + pe.spans.error(pe.hint.as_ref()) + ) + } }, CompilerError::PathBufRead(os_str) => { write!( From c8ffae5787b320b32e7127ffb0c9d54f38baff06 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Fri, 8 Sep 2023 01:22:37 +0800 Subject: [PATCH 2/7] refactor: create seperate method to check duplicates --- huff_parser/src/lib.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index 709bc691..22b122c5 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -104,13 +104,7 @@ impl Parser { TokenKind::Macro | TokenKind::Fn | TokenKind::Test => { let m = self.parse_macro()?; tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name); - if contract.macros.iter().any(|existing| existing.name == m.name) { - return Err(ParserError { - kind: ParserErrorKind::DuplicateMacro(m.name), - hint: Some("MACRO names should be unique".to_string()), - spans: AstSpan(vec![self.spans[2].clone()]), - }) - } + self.check_duplicate_macro(&mut contract, m.clone())?; contract.macros.push(m); } TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => { @@ -192,6 +186,23 @@ impl Parser { std::mem::discriminant(&self.current_token.kind) == std::mem::discriminant(&kind) } + /// Checks if there is a duplicate macro name + pub fn check_duplicate_macro( + &self, + contract: &mut Contract, + m: MacroDefinition, + ) -> Result<(), ParserError> { + if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() { + return Err(ParserError { + kind: ParserErrorKind::DuplicateMacro(m.name), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![self.spans[2].clone()]), + }) + } else { + Ok(()) + } + } + /// Consumes the next token. pub fn consume(&mut self) { self.spans.push(self.current_token.span.clone()); From 33484be9117a1da9baf5cabfaab03833a5a729b9 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Thu, 24 Aug 2023 03:29:19 +0800 Subject: [PATCH 3/7] fix: restrict duplicate macros --- huff_parser/src/lib.rs | 7 ++++ huff_parser/tests/macro.rs | 66 ++++++++++++++++++++++++++++++++++++++ huff_utils/src/error.rs | 10 ++++++ 3 files changed, 83 insertions(+) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index 48e88c0e..709bc691 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -104,6 +104,13 @@ impl Parser { TokenKind::Macro | TokenKind::Fn | TokenKind::Test => { let m = self.parse_macro()?; tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name); + if contract.macros.iter().any(|existing| existing.name == m.name) { + return Err(ParserError { + kind: ParserErrorKind::DuplicateMacro(m.name), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![self.spans[2].clone()]), + }) + } contract.macros.push(m); } TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => { diff --git a/huff_parser/tests/macro.rs b/huff_parser/tests/macro.rs index bc34e903..412d2136 100644 --- a/huff_parser/tests/macro.rs +++ b/huff_parser/tests/macro.rs @@ -1232,3 +1232,69 @@ fn empty_test_with_multi_flag_decorator() { assert_eq!(macro_definition, expected); assert_eq!(parser.current_token.kind, TokenKind::Eof); } + +#[test] +fn test_duplicate_macro_error() { + let source = r#" + #define macro CONSTRUCTOR() = takes(0) returns (0) {} + + #define macro MINT() = takes(0) returns (0) { + 0x04 calldataload // [to] + 0x00 // [from (0x00), to] + 0x24 calldataload // [value, from, to] + } + + #define macro MINT() = takes(0) returns (0) { + 0x04 calldataload // [to] + 0x00 // [from (0x00), to] + 0x24 calldataload // [value, from, to] + } + + #define macro MAIN() = takes(0) returns (0) { + 0x00 calldataload 0xE0 shr + dup1 0x40c10f19 eq mints jumpi + + mints: + MINT() + } + "#; + + //let const_start = source.find("MINT()").unwrap_or(0); + //let const_end = const_start + "MINT()".len() - 1; + let mut start = 0; + let target = "MINT"; + let target_len = target.len(); + let mut occurrences = Vec::new(); + + while let Some(pos) = source[start..].find(target) { + let adjusted_start = start + pos; + let adjusted_end = adjusted_start + target_len - 1; + + occurrences.push((adjusted_start, adjusted_end)); + start = adjusted_end + 1; + } + + let full_source = FullFileSource { source, file: None, spans: vec![] }; + let lexer = Lexer::new(full_source.source); + let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::>(); + let mut parser = Parser::new(tokens, Some("".to_string())); + + // This should be caught before codegen invalid macro statement + match parser.parse() { + Ok(_) => panic!("moose"), + Err(e) => { + assert_eq!( + e, + ParserError { + kind: ParserErrorKind::DuplicateMacro("MINT".to_string()), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![Span { + start: occurrences[1].0.clone(), + end: occurrences[1].1.clone(), + file: None + }]), + } + ) + } + } +} diff --git a/huff_utils/src/error.rs b/huff_utils/src/error.rs index 9ecb2631..b1d1af81 100644 --- a/huff_utils/src/error.rs +++ b/huff_utils/src/error.rs @@ -63,6 +63,8 @@ pub enum ParserErrorKind { InvalidDecoratorFlag(String), /// Invalid decorator flag argument InvalidDecoratorFlagArg(TokenKind), + /// Duplicate MACRO + DuplicateMacro(String), } /// A Lexing Error @@ -488,6 +490,14 @@ impl fmt::Display for CompilerError { pe.spans.error(pe.hint.as_ref()) ) } + ParserErrorKind::DuplicateMacro(mn) => { + write!( + f, + "\nError: Duplicate MACRO name found: \"{}\" \n{}\n", + mn, + pe.spans.error(pe.hint.as_ref()) + ) + } }, CompilerError::PathBufRead(os_str) => { write!( From 39ac6bf53b432be896f457ce4b38f4f4a9e44139 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Fri, 8 Sep 2023 01:22:37 +0800 Subject: [PATCH 4/7] refactor: create seperate method to check duplicates --- huff_parser/src/lib.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index 709bc691..22b122c5 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -104,13 +104,7 @@ impl Parser { TokenKind::Macro | TokenKind::Fn | TokenKind::Test => { let m = self.parse_macro()?; tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name); - if contract.macros.iter().any(|existing| existing.name == m.name) { - return Err(ParserError { - kind: ParserErrorKind::DuplicateMacro(m.name), - hint: Some("MACRO names should be unique".to_string()), - spans: AstSpan(vec![self.spans[2].clone()]), - }) - } + self.check_duplicate_macro(&mut contract, m.clone())?; contract.macros.push(m); } TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => { @@ -192,6 +186,23 @@ impl Parser { std::mem::discriminant(&self.current_token.kind) == std::mem::discriminant(&kind) } + /// Checks if there is a duplicate macro name + pub fn check_duplicate_macro( + &self, + contract: &mut Contract, + m: MacroDefinition, + ) -> Result<(), ParserError> { + if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() { + return Err(ParserError { + kind: ParserErrorKind::DuplicateMacro(m.name), + hint: Some("MACRO names should be unique".to_string()), + spans: AstSpan(vec![self.spans[2].clone()]), + }) + } else { + Ok(()) + } + } + /// Consumes the next token. pub fn consume(&mut self) { self.spans.push(self.current_token.span.clone()); From 96e7c8de90af51da291b4e5eea034f2897b0ce44 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Sun, 10 Sep 2023 22:08:15 +0000 Subject: [PATCH 5/7] fix: run clippy --- huff_parser/src/lib.rs | 6 +++--- huff_parser/tests/macro.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index 22b122c5..c29fc5d1 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -104,7 +104,7 @@ impl Parser { TokenKind::Macro | TokenKind::Fn | TokenKind::Test => { let m = self.parse_macro()?; tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name); - self.check_duplicate_macro(&mut contract, m.clone())?; + self.check_duplicate_macro(&contract, m.clone())?; contract.macros.push(m); } TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => { @@ -189,11 +189,11 @@ impl Parser { /// Checks if there is a duplicate macro name pub fn check_duplicate_macro( &self, - contract: &mut Contract, + contract: &Contract, m: MacroDefinition, ) -> Result<(), ParserError> { if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() { - return Err(ParserError { + Err(ParserError { kind: ParserErrorKind::DuplicateMacro(m.name), hint: Some("MACRO names should be unique".to_string()), spans: AstSpan(vec![self.spans[2].clone()]), diff --git a/huff_parser/tests/macro.rs b/huff_parser/tests/macro.rs index 412d2136..6217eda2 100644 --- a/huff_parser/tests/macro.rs +++ b/huff_parser/tests/macro.rs @@ -1289,8 +1289,8 @@ fn test_duplicate_macro_error() { kind: ParserErrorKind::DuplicateMacro("MINT".to_string()), hint: Some("MACRO names should be unique".to_string()), spans: AstSpan(vec![Span { - start: occurrences[1].0.clone(), - end: occurrences[1].1.clone(), + start: occurrences[1].0, + end: occurrences[1].1, file: None }]), } From 790887cb69c29bb6529af999dce8e201e935c971 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Sat, 30 Sep 2023 20:23:08 +0530 Subject: [PATCH 6/7] chore: add tracing::error --- huff_parser/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs index c6f40627..1bc5ad49 100644 --- a/huff_parser/src/lib.rs +++ b/huff_parser/src/lib.rs @@ -193,6 +193,7 @@ impl Parser { m: MacroDefinition, ) -> Result<(), ParserError> { if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() { + tracing::error!(target: "parser", "DUPLICATE MACRO NAME FOUND: {}", m.name); Err(ParserError { kind: ParserErrorKind::DuplicateMacro(m.name), hint: Some("MACRO names should be unique".to_string()), From dc2943d45acd8b6bd14765224e340cc70aaa1411 Mon Sep 17 00:00:00 2001 From: PraneshASP Date: Sat, 30 Sep 2023 20:23:40 +0530 Subject: [PATCH 7/7] chore: remove comments --- huff_parser/tests/macro.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/huff_parser/tests/macro.rs b/huff_parser/tests/macro.rs index 6217eda2..a2017997 100644 --- a/huff_parser/tests/macro.rs +++ b/huff_parser/tests/macro.rs @@ -1259,8 +1259,6 @@ fn test_duplicate_macro_error() { } "#; - //let const_start = source.find("MINT()").unwrap_or(0); - //let const_end = const_start + "MINT()".len() - 1; let mut start = 0; let target = "MINT"; let target_len = target.len();