Skip to content

Commit

Permalink
Implement selection of analysis target via command line. (#121)
Browse files Browse the repository at this point in the history
## What Changed?

Adds a new command line and environment argument `--analyze` and
`PARALEGAL_ANALYZE` which select an analysis target, the same as the
inline `#[paralegal::analyze]` does.

The target is required to resolve to a local function item.

The command line argument may be specified multiple times. Both command
line and env variable admit multiple, comma-separated paths, e.g. a
format `std::vec::Vec::push,std::env::var`.

## Why Does It Need To?

This allow users to apply paralegal to their project without *any*
source-level modifications.

## Checklist

- [x] Above description has been filled out so that upon quash merge we
have a
  good record of what changed.
- [x] New functions, methods, types are documented. Old documentation is
updated
  if necessary
- [ ] Documentation in Notion has been updated
- [ ] Tests for new behaviors are provided
  - [ ] New test suites (if any) ave been added to the CI tests (in
`.github/workflows/rust.yml`) either as compiler test or integration
test.
*Or* justification for their omission from CI has been provided in this
PR
    description.
  • Loading branch information
JustusAdam authored Feb 12, 2024
1 parent c1b8054 commit 98a2d99
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 23 deletions.
46 changes: 44 additions & 2 deletions crates/paralegal-flow/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,32 @@

use anyhow::Error;
use clap::ValueEnum;
use std::ffi::{OsStr, OsString};
use std::str::FromStr;

use crate::utils::TinyBitSet;
use crate::{num_derive, num_traits::FromPrimitive};

#[derive(thiserror::Error, Debug)]
enum VarError {
#[error("env variable value is not unicode, approximate key and value are {}: {}", key.to_string_lossy(), value.to_string_lossy())]
NotUnicode { key: OsString, value: OsString },
}

/// A thin wrapper around `std::env::var` that returns `None` if the variable is
/// not present.
fn env_var_expect_unicode(k: impl AsRef<OsStr>) -> Result<Option<String>, VarError> {
let k_ref = k.as_ref();
match std::env::var(k_ref) {
Ok(v) => Ok(Some(v)),
Err(std::env::VarError::NotUnicode(u)) => Err(VarError::NotUnicode {
key: k_ref.to_owned(),
value: u,
}),
Err(std::env::VarError::NotPresent) => Ok(None),
}
}

impl TryFrom<ClapArgs> for Args {
type Error = Error;
fn try_from(value: ClapArgs) -> Result<Self, Self::Error> {
Expand All @@ -28,17 +49,27 @@ impl TryFrom<ClapArgs> for Args {
relaxed,
target,
abort_after_analysis,
anactrl,
mut anactrl,
modelctrl,
dump,
marker_control,
} = value;
let mut dump: DumpArgs = dump.into();
if let Ok(from_env) = std::env::var("PARALEGAL_DUMP") {
if let Some(from_env) = env_var_expect_unicode("PARALEGAL_DUMP")? {
let from_env =
DumpArgs::from_str(&from_env, false).map_err(|s| anyhow::anyhow!("{}", s))?;
dump.0 |= from_env.0;
}
anactrl.analyze = anactrl
.analyze
.iter()
.flat_map(|s| s.split(',').map(ToOwned::to_owned))
.collect();
if let Some(from_env) = env_var_expect_unicode("PARALEGAL_ANALYZE")? {
anactrl
.analyze
.extend(from_env.split(',').map(ToOwned::to_owned));
}
let build_config_file = std::path::Path::new("Paralegal.toml");
let build_config = if build_config_file.exists() {
toml::from_str(&std::fs::read_to_string(build_config_file)?)?
Expand Down Expand Up @@ -396,6 +427,12 @@ impl MarkerControl {
/// Arguments that control the flow analysis
#[derive(serde::Serialize, serde::Deserialize, clap::Args)]
pub struct AnalysisCtrl {
/// Target this function as analysis target. Command line version of
/// `#[paralegal::analyze]`). Must be a full rust path and resolve to a
/// function. May be specified multiple times and multiple, comma separated
/// paths may be supplied at the same time.
#[clap(long)]
analyze: Vec<String>,
/// Disables all recursive analysis (both paralegal_flow's inlining as well as
/// Flowistry's recursive analysis).
///
Expand Down Expand Up @@ -486,6 +523,11 @@ impl PruningStrategy {
}

impl AnalysisCtrl {
/// Externally (via command line) selected analysis targets
pub fn selected_targets(&self) -> &[String] {
&self.analyze
}

/// Are we recursing into (unmarked) called functions with the analysis?
pub fn use_recursive_analysis(&self) -> bool {
!self.no_cross_function_analysis
Expand Down
20 changes: 19 additions & 1 deletion crates/paralegal-flow/src/discover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use rustc_span::{symbol::Ident, Span, Symbol};

use anyhow::Result;

use self::resolve::expect_resolve_string_to_def_id;

/// Values of this type can be matched against Rust attributes
pub type AttrMatchT = Vec<Symbol>;

Expand Down Expand Up @@ -58,10 +60,26 @@ impl FnToAnalyze {

impl<'tcx> CollectingVisitor<'tcx> {
pub(crate) fn new(tcx: TyCtxt<'tcx>, opts: &'static crate::Args) -> Self {
let functions_to_analyze = opts
.anactrl()
.selected_targets()
.iter()
.filter_map(|path| {
let def_id = expect_resolve_string_to_def_id(tcx, path, opts.relaxed())?;
if !def_id.is_local() {
tcx.sess.span_err(tcx.def_span(def_id), "found an external function as analysis target. Analysis targets are required to be local.");
return None;
}
Some(FnToAnalyze {
def_id,
name: tcx.opt_item_ident(def_id).unwrap(),
})
})
.collect();
Self {
tcx,
opts,
functions_to_analyze: vec![],
functions_to_analyze,
marker_ctx: MarkerDatabase::init(tcx, opts),
}
}
Expand Down
26 changes: 6 additions & 20 deletions crates/paralegal-flow/src/marker_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use crate::{
desc::{Annotation, MarkerAnnotation},
mir, ty,
utils::{
AsFnAndArgs, FnResolution, GenericArgExt, IntoDefId, IntoHirId, MetaItemMatch, TyCtxtExt,
TyExt,
resolve::expect_resolve_string_to_def_id, AsFnAndArgs, FnResolution, GenericArgExt,
IntoDefId, IntoHirId, MetaItemMatch, TyCtxtExt, TyExt,
},
DefId, HashMap, LocalDefId, TyCtxt,
};
Expand Down Expand Up @@ -322,27 +322,13 @@ fn resolve_external_markers(opts: &Args, tcx: TyCtxt) -> ExternalMarkers {
}),
)
.unwrap();
use crate::utils::resolve::{def_path_res, Res};
let new_map: ExternalMarkers = from_toml
.iter()
.filter_map(|(path, marker)| {
let segment_vec = path.split("::").collect::<Vec<_>>();
let res = def_path_res(tcx, &segment_vec)
.map_err(|err| tcx.sess.err(format!("Could not resolve {path}: {err:?}")))
.ok()?;
let did = match res {
Res::Def(_, did) => Some(did),
other => {
let msg = format!("{path} did not resolve to an item ({other:?})");
if opts.relaxed() {
tcx.sess.warn(msg);
} else {
tcx.sess.err(msg);
}
None
}
}?;
Some((did, marker.clone()))
Some((
expect_resolve_string_to_def_id(tcx, path, opts.relaxed())?,
marker.clone(),
))
})
.collect();
new_map
Expand Down
23 changes: 23 additions & 0 deletions crates/paralegal-flow/src/utils/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,29 @@ impl Res {
}
}

/// A small helper wrapper around [`def_path_res`] that represents a common way
/// that `def_path_res` is used. In the case of errors they are reported to the
/// user and `None` is returned so the caller has the option of making progress
/// before exiting.
pub fn expect_resolve_string_to_def_id(tcx: TyCtxt, path: &str, relaxed: bool) -> Option<DefId> {
let segment_vec = path.split("::").collect::<Vec<_>>();
let res = def_path_res(tcx, &segment_vec)
.map_err(|e| tcx.sess.err(format!("Could not resolve {path}: {e:?}")))
.ok()?;
match res {
Res::Def(_, did) => Some(did),
other => {
let msg = format!("expected {path} to resolve to an item, got {other:?}");
if relaxed {
tcx.sess.warn(msg);
} else {
tcx.sess.err(msg);
};
None
}
}
}

/// Lifted from `clippy_utils`
pub fn def_path_res<'a>(tcx: TyCtxt, path: &[&'a str]) -> Result<Res, ResolutionError<'a>> {
fn item_child_by_name<'a>(
Expand Down

0 comments on commit 98a2d99

Please sign in to comment.