From 9ea712598cefd409d377cfd38bdc6f6aeec19534 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Wed, 14 Jun 2023 12:22:16 +0200 Subject: [PATCH 1/4] use oncelock/threadlocal where possible --- fuzz/fuzz_targets/rome_common.rs | 109 ++++++++++++++++--------------- 1 file changed, 57 insertions(+), 52 deletions(-) diff --git a/fuzz/fuzz_targets/rome_common.rs b/fuzz/fuzz_targets/rome_common.rs index 158045afc7a..f79a525f375 100644 --- a/fuzz/fuzz_targets/rome_common.rs +++ b/fuzz/fuzz_targets/rome_common.rs @@ -18,6 +18,7 @@ use rome_json_parser::parse_json; use rome_service::Rules; use similar::TextDiff; use std::fmt::{Display, Formatter}; +use std::sync::OnceLock; pub fn fuzz_js_parser_with_source_type(data: &[u8], source: JsFileSource) -> Corpus { let Ok(code1) = std::str::from_utf8(data) else { return Corpus::Reject; }; @@ -32,9 +33,13 @@ pub fn fuzz_js_parser_with_source_type(data: &[u8], source: JsFileSource) -> Cor Corpus::Keep } -static mut ANALYSIS_RULES: Option = None; -static mut ANALYSIS_RULE_FILTERS: Option> = None; -static mut ANALYSIS_OPTIONS: Option = None; +static ANALYSIS_RULES: OnceLock = OnceLock::new(); +static ANALYSIS_RULE_FILTERS: OnceLock> = OnceLock::new(); + +// have to use thread local because AnalyzerOptions contains a Box, which isn't thread-safe +thread_local! { + static ANALYSIS_OPTIONS: AnalyzerOptions = AnalyzerOptions::default(); +} struct DiagnosticDescriptionExtractor<'a, D> { diagnostic: &'a D, @@ -58,43 +63,36 @@ where pub fn fuzz_js_formatter_with_source_type(data: &[u8], source: JsFileSource) -> Corpus { let Ok(code1) = std::str::from_utf8(data) else { return Corpus::Reject; }; - // TODO: replace with OnceLock when upgrading to 1.70 - let rule_filters = if let Some(rules) = unsafe { ANALYSIS_RULE_FILTERS.as_ref() } { - rules - } else { - let rules = unsafe { - ANALYSIS_RULES.get_or_insert_with(|| Rules { - all: Some(true), - ..Default::default() - }) - }; - let rules = rules.as_enabled_rules().into_iter().collect::>(); - unsafe { - ANALYSIS_RULE_FILTERS = Some(rules); - ANALYSIS_RULE_FILTERS.as_ref().unwrap_unchecked() - } - }; - let options = unsafe { ANALYSIS_OPTIONS.get_or_insert_with(AnalyzerOptions::default) }; + let rules = ANALYSIS_RULES.get_or_init(|| Rules { + all: Some(true), + ..Default::default() + }); + let rule_filters = ANALYSIS_RULE_FILTERS + .get_or_init(|| rules.as_enabled_rules().into_iter().collect::>()); let parse1 = parse(code1, source); if !parse1.has_errors() { let language = JsFormatLanguage::new(JsFormatOptions::new(source)); let tree1 = parse1.tree(); let mut linter_errors = Vec::new(); - let _ = analyze( - &tree1, - AnalysisFilter::from_enabled_rules(Some(rule_filters)), - options, - source, - |e| -> ControlFlow<()> { - if let Some(diagnostic) = e.diagnostic() { - linter_errors - .push(DiagnosticDescriptionExtractor::new(&diagnostic).to_string()); - } + let _ = ANALYSIS_OPTIONS + .try_with(|options| { + analyze( + &tree1, + AnalysisFilter::from_enabled_rules(Some(rule_filters)), + options, + source, + |e| -> ControlFlow<()> { + if let Some(diagnostic) = e.diagnostic() { + linter_errors + .push(DiagnosticDescriptionExtractor::new(&diagnostic).to_string()); + } - ControlFlow::Continue(()) - }, - ); + ControlFlow::Continue(()) + }, + ) + }) + .unwrap(); let syntax1 = parse1.syntax(); if let Ok(formatted1) = format_node(&syntax1, language.clone()) { if let Ok(printed1) = formatted1.print() { @@ -108,25 +106,32 @@ pub fn fuzz_js_formatter_with_source_type(data: &[u8], source: JsFileSource) -> .header("original code", "formatted") ); let tree2 = parse2.tree(); - let (maybe_diagnostic, _) = analyze( - &tree2, - AnalysisFilter::from_enabled_rules(Some(rule_filters)), - options, - source, - |e| { - if let Some(diagnostic) = e.diagnostic() { - let new_error = - DiagnosticDescriptionExtractor::new(&diagnostic).to_string(); - if let Some(idx) = linter_errors.iter().position(|e| *e == new_error) { - linter_errors.remove(idx); - } else { - return ControlFlow::Break(new_error); - } - } - - ControlFlow::Continue(()) - }, - ); + let (maybe_diagnostic, _) = ANALYSIS_OPTIONS + .try_with(|options| { + analyze( + &tree2, + AnalysisFilter::from_enabled_rules(Some(rule_filters)), + options, + source, + |e| { + if let Some(diagnostic) = e.diagnostic() { + let new_error = + DiagnosticDescriptionExtractor::new(&diagnostic) + .to_string(); + if let Some(idx) = + linter_errors.iter().position(|e| *e == new_error) + { + linter_errors.remove(idx); + } else { + return ControlFlow::Break(new_error); + } + } + + ControlFlow::Continue(()) + }, + ) + }) + .unwrap(); if let Some(diagnostic) = maybe_diagnostic { panic!( "formatter introduced linter failure: {} (expected one of: {})\n{}", From b3b688daa2d8561cd1312b9ef34e8838f9bdc81a Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Wed, 28 Jun 2023 17:03:56 +0200 Subject: [PATCH 2/4] fuzz in CI --- .github/workflows/pull_request.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index ff9ece5e0c5..55e05e3e92e 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -97,6 +97,10 @@ jobs: bins: cargo-fuzz - name: Run init-fuzzer run: bash fuzz/init-fuzzer.sh + - name: Run JS fuzzer for 5 minutes + run: timeout 5m cargo fuzz run --strip-dead-code --features rome_all -s none rome_format_all -- -timeout=5 || [[ $? -eq 124 ]] + - name: Run JSON fuzzer for 5 minutes + run: timeout 5m cargo fuzz run --strip-dead-code -s none rome_format_json -- -timeout=5 || [[ $? -eq 124 ]] test-node-api: name: Test node.js API From 7b5bd6c012642755f1cc81d7ee817bed60e454cb Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Wed, 28 Jun 2023 17:07:25 +0200 Subject: [PATCH 3/4] rename task for clarity --- .github/workflows/pull_request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 55e05e3e92e..4218f389014 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -85,7 +85,7 @@ jobs: run: cargo test --doc fuzz-all: - name: Build and init fuzzers + name: Briefly run fuzzers runs-on: ubuntu-latest steps: From f4dfc8779b3dbf3dfd4e38d794f14c0f1b8ab615 Mon Sep 17 00:00:00 2001 From: Addison Crump Date: Wed, 28 Jun 2023 17:15:09 +0200 Subject: [PATCH 4/4] fixup: updates affected fuzzers missed --- fuzz/fuzz_targets/rome_common.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fuzz/fuzz_targets/rome_common.rs b/fuzz/fuzz_targets/rome_common.rs index f79a525f375..7d4abd2e434 100644 --- a/fuzz/fuzz_targets/rome_common.rs +++ b/fuzz/fuzz_targets/rome_common.rs @@ -10,7 +10,7 @@ use rome_formatter::format_node; use rome_js_analyze::analyze; use rome_js_formatter::context::JsFormatOptions; use rome_js_formatter::JsFormatLanguage; -use rome_js_parser::parse; +use rome_js_parser::{JsParserOptions, parse}; use rome_js_syntax::JsFileSource; use rome_json_formatter::context::JsonFormatOptions; use rome_json_formatter::JsonFormatLanguage; @@ -23,7 +23,7 @@ use std::sync::OnceLock; pub fn fuzz_js_parser_with_source_type(data: &[u8], source: JsFileSource) -> Corpus { let Ok(code1) = std::str::from_utf8(data) else { return Corpus::Reject; }; - let parse1 = parse(code1, source); + let parse1 = parse(code1, source, JsParserOptions::default()); if !parse1.has_errors() { let syntax1 = parse1.syntax(); let code2 = syntax1.to_string(); @@ -70,7 +70,7 @@ pub fn fuzz_js_formatter_with_source_type(data: &[u8], source: JsFileSource) -> let rule_filters = ANALYSIS_RULE_FILTERS .get_or_init(|| rules.as_enabled_rules().into_iter().collect::>()); - let parse1 = parse(code1, source); + let parse1 = parse(code1, source, JsParserOptions::default()); if !parse1.has_errors() { let language = JsFormatLanguage::new(JsFormatOptions::new(source)); let tree1 = parse1.tree(); @@ -97,7 +97,7 @@ pub fn fuzz_js_formatter_with_source_type(data: &[u8], source: JsFileSource) -> if let Ok(formatted1) = format_node(&syntax1, language.clone()) { if let Ok(printed1) = formatted1.print() { let code2 = printed1.as_code(); - let parse2 = parse(code2, source); + let parse2 = parse(code2, source, JsParserOptions::default()); assert!( !parse2.has_errors(), "formatter introduced errors:\n{}",