Skip to content

Commit

Permalink
do not use std::panic::catch_unwind
Browse files Browse the repository at this point in the history
Summary:
Recovering from ad-hoc panics is an anti-pattern that can lead to
undefined behavior. We need to be careful removing catch_unwinds()
from production code since it could be papering over real bugs,
but it should be safe to start with `hh_compile`.

While I'm here, remove some dead code.

Reviewed By: aorenste

Differential Revision: D34999905

fbshipit-source-id: 28b7e617db201e25db9af65103855dce91bda5d9
  • Loading branch information
edwinsmith authored and facebook-github-bot committed Mar 21, 2022
1 parent 708dcc3 commit 9217447
Showing 1 changed file with 1 addition and 29 deletions.
30 changes: 1 addition & 29 deletions hphp/hack/src/hh_compile/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ fn process_single_file_impl(
Ok((output, profile))
}

fn process_single_file_with_retry(
pub(crate) fn process_single_file(
opts: &SingleFileOpts,
filepath: PathBuf,
content: Vec<u8>,
Expand All @@ -234,20 +234,6 @@ fn process_single_file_with_retry(
})?
}

pub(crate) fn process_single_file(
opts: &SingleFileOpts,
filepath: PathBuf,
content: Vec<u8>,
) -> Result<(Vec<u8>, Profile)> {
match std::panic::catch_unwind(|| process_single_file_with_retry(opts, filepath, content)) {
Ok(r) => r,
Err(panic) => match panic.downcast::<String>() {
Ok(msg) => Err(anyhow!("panic: {}", msg)),
Err(_) => Err(anyhow!("panic: unknown")),
},
}
}

fn assert_regular_file(filepath: impl AsRef<Path>) {
let filepath = filepath.as_ref();
if !filepath.is_file() {
Expand Down Expand Up @@ -276,20 +262,6 @@ impl Config {
ret
}

#[allow(dead_code)] // will be used if --daemon (by HHVM)
fn with_merged<T>(
&mut self,
json: String,
cli_args: &[String],
f: impl FnOnce(&Options) -> T,
) -> T {
self.jsons.push(json);
let hhbc_options = self.to_options(cli_args);
let ret = f(&hhbc_options);
self.jsons.pop();
ret
}

fn to_options(&self, cli_args: &[String]) -> Options {
Options::from_configs(&self.jsons, cli_args).unwrap()
}
Expand Down

0 comments on commit 9217447

Please sign in to comment.