Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Update fuzzers for 1.70 and integrate with CI #4570

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Update fuzzers for 1.70 and integrate with CI #4570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ea7125
use oncelock/threadlocal where possible
addisoncrump Jun 14, 2023
b3b688d
fuzz in CI
addisoncrump Jun 28, 2023
7b5bd6c
rename task for clarity
addisoncrump Jun 28, 2023
f4dfc87
fixup: updates affected fuzzers missed
addisoncrump Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/pull_request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be reasonable to put these in a github action matrix to run them in parallel jobs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially so, but we would need to transfer the build artifacts from the common init step. OR split the init step. Both are possible.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ig a similar thing to do would be to not fail the job if the first fuzz test run fails so the second can also run, with https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast

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
Expand Down
117 changes: 61 additions & 56 deletions fuzz/fuzz_targets/rome_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,20 @@ 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;
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; };

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();
Expand All @@ -32,9 +33,13 @@ pub fn fuzz_js_parser_with_source_type(data: &[u8], source: JsFileSource) -> Cor
Corpus::Keep
}

static mut ANALYSIS_RULES: Option<Rules> = None;
static mut ANALYSIS_RULE_FILTERS: Option<Vec<RuleFilter>> = None;
static mut ANALYSIS_OPTIONS: Option<AnalyzerOptions> = None;
static ANALYSIS_RULES: OnceLock<Rules> = OnceLock::new();
static ANALYSIS_RULE_FILTERS: OnceLock<Vec<RuleFilter>> = OnceLock::new();

// have to use thread local because AnalyzerOptions contains a Box<dyn Any>, which isn't thread-safe
thread_local! {
static ANALYSIS_OPTIONS: AnalyzerOptions = AnalyzerOptions::default();
}

struct DiagnosticDescriptionExtractor<'a, D> {
diagnostic: &'a D,
Expand All @@ -58,48 +63,41 @@ 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::<Vec<_>>();
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::<Vec<_>>());

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();
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() {
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{}",
Expand All @@ -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{}",
Expand Down
Loading