From 56236fb58e0a6209955e1ef26dc7c56478f9982c Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 23 Feb 2022 14:34:32 -0800 Subject: [PATCH] ISLE: fix compile fuzz target, and fix a simple error-reporting bug. (#3845) It seems our `compile` fuzz target for ISLE has not been regularly tested, as it was never updated for the `isle` -> `cranelift_isle` crate renaming. This PR fixes it to compile again. This also includes a simple fix in the typechecking: when verifying that a term decl is valid, we might insert a term ID into the name->ID map before fully checking that all of the types exist, and then skipping (for error recovery purposes) the actual push onto the term-signature vector if one of the types does have an error. This phantom TID can later cause a panic. The fix is to avoid adding to the map until we have fully verified the term decl. --- .github/workflows/main.yml | 2 ++ cranelift/isle/fuzz/fuzz_targets/compile.rs | 6 +++--- cranelift/isle/isle/src/sema.rs | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c636a0856a..f6bd953714 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -185,6 +185,8 @@ jobs: - run: cargo fetch working-directory: ./fuzz - run: cargo fuzz build --dev + # Check that the ISLE fuzz targets build too. + - run: cargo fuzz build --dev --fuzz-dir ./cranelift/isle/fuzz rebuild_isle: name: Rebuild ISLE diff --git a/cranelift/isle/fuzz/fuzz_targets/compile.rs b/cranelift/isle/fuzz/fuzz_targets/compile.rs index 8526c29cce..2a21361b52 100644 --- a/cranelift/isle/fuzz/fuzz_targets/compile.rs +++ b/cranelift/isle/fuzz/fuzz_targets/compile.rs @@ -5,21 +5,21 @@ use libfuzzer_sys::fuzz_target; fuzz_target!(|s: &str| { let _ = env_logger::try_init(); - let lexer = isle::lexer::Lexer::from_str(s, "fuzz-input.isle"); + let lexer = cranelift_isle::lexer::Lexer::from_str(s, "fuzz-input.isle"); log::debug!("lexer = {:?}", lexer); let lexer = match lexer { Ok(l) => l, Err(_) => return, }; - let defs = isle::parser::parse(lexer); + let defs = cranelift_isle::parser::parse(lexer); log::debug!("defs = {:?}", defs); let defs = match defs { Ok(d) => d, Err(_) => return, }; - let code = isle::compile::compile(&defs); + let code = cranelift_isle::compile::compile(&defs); log::debug!("code = {:?}", code); let code = match code { Ok(c) => c, diff --git a/cranelift/isle/isle/src/sema.rs b/cranelift/isle/isle/src/sema.rs index 3dff008b5b..e36cd940ab 100644 --- a/cranelift/isle/isle/src/sema.rs +++ b/cranelift/isle/isle/src/sema.rs @@ -805,7 +805,6 @@ impl TermEnv { for def in &defs.defs { match def { &ast::Def::Decl(ref decl) => { - let tid = TermId(self.terms.len()); let name = tyenv.intern_mut(&decl.term); if let Some(tid) = self.term_map.get(&name) { tyenv.report_error( @@ -817,7 +816,6 @@ impl TermEnv { format!("Duplicate decl for '{}'", decl.term.0), ); } - self.term_map.insert(name, tid); let arg_tys = decl .arg_tys @@ -850,6 +848,8 @@ impl TermEnv { } }; + let tid = TermId(self.terms.len()); + self.term_map.insert(name, tid); self.terms.push(Term { id: tid, decl_pos: decl.pos,