Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ast: Avoid aborts on fatal errors thrown from mutable AST visitor #91519

Merged
merged 1 commit into from
Dec 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
128 changes: 116 additions & 12 deletions compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ use crate::tokenstream::*;

use rustc_data_structures::map_in_place::MapInPlace;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::Ident;
use rustc_span::Span;

use smallvec::{smallvec, Array, SmallVec};
use std::ops::DerefMut;
use std::{panic, process, ptr};
use std::{panic, ptr};

pub trait ExpectOne<A: Array> {
fn expect_one(self, err: &'static str) -> A::Item;
Expand Down Expand Up @@ -283,23 +284,21 @@ pub trait MutVisitor: Sized {

/// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful
/// when using a `flat_map_*` or `filter_map_*` method within a `visit_`
/// method. Abort the program if the closure panics.
///
/// FIXME: Abort on panic means that any fatal error inside `visit_clobber` will abort the compiler.
/// Instead of aborting on catching a panic we need to reset the visited node to some valid but
/// possibly meaningless value and rethrow the panic.
/// method.
//
// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
pub fn visit_clobber<T, F>(t: &mut T, f: F)
where
F: FnOnce(T) -> T,
{
pub fn visit_clobber<T: DummyAstNode>(t: &mut T, f: impl FnOnce(T) -> T) {
unsafe {
// Safe because `t` is used in a read-only fashion by `read()` before
// being overwritten by `write()`.
let old_t = ptr::read(t);
let new_t = panic::catch_unwind(panic::AssertUnwindSafe(|| f(old_t)))
.unwrap_or_else(|_| process::abort());
let new_t =
panic::catch_unwind(panic::AssertUnwindSafe(|| f(old_t))).unwrap_or_else(|err| {
// Set `t` to some valid but possible meaningless value,
// and pass the fatal error further.
ptr::write(t, T::dummy());
panic::resume_unwind(err);
});
ptr::write(t, new_t);
}
}
Expand Down Expand Up @@ -1454,3 +1453,108 @@ pub fn noop_visit_vis<T: MutVisitor>(visibility: &mut Visibility, vis: &mut T) {
}
vis.visit_span(&mut visibility.span);
}

/// Some value for the AST node that is valid but possibly meaningless.
pub trait DummyAstNode {
fn dummy() -> Self;
}

impl<T> DummyAstNode for Option<T> {
fn dummy() -> Self {
Default::default()
}
}

impl<T: DummyAstNode + 'static> DummyAstNode for P<T> {
fn dummy() -> Self {
P(DummyAstNode::dummy())
}
}

impl<T> DummyAstNode for ThinVec<T> {
fn dummy() -> Self {
Default::default()
}
}

impl DummyAstNode for Item {
fn dummy() -> Self {
Item {
attrs: Default::default(),
id: DUMMY_NODE_ID,
span: Default::default(),
vis: Visibility {
kind: VisibilityKind::Public,
span: Default::default(),
tokens: Default::default(),
},
ident: Ident::empty(),
kind: ItemKind::ExternCrate(None),
tokens: Default::default(),
}
}
}

impl DummyAstNode for Expr {
fn dummy() -> Self {
Expr {
id: DUMMY_NODE_ID,
kind: ExprKind::Err,
span: Default::default(),
attrs: Default::default(),
tokens: Default::default(),
}
}
}

impl DummyAstNode for Ty {
fn dummy() -> Self {
Ty {
id: DUMMY_NODE_ID,
kind: TyKind::Err,
span: Default::default(),
tokens: Default::default(),
}
}
}

impl DummyAstNode for Pat {
fn dummy() -> Self {
Pat {
id: DUMMY_NODE_ID,
kind: PatKind::Wild,
span: Default::default(),
tokens: Default::default(),
}
}
}

impl DummyAstNode for Stmt {
fn dummy() -> Self {
Stmt { id: DUMMY_NODE_ID, kind: StmtKind::Empty, span: Default::default() }
}
}

impl DummyAstNode for Block {
fn dummy() -> Self {
Block {
stmts: Default::default(),
id: DUMMY_NODE_ID,
rules: BlockCheckMode::Default,
span: Default::default(),
tokens: Default::default(),
could_be_bare_literal: Default::default(),
}
}
}

impl DummyAstNode for Crate {
fn dummy() -> Self {
Crate {
attrs: Default::default(),
items: Default::default(),
span: Default::default(),
is_placeholder: Default::default(),
}
}
}
20 changes: 11 additions & 9 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,13 +1160,18 @@ macro_rules! assign_id {

impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
fn visit_crate(&mut self, krate: &mut ast::Crate) {
let span = krate.span;
let empty_crate =
|| ast::Crate { attrs: Vec::new(), items: Vec::new(), span, is_placeholder: None };
let mut fold_crate = |krate: ast::Crate| {
visit_clobber(krate, |krate| {
let span = krate.span;
let mut krate = match self.configure(krate) {
Some(krate) => krate,
None => return empty_crate(),
None => {
return ast::Crate {
attrs: Vec::new(),
items: Vec::new(),
span,
is_placeholder: None,
};
}
};

if let Some(attr) = self.take_first_attr(&mut krate) {
Expand All @@ -1177,10 +1182,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {

noop_visit_crate(&mut krate, self);
krate
};

// Cannot use `visit_clobber` here, see the FIXME on it.
*krate = fold_crate(mem::replace(krate, empty_crate()));
})
}

fn visit_expr(&mut self, expr: &mut P<ast::Expr>) {
Expand Down