diff --git a/Cargo.lock b/Cargo.lock index f408e83812..984344d7ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -283,6 +283,17 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "colored" +version = "1.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a5f741c91823341bebf717d4c71bda820630ce065443b58bd1b7451af008355" +dependencies = [ + "is-terminal", + "lazy_static", + "winapi", +] + [[package]] name = "colored" version = "2.0.4" @@ -822,6 +833,7 @@ version = "0.1.0" dependencies = [ "anyhow", "bitvec", + "colored 1.9.4", "indexical", "itertools 0.12.0", "lazy_static", @@ -831,6 +843,7 @@ dependencies = [ "petgraph", "serde_json", "simple_logger", + "strum", ] [[package]] @@ -1108,7 +1121,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48047e77b528151aaf841a10a9025f9459da80ba820e425ff7eb005708a76dc7" dependencies = [ "atty", - "colored", + "colored 2.0.4", "log", "time", "winapi", diff --git a/Cargo.toml b/Cargo.toml index 5ebb9522ff..f852567703 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,12 +1,18 @@ [workspace] members = ["crates/*"] -exclude = ["props", "crates/paralegal-flow/tests", "crates/paralegal-policy/tests", "crates/paralegal-explore"] +exclude = [ + "props", + "crates/paralegal-flow/tests", + "crates/paralegal-policy/tests", + "crates/paralegal-explore", +] resolver = "2" [workspace.dependencies] indexical = "0.3.1" serde = "1.0.188" petgraph = { version = "0.6", features = ["serde-1"] } +strum = { version = "0.25", features = ["derive"] } [profile.release] debug = true diff --git a/crates/paralegal-flow/Cargo.toml b/crates/paralegal-flow/Cargo.toml index 4249ba6d85..e5975be499 100644 --- a/crates/paralegal-flow/Cargo.toml +++ b/crates/paralegal-flow/Cargo.toml @@ -40,7 +40,7 @@ num-derive = "0.4" num-traits = "0.2" petgraph = { workspace = true } humantime = "2" -strum = { version = "0.25", features = ["derive"] } +strum = { workspace = true } #dot = "0.1" diff --git a/crates/paralegal-flow/src/ana/mod.rs b/crates/paralegal-flow/src/ana/mod.rs index 45c9c2c2be..e8cde14c52 100644 --- a/crates/paralegal-flow/src/ana/mod.rs +++ b/crates/paralegal-flow/src/ana/mod.rs @@ -27,6 +27,7 @@ use flowistry::pdg::{ }; use itertools::Itertools; use petgraph::visit::{GraphBase, IntoNodeReferences, NodeIndexable, NodeRef}; +use rustc_span::{FileNameDisplayPreference, Span as RustSpan}; mod inline_judge; @@ -119,6 +120,7 @@ impl<'tcx> SPDGGenerator<'tcx> { .iter() .map(|id| (*id, def_info_for_item(*id, tcx))) .collect(); + let type_info = self.collect_type_info(); type_info_sanity_check(&controllers, &type_info); ProgramDescription { @@ -147,25 +149,49 @@ impl<'tcx> SPDGGenerator<'tcx> { all_instructions .into_iter() .map(|i| { - let body = self.tcx.body_for_def_id(i.function).unwrap(); - let info = match i.location { - RichLocation::End => InstructionInfo::Return, - RichLocation::Start => InstructionInfo::Start, - RichLocation::Location(loc) => match body.body.stmt_at(loc) { - crate::Either::Right(term) => { - if let Ok((id, ..)) = term.as_fn_and_args(self.tcx) { - InstructionInfo::FunctionCall(FunctionCallInfo { - id, - is_inlined: id.is_local(), - }) - } else { - InstructionInfo::Terminator + let body = &self.tcx.body_for_def_id(i.function).unwrap().body; + + let kind = match i.location { + RichLocation::End => InstructionKind::Return, + RichLocation::Start => InstructionKind::Start, + RichLocation::Location(loc) => { + let kind = match body.stmt_at(loc) { + crate::Either::Right(term) => { + if let Ok((id, ..)) = term.as_fn_and_args(self.tcx) { + InstructionKind::FunctionCall(FunctionCallInfo { + id, + is_inlined: id.is_local(), + }) + } else { + InstructionKind::Terminator + } } - } - _ => InstructionInfo::Statement, - }, + crate::Either::Left(_) => InstructionKind::Statement, + }; + + kind + } }; - (i, info) + let rust_span = match i.location { + RichLocation::Location(loc) => { + let expanded_span = match body.stmt_at(loc) { + crate::Either::Right(term) => term.source_info.span, + crate::Either::Left(stmt) => stmt.source_info.span, + }; + self.tcx + .sess + .source_map() + .stmt_span(expanded_span, body.span) + } + RichLocation::Start | RichLocation::End => self.tcx.def_span(i.function), + }; + ( + i, + InstructionInfo { + kind, + span: src_loc_for_span(rust_span, self.tcx), + }, + ) }) .collect() } @@ -202,6 +228,38 @@ impl<'tcx> SPDGGenerator<'tcx> { } } +fn src_loc_for_span(span: RustSpan, tcx: TyCtxt) -> Span { + let (source_file, start_line, start_col, end_line, end_col) = + tcx.sess.source_map().span_to_location_info(span); + let file_path = source_file + .expect("could not find source file") + .name + .display(FileNameDisplayPreference::Local) + .to_string(); + let abs_file_path = if !file_path.starts_with('/') { + std::env::current_dir() + .expect("failed to obtain current working directory") + .join(&file_path) + } else { + std::path::PathBuf::from(&file_path) + }; + let src_info = SourceFileInfo { + file_path, + abs_file_path, + }; + Span { + source_file: src_info.intern(), + start: SpanCoord { + line: start_line as u32, + col: start_col as u32, + }, + end: SpanCoord { + line: end_line as u32, + col: end_col as u32, + }, + } +} + fn default_index() -> ::NodeId { ::NodeId::end() } @@ -545,16 +603,22 @@ impl<'a, 'tcx, C: Extend> GraphConverter<'tcx, 'a, C> { use petgraph::prelude::*; let g_ref = self.dep_graph.clone(); let input = &g_ref.graph; + let tcx = self.tcx(); let mut markers: HashMap> = HashMap::new(); for (i, weight) in input.node_references() { let (kind, is_external_call_source, node_markers) = self.determine_node_kind(weight); + let at = weight.at.leaf(); + let body = &tcx.body_for_def_id(at.function).unwrap().body; + + let node_span = body.local_decls[weight.place.local].source_info.span; let new_idx = self.register_node( i, NodeInfo { at: weight.at, description: format!("{:?}", weight.place), kind, + span: src_loc_for_span(node_span, tcx), }, ); @@ -729,7 +793,12 @@ fn def_info_for_item(id: DefId, tcx: TyCtxt) -> DefInfo { } })) .collect(); - DefInfo { name, path, kind } + DefInfo { + name, + path, + kind, + src_info: src_loc_for_span(tcx.def_span(id), tcx), + } } /// A higher order function that increases the logging level if the `target` diff --git a/crates/paralegal-flow/src/test_utils.rs b/crates/paralegal-flow/src/test_utils.rs index 96c558889a..9c87583eb0 100644 --- a/crates/paralegal-flow/src/test_utils.rs +++ b/crates/paralegal-flow/src/test_utils.rs @@ -350,6 +350,7 @@ impl<'g> CtrlRef<'g> { .chain(self.ctrl.graph.node_weights().map(|info| info.at)) .filter(|m| { instruction_info[&m.leaf()] + .kind .as_function_call() .map_or(false, |i| i.id == fun.ident) }) diff --git a/crates/paralegal-policy/Cargo.toml b/crates/paralegal-policy/Cargo.toml index 5279a39555..f150c6b738 100644 --- a/crates/paralegal-policy/Cargo.toml +++ b/crates/paralegal-policy/Cargo.toml @@ -16,6 +16,8 @@ simple_logger = "2" lazy_static = "1" bitvec = "1" petgraph = { workspace = true } +colored = "1" +strum = { workspace = true } [dev-dependencies] paralegal-flow = { path = "../paralegal-flow", features = ["test"] } diff --git a/crates/paralegal-policy/src/context.rs b/crates/paralegal-policy/src/context.rs index 9395eb29f1..bb366a5eaf 100644 --- a/crates/paralegal-policy/src/context.rs +++ b/crates/paralegal-policy/src/context.rs @@ -1,24 +1,24 @@ -use std::{io::Write, process::exit, sync::Arc}; +use std::{collections::HashSet, io::Write, process::exit, sync::Arc}; pub use paralegal_spdg::rustc_portable::{DefId, LocalDefId}; use paralegal_spdg::traverse::{generic_flows_to, EdgeSelection}; use paralegal_spdg::{ CallString, DisplayNode, Endpoint, GlobalNode, HashMap, Identifier, InstructionInfo, IntoIterGlobalNodes, Node as SPDGNode, NodeCluster, NodeInfo, ProgramDescription, SPDGImpl, - TypeId, SPDG, + Span, TypeId, SPDG, }; use anyhow::{anyhow, bail, ensure, Result}; use itertools::{Either, Itertools}; use petgraph::prelude::Bfs; -use petgraph::visit::{Control, DfsEvent, EdgeFiltered, EdgeRef, Walker}; +use petgraph::visit::{Control, DfsEvent, EdgeFiltered, EdgeRef, GraphBase, NodeIndexable, Walker}; use petgraph::Incoming; use super::flows_to::CtrlFlowsTo; use crate::Diagnostics; use crate::{ - assert_error, assert_warning, + assert_warning, diagnostics::{CombinatorContext, DiagnosticsRecorder, HasDiagnosticsBase}, }; @@ -84,9 +84,11 @@ fn bfs_iter< /// /// To communicate the results of your policies with the user you can emit /// diagnostic messages. To communicate a policy failure use -/// [`error`](crate::Diagnostics::error) or the [`assert_error`] macro. To +/// [`error`](crate::Diagnostics::error) or the [`crate::assert_error`] macro. To /// communicate suspicious circumstances that are not outright cause for failure -/// use [`warning`](crate::Diagnostics::error) or [`assert_warning`]. +/// use [`warning`](crate::Diagnostics::warning) or [`assert_warning`]. For all +/// types of errors, including those with span information for a particular +/// node, see the [`crate::Diagnostics`] trait. /// /// Note that these methods just queue the diagnostics messages. To emit them /// (and potentially terminate the program if the policy does not hold) use @@ -483,26 +485,36 @@ impl Context { mut is_checkpoint: impl FnMut(GlobalNode) -> bool, mut is_terminal: impl FnMut(GlobalNode) -> bool, ) -> Result { - let mut num_reached = 0; - let mut num_checkpointed = 0; + let mut reached = HashMap::new(); + let mut checkpointed = HashSet::new(); let start_map = starting_points .into_iter() .map(|i| (i.controller_id(), i.local_node())) .into_group_map(); - let started_with = start_map.values().map(Vec::len).sum(); - for (ctrl_id, starts) in start_map { + for (ctrl_id, starts) in &start_map { let spdg = &self.desc.controllers[&ctrl_id]; let g = &spdg.graph; - petgraph::visit::depth_first_search(g, starts, |event| match event { + let mut origin_map = vec![::NodeId::end(); g.node_bound()]; + for s in starts { + origin_map[s.index()] = *s; + } + petgraph::visit::depth_first_search(g, starts.iter().copied(), |event| match event { + DfsEvent::TreeEdge(from, to) => { + origin_map[to.index()] = origin_map[from.index()]; + Control::<()>::Continue + } DfsEvent::Discover(inner, _) => { - let as_node = GlobalNode::from_local_node(ctrl_id, inner); + let as_node = GlobalNode::from_local_node(*ctrl_id, inner); if is_checkpoint(as_node) { - num_checkpointed += 1; + checkpointed.insert(as_node); Control::<()>::Prune } else if is_terminal(as_node) { - num_reached += 1; + reached.insert( + as_node, + GlobalNode::from_local_node(*ctrl_id, origin_map[inner.index()]), + ); Control::Prune } else { Control::Continue @@ -513,9 +525,9 @@ impl Context { } Ok(AlwaysHappensBefore { - num_reached, - num_checkpointed, - started_with, + reached: reached.into_iter().collect(), + checkpointed: checkpointed.into_iter().collect(), + started_with: start_map.values().map(Vec::len).sum(), }) } @@ -629,6 +641,11 @@ impl Context { } NodeCluster::new(src.controller_id(), start) } + + /// Get the span of a node + pub fn get_location(&self, node: GlobalNode) -> &Span { + &self.node_info(node).span + } } /// Provide display trait for DefId in a Context. @@ -670,11 +687,12 @@ impl<'a> std::fmt::Display for DisplayDef<'a> { /// [`Self::is_vacuous`]. Otherwise the information in this struct and its /// printed representations should be considered unstable and /// for-human-eyes-only. +#[must_use = "call `report` or similar evaluations function to ensure the property is checked"] pub struct AlwaysHappensBefore { /// How many paths terminated at the end? - num_reached: i32, + reached: Vec<(GlobalNode, GlobalNode)>, /// How many paths lead to the checkpoints? - num_checkpointed: i32, + checkpointed: Vec, /// How large was the set of initial nodes this traversal started with. started_with: usize, } @@ -683,16 +701,14 @@ impl std::fmt::Display for AlwaysHappensBefore { /// Format the results of this combinator, using the `def_info` to print /// readable names instead of ids fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let Self { - num_reached, - num_checkpointed, - started_with, - } = self; write!( f, - "{num_reached} paths reached the terminal, \ - {num_checkpointed} paths reached the checkpoints, \ - started with {started_with} nodes" + "{} paths reached the terminal, \ + {} paths reached the checkpoints, \ + started with {} nodes", + self.reached.len(), + self.checkpointed.len(), + self.started_with, ) } } @@ -710,12 +726,18 @@ impl AlwaysHappensBefore { let ctx = CombinatorContext::new(*ALWAYS_HAPPENS_BEFORE_NAME, ctx); assert_warning!(ctx, self.started_with != 0, "Started with 0 nodes."); assert_warning!(ctx, !self.is_vacuous(), "Is vacuously true."); - assert_error!(ctx, self.holds(), format!("Violation detected: {}", self)); + if !self.holds() { + for &(reached, from) in &self.reached { + let mut err = ctx.struct_node_error(reached, "Reached this terminal"); + err.with_node_note(from, "Started from this node"); + err.emit(); + } + } } /// Returns `true` if the property that created these statistics holds. pub fn holds(&self) -> bool { - self.num_reached == 0 + self.reached.is_empty() } /// Fails if [`Self::holds`] is false. @@ -723,7 +745,7 @@ impl AlwaysHappensBefore { ensure!( self.holds(), "AlwaysHappensBefore failed: found {} violating paths", - self.num_reached + self.reached.len() ); Ok(()) } @@ -732,7 +754,7 @@ impl AlwaysHappensBefore { /// or no path from them can reach the terminal or the checkpoints (the /// graphs are disjoined). pub fn is_vacuous(&self) -> bool { - self.num_checkpointed + self.num_reached == 0 + self.checkpointed.is_empty() && self.reached.is_empty() } } @@ -741,8 +763,6 @@ fn overlaps( one: impl IntoIterator, other: impl IntoIterator, ) -> bool { - use paralegal_spdg::HashSet; - let target = one.into_iter().collect::>(); other.into_iter().any(|n| target.contains(&n)) } diff --git a/crates/paralegal-policy/src/diagnostics.rs b/crates/paralegal-policy/src/diagnostics.rs index 380df871cc..35aba31b92 100644 --- a/crates/paralegal-policy/src/diagnostics.rs +++ b/crates/paralegal-policy/src/diagnostics.rs @@ -78,10 +78,11 @@ #![allow(clippy::arc_with_non_send_sync)] +use colored::*; use std::rc::Rc; use std::{io::Write, sync::Arc}; -use paralegal_spdg::{Identifier, SPDG}; +use paralegal_spdg::{GlobalNode, Identifier, Span, SpanCoord, SPDG}; use crate::{Context, ControllerId}; @@ -126,28 +127,299 @@ macro_rules! assert_warning { } /// Severity of a recorded diagnostic message -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, strum::AsRefStr)] +#[strum(serialize_all = "snake_case")] pub enum Severity { /// This indicates that the policy failed. - Fail, + Error, /// This could indicate that the policy does not operate as intended. Warning, + /// Additional information for a diagnostic + Note, + /// Some helpful hint + Help, } impl Severity { fn must_abort(self) -> bool { - matches!(self, Severity::Fail) + matches!(self, Severity::Error) + } + + fn color(self) -> Color { + match self { + Severity::Error => Color::Red, + Severity::Warning => Color::Yellow, + Severity::Note => Color::Blue, + Severity::Help => Color::Green, + } } } /// Context provided to [`HasDiagnosticsBase::record`]. -pub type DiagnosticContextStack = Vec; +type DiagnosticContextStack = Vec; +/// Representation of a diagnostic message. You should not interact with this +/// type directly but use the methods on [`Diagnostics`] or +/// [`DiagnosticBuilder`] to create these. #[derive(Debug)] -struct Diagnostic { +pub struct Diagnostic { + context: DiagnosticContextStack, + main: DiagnosticPart, + children: Vec, +} + +impl Diagnostic { + fn write(&self, w: &mut impl std::fmt::Write) -> std::fmt::Result { + for ctx in self.context.iter().rev() { + write!(w, "{ctx} ")?; + } + self.main.write(w)?; + for c in &self.children { + c.write(w)?; + } + Ok(()) + } +} + +#[derive(Debug)] +struct DiagnosticPart { message: String, severity: Severity, - context: DiagnosticContextStack, + span: Option, +} + +#[derive(Clone, Debug)] +struct SubSpan { + start: SpanCoord, + end: SpanCoord, +} + +#[derive(Clone, Debug)] +/// A span with only a portion highlighted. +pub struct HighlightedSpan { + span: Span, + highlight: Option, +} + +impl HighlightedSpan { + /// Create a new span with a highlighted section + pub fn new(span: Span, start: SpanCoord, end: SpanCoord) -> Self { + assert!(start >= span.start); + assert!(end <= span.end); + assert!(start <= end); + HighlightedSpan { + span, + highlight: Some(SubSpan { start, end }), + } + } +} + +impl From for HighlightedSpan { + fn from(value: Span) -> Self { + Self { + span: value, + highlight: None, + } + } +} + +impl DiagnosticPart { + fn write(&self, s: &mut impl std::fmt::Write) -> std::fmt::Result { + let severity = self.severity; + let coloring = severity.color(); + + use std::io::BufRead; + + writeln!(s, "{}: {}", severity.as_ref().color(coloring), self.message)?; + if let Some(src_loc) = &self.span { + let start_line = src_loc.span.start.line as usize; + let start_col = src_loc.span.start.col as usize; + let end_line = src_loc.span.end.line as usize; + let end_col = src_loc.span.end.col as usize; + let (hl_start_line, hl_start_col, hl_end_line, hl_end_col) = + if let Some(hl) = &src_loc.highlight { + ( + hl.start.line as usize, + hl.start.col as usize, + hl.end.line as usize, + hl.end.col as usize, + ) + } else { + (start_line, start_col, end_line, end_col) + }; + let max_line_len = + std::cmp::max(start_line.to_string().len(), end_line.to_string().len()); + let tab: String = " ".repeat(max_line_len); + writeln!( + s, + "{tab}{} {}:{}:{}", + "-->".blue(), + src_loc.span.source_file.file_path, + start_line, + start_col, + )?; + writeln!(s, "{tab} {}", "|".blue())?; + let lines = std::io::BufReader::new( + std::fs::File::open(&src_loc.span.source_file.abs_file_path).unwrap(), + ) + .lines() + .skip(start_line - 1) + .take(end_line - start_line + 1) + .enumerate(); + for (i, line) in lines { + let line_content: String = line.unwrap(); + let line_num = start_line + i; + + writeln!( + s, + "{:= hl_start_line && line_num <= hl_end_line { + let end: usize = if line_num == hl_end_line { + line_length_while(&line_content[0..hl_end_col - 1], |_| true) + } else { + line_length_while(&line_content, |_| true) + }; + let start: usize = if line_num == hl_start_line { + line_length_while(&line_content[0..hl_start_col - 1], |_| true) + } else { + line_length_while(&line_content, char::is_whitespace) + }; + writeln!( + s, + "{tab} {} {}{}", + "|".blue(), + " ".repeat(start), + "^".repeat(end - start).color(coloring) + )?; + } + } + writeln!(s, "{tab} {}", "|".blue())?; + } + + Ok(()) + } +} + +/// Facility to create structured diagnostics including spans and multi-part +/// messages. New builders are created with methods on [`Diagnostics`]. +/// `struct_` creates simple main diagnostics with only a message, +/// `struct_node_` creates a main diagnostic with a message and the +/// span of a graph node and `struct_span_` creates a main diagnostic +/// with a message and a custom source code span. +/// +/// The builder allows chaining additional sub diagnostics to the main +/// diagnostic. Analogous to the initializers the `with_` family of +/// functions adds simple messages, `with_node_` adds messages with +/// spans from a node and `with_span_` adds messages with custom +/// spans. +/// +/// Make sure to call [`Self::emit`] after construction, otherwise the +/// diagnostic is not shown. +#[derive(Debug)] +#[must_use = "you must call `emit`, otherwise the message is not shown"] +pub struct DiagnosticBuilder<'a, A: ?Sized> { + diagnostic: Diagnostic, + base: &'a A, +} + +impl<'a, A: ?Sized> DiagnosticBuilder<'a, A> { + fn init( + message: String, + severity: Severity, + span: Option>, + base: &'a A, + ) -> Self { + DiagnosticBuilder { + diagnostic: Diagnostic { + context: vec![], + main: DiagnosticPart { + message, + severity, + span: span.map(Into::into), + }, + children: vec![], + }, + base, + } + } + + fn with_child( + &mut self, + message: impl Into, + severity: Severity, + span: Option>, + ) -> &mut Self { + self.diagnostic.children.push(DiagnosticPart { + message: message.into(), + severity, + span: span.map(Into::into), + }); + self + } +} + +impl<'a, A: HasDiagnosticsBase + ?Sized> DiagnosticBuilder<'a, A> { + /// Queue the diagnostic for display to the user. + pub fn emit(self) { + self.base.record(self.diagnostic) + } + + /// Append a help message to the diagnostic. + pub fn with_help(&mut self, message: impl Into) -> &mut Self { + self.with_child(message, Severity::Help, Option::::None) + } + + /// Append a help message with a source code span to the diagnostic. + pub fn with_span_help(&mut self, span: Span, message: impl Into) -> &mut Self { + self.with_child(message, Severity::Help, Some(span)) + } + + /// Append a help message and the span of a graph node to the diagnostic. + pub fn with_node_help(&mut self, node: GlobalNode, message: impl Into) -> &mut Self { + self.with_node(Severity::Help, node, message.into()) + } + + /// Append a warning to the diagnostic. + pub fn with_warning(&mut self, message: impl Into) -> &mut Self { + self.with_child(message, Severity::Warning, Option::::None) + } + + /// Append a warning and the span of a graph node to the diagnostic. + pub fn with_span_warning(&mut self, span: Span, message: impl Into) -> &mut Self { + self.with_child(message, Severity::Warning, Some(span)) + } + + /// Append a warning with a source code span to the diagnostic. + pub fn with_node_warning(&mut self, node: GlobalNode, message: impl Into) -> &mut Self { + self.with_node(Severity::Warning, node, message.into()) + } + + /// Append a note to the diagnostic. + pub fn with_note(&mut self, message: impl Into) -> &mut Self { + self.with_child(message, Severity::Note, Option::::None) + } + + /// Append a note with a source code span to the diagnostic. + pub fn with_span_note(&mut self, span: Span, message: impl Into) -> &mut Self { + self.with_child(message.into(), Severity::Note, Some(span)) + } + + /// Append a note and the span of a graph node to the diagnostic. + pub fn with_node_note(&mut self, node: GlobalNode, message: impl Into) -> &mut Self { + self.with_node(Severity::Note, node, message.into()) + } + + fn with_node(&mut self, severity: Severity, node: GlobalNode, message: String) -> &mut Self { + self.with_child( + message, + severity, + Some(highlighted_node_span(self.base.as_ctx(), node)), + ) + } } /// Low level machinery for diagnostics. @@ -161,16 +433,16 @@ pub trait HasDiagnosticsBase { /// This should be used by implementors of new wrappers, users should use /// high level functions like [`Diagnostics::error`] or /// [`Diagnostics::warning`] instead. - fn record(&self, msg: String, severity: Severity, context: DiagnosticContextStack); + fn record(&self, diagnostic: Diagnostic); /// Access to [`Context`], usually also available via [`std::ops::Deref`]. fn as_ctx(&self) -> &Context; } impl HasDiagnosticsBase for Arc { - fn record(&self, msg: String, severity: Severity, context: DiagnosticContextStack) { + fn record(&self, diagnostic: Diagnostic) { let t: &T = self.as_ref(); - t.record(msg, severity, context) + t.record(diagnostic) } fn as_ctx(&self) -> &Context { @@ -183,8 +455,8 @@ impl HasDiagnosticsBase for &'_ T { (*self).as_ctx() } - fn record(&self, msg: String, severity: Severity, context: DiagnosticContextStack) { - (*self).record(msg, severity, context) + fn record(&self, diagnostic: Diagnostic) { + (*self).record(diagnostic) } } @@ -193,8 +465,8 @@ impl HasDiagnosticsBase for Rc { (**self).as_ctx() } - fn record(&self, msg: String, severity: Severity, context: DiagnosticContextStack) { - (**self).record(msg, severity, context) + fn record(&self, diagnostic: Diagnostic) { + (**self).record(diagnostic) } } @@ -203,15 +475,255 @@ impl HasDiagnosticsBase for Rc { /// This is how any types implementing [`HasDiagnosticsBase`] should actually be /// used. pub trait Diagnostics: HasDiagnosticsBase { + /// Initialize a diagnostic builder for an error. + /// + /// This will fail the policy. + fn struct_error(&self, msg: impl Into) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init( + msg.into(), + Severity::Error, + Option::::None, + self, + ) + } + + /// Initialize a diagnostic builder for an error with a source code span. + /// + /// This will fail the policy. + fn struct_span_error( + &self, + span: impl Into, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init(msg.into(), Severity::Error, Some(span), self) + } + + /// Initialize a diagnostic builder for a warning. + /// + /// Does not fail the policy. + fn struct_warning(&self, msg: impl Into) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init( + msg.into(), + Severity::Warning, + Option::::None, + self, + ) + } + + /// Initialize a diagnostic builder for a warning with a source code span + /// + /// Does not fail the policy. + fn struct_span_warning( + &self, + span: impl Into, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init(msg.into(), Severity::Warning, Some(span), self) + } + + /// Initialize a diagnostic builder for a help message. + fn struct_help(&self, msg: impl Into) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init( + msg.into(), + Severity::Help, + Option::::None, + self, + ) + } + + /// Initialize a diagnostic builder for a help message with a source code span + fn struct_span_help( + &self, + span: impl Into, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init(msg.into(), Severity::Help, Some(span), self) + } + + /// Initialize a diagnostic builder for a note + fn struct_note(&self, msg: impl Into) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init( + msg.into(), + Severity::Note, + Option::::None, + self, + ) + } + + /// Initialize a diagnostic builder for a note with a source code span + fn struct_span_note( + &self, + span: impl Into, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + DiagnosticBuilder::init(msg.into(), Severity::Note, Some(span), self) + } + /// Emit a message that is severe enough that it causes the policy to fail. fn error(&self, msg: impl Into) { - self.record(msg.into(), Severity::Fail, vec![]) + self.struct_error(msg).emit() } /// Emit a message that indicates to the user that the policy might be /// fraudulent but could be correct. fn warning(&self, msg: impl Into) { - self.record(msg.into(), Severity::Warning, vec![]) + self.struct_warning(msg).emit() + } + + /// Emit a message that provides additional information to the user. + fn note(&self, msg: impl Into) { + self.struct_note(msg).emit() + } + + /// Emit a message that suggests something to the user. + fn help(&self, msg: impl Into) { + self.struct_help(msg).emit() + } + + /// Emit a message that is severe enough that it causes the policy to fail + /// with a source code span. + fn span_error(&self, msg: impl Into, span: Span) { + self.struct_span_error(span, msg).emit() + } + + /// Emit a message that indicates to the user that the policy might be + /// fraudulent but could be correct. Includes a source code span. + fn span_warning(&self, msg: impl Into, span: Span) { + self.struct_span_warning(span, msg).emit() + } + + /// Emit a message that provides additional information to the user. + fn span_note(&self, msg: impl Into, span: Span) { + self.struct_span_note(span, msg).emit() + } + + /// Emit a message that suggests something to the user. + fn span_help(&self, msg: impl Into, span: Span) { + self.struct_span_help(span, msg).emit() + } + + /// Initialize a diagnostic builder for an error with the span of a graph + /// node. + /// + /// This will fail the policy. + fn struct_node_error( + &self, + node: GlobalNode, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + struct_node_diagnostic(self, node, Severity::Error, msg) + } + + /// Initialize a diagnostic builder for an error with the span of a graph + /// node. + /// + /// This will not fail the policy. + fn struct_node_warning( + &self, + node: GlobalNode, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + struct_node_diagnostic(self, node, Severity::Warning, msg) + } + + /// Initialize a diagnostic builder for an note with the span of a graph + /// node. + fn struct_node_note( + &self, + node: GlobalNode, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + struct_node_diagnostic(self, node, Severity::Note, msg) + } + + /// Initialize a diagnostic builder for an help message with the span of a graph + /// node. + fn struct_node_help( + &self, + node: GlobalNode, + msg: impl Into, + ) -> DiagnosticBuilder<'_, Self> { + struct_node_diagnostic(self, node, Severity::Help, msg) + } + + /// Emit an error, failing the policy, with the span of a graph node. + fn node_error(&self, node: GlobalNode, msg: impl Into) { + self.struct_node_error(node, msg).emit() + } + + /// Emit an warning, that does not fail the policy, with the span of a graph + /// node. + fn node_warning(&self, node: GlobalNode, msg: impl Into) { + self.struct_node_warning(node, msg).emit() + } + + /// Emit a note with the span of a graph node. + fn node_note(&self, node: GlobalNode, msg: impl Into) { + self.struct_node_note(node, msg).emit() + } + + /// Emit a help message with the span of a graph node. + fn node_help(&self, node: GlobalNode, msg: impl Into) { + self.struct_node_note(node, msg).emit() + } +} + +fn highlighted_node_span(ctx: &Context, node: GlobalNode) -> HighlightedSpan { + let node_span = ctx.get_location(node); + let stmt_span = &ctx.instruction_at_node(node).span; + if stmt_span.contains(node_span) { + HighlightedSpan::new(stmt_span.clone(), node_span.start, node_span.end) + } else { + stmt_span.clone().into() + } +} + +fn struct_node_diagnostic( + base: &B, + node: GlobalNode, + severity: Severity, + msg: impl Into, +) -> DiagnosticBuilder<'_, B> { + DiagnosticBuilder::init( + msg.into(), + severity, + Some(highlighted_node_span(base.as_ctx(), node)), + base, + ) +} + +const TAB_SIZE: usize = 4; + +fn line_length_while(s: &str, mut cont: impl FnMut(char) -> bool) -> usize { + s.chars() + .fold((false, 0), |(found, num), c| { + if found || !cont(c) { + (true, num) + } else { + let more = if c == '\t' { TAB_SIZE } else { 1 }; + (false, num + more) + } + }) + .1 +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::{line_length_while, TAB_SIZE}; + + #[test] + fn test_line_length() { + assert_eq!(line_length_while(" ", |_| true), 2); + assert_eq!(line_length_while(" . ", |_| true), 4); + assert_eq!(line_length_while(" . ", char::is_whitespace), 2); + assert_eq!(line_length_while("\t", |_| true), TAB_SIZE); + assert_eq!(line_length_while("\t . ", |_| true), TAB_SIZE + 3); + assert_eq!(line_length_while(" . \t", |_| true), TAB_SIZE + 3); + assert_eq!(line_length_while("\t. ", char::is_whitespace), TAB_SIZE); + assert_eq!( + line_length_while("\t \t. ", char::is_whitespace), + 2 * TAB_SIZE + 1 + ); } } @@ -275,9 +787,9 @@ impl PolicyContext { } impl HasDiagnosticsBase for PolicyContext { - fn record(&self, msg: String, severity: Severity, mut context: DiagnosticContextStack) { - context.push(format!("[policy: {}]", self.name)); - self.inner.record(msg, severity, context) + fn record(&self, mut diagnostic: Diagnostic) { + diagnostic.context.push(format!("[policy: {}]", self.name)); + self.inner.record(diagnostic) } fn as_ctx(&self) -> &Context { @@ -363,10 +875,10 @@ impl ControllerContext { } impl HasDiagnosticsBase for ControllerContext { - fn record(&self, msg: String, severity: Severity, mut context: DiagnosticContextStack) { + fn record(&self, mut diagnostic: Diagnostic) { let name = self.as_ctx().desc().controllers[&self.id].name; - context.push(format!("[controller: {}]", name)); - self.inner.record(msg, severity, context) + diagnostic.context.push(format!("[controller: {}]", name)); + self.inner.record(diagnostic) } fn as_ctx(&self) -> &Context { @@ -416,9 +928,9 @@ impl CombinatorContext { } impl HasDiagnosticsBase for CombinatorContext { - fn record(&self, msg: String, severity: Severity, mut context: DiagnosticContextStack) { - context.push(format!("{}", self.name)); - self.inner.record(msg, severity, context) + fn record(&self, mut diagnostic: Diagnostic) { + diagnostic.context.push(format!("{}", self.name)); + self.inner.record(diagnostic) } fn as_ctx(&self) -> &Context { @@ -479,6 +991,14 @@ impl Context { #[derive(Debug, Default)] pub(crate) struct DiagnosticsRecorder(std::sync::Mutex>); +struct DisplayDiagnostic<'a>(&'a Diagnostic); + +impl<'a> std::fmt::Display for DisplayDiagnostic<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.write(f) + } +} + impl DiagnosticsRecorder { /// Emit queued diagnostics, draining the internal queue of diagnostics. /// @@ -488,11 +1008,8 @@ impl DiagnosticsRecorder { let w = &mut w; let mut can_continue = true; for diag in self.0.lock().unwrap().drain(..) { - for ctx in diag.context.iter().rev() { - write!(w, "{ctx} ")?; - } - writeln!(w, "{}", diag.message)?; - can_continue &= !diag.severity.must_abort(); + writeln!(w, "{}", DisplayDiagnostic(&diag))?; + can_continue &= !diag.main.severity.must_abort(); } Ok(can_continue) } @@ -500,12 +1017,8 @@ impl DiagnosticsRecorder { impl HasDiagnosticsBase for Context { /// Record a diagnostic message. - fn record(&self, message: String, severity: Severity, context: DiagnosticContextStack) { - self.diagnostics.0.lock().unwrap().push(Diagnostic { - message, - severity, - context, - }) + fn record(&self, diagnostic: Diagnostic) { + self.diagnostics.0.lock().unwrap().push(diagnostic); } fn as_ctx(&self) -> &Context { diff --git a/crates/paralegal-policy/src/lib.rs b/crates/paralegal-policy/src/lib.rs index 63bebef12c..51746f41f4 100644 --- a/crates/paralegal-policy/src/lib.rs +++ b/crates/paralegal-policy/src/lib.rs @@ -191,3 +191,19 @@ impl GraphLocation { Ok(Context::new(desc)) } } + +/// A convenience macro that uses `file!`, `line!` and `column!` to return the +/// string `"file:line:column"`. This can be used to mention policy source +/// locations in policies. +/// +/// If additional arguments are procided these are `concat!`ed to the end with a +/// space in betwee the location and the rest. +#[macro_export] +macro_rules! loc { + () => { + concat!(file!(), ':', line!(), ':', column!(),) + }; + ($($t:tt)+) => { + concat!(file!(), ':', line!(), ':', column!(), ' ', $($t)+) + }; +} diff --git a/crates/paralegal-policy/src/test_utils.rs b/crates/paralegal-policy/src/test_utils.rs index 5c9f263c18..9c0c76441f 100644 --- a/crates/paralegal-policy/src/test_utils.rs +++ b/crates/paralegal-policy/src/test_utils.rs @@ -3,7 +3,7 @@ use crate::ControllerId; use paralegal_flow::test_utils::PreFrg; use paralegal_spdg::IntoIterGlobalNodes; use paralegal_spdg::NodeCluster; -use paralegal_spdg::{Identifier, InstructionInfo, Node as SPDGNode, SPDG}; +use paralegal_spdg::{Identifier, InstructionKind, Node as SPDGNode, SPDG}; use std::sync::Arc; use std::sync::OnceLock; @@ -51,8 +51,8 @@ fn is_at_function_call_with_name( let weight = ctrl.graph.node_weight(node).unwrap().at; let instruction = &ctx.desc().instruction_info[&weight.leaf()]; matches!( - instruction, - InstructionInfo::FunctionCall(call) if + instruction.kind, + InstructionKind::FunctionCall(call) if ctx.desc().def_info[&call.id].name == name ) } diff --git a/crates/paralegal-spdg/Cargo.toml b/crates/paralegal-spdg/Cargo.toml index 2e85c4a999..f200fe0680 100644 --- a/crates/paralegal-spdg/Cargo.toml +++ b/crates/paralegal-spdg/Cargo.toml @@ -16,7 +16,7 @@ log = "0.4" internment = { version = "0.7.1", features = ["serde"] } indexical = { workspace = true } itertools = "0.11.0" -strum = { version = "0.25", features = ["derive"] } +strum = { workspace = true } cfg-if = "1" #flowistry_pdg = { path = "../../../flowistry/crates/flowistry_pdg" } flowistry_pdg = { git = "https://github.com/brownsys/flowistry", rev = "d1fcc76509032dd94f5255fd03c0ad0397efe834" } diff --git a/crates/paralegal-spdg/src/dot.rs b/crates/paralegal-spdg/src/dot.rs index bb12950e2c..e6c228f682 100644 --- a/crates/paralegal-spdg/src/dot.rs +++ b/crates/paralegal-spdg/src/dot.rs @@ -1,6 +1,6 @@ //! Display SPDGs as dot graphs -use crate::{GlobalEdge, InstructionInfo, Node, ProgramDescription}; +use crate::{GlobalEdge, InstructionKind, Node, ProgramDescription}; use dot::{CompassPoint, Edges, Id, LabelText, Nodes}; use flowistry_pdg::rustc_portable::LocalDefId; use flowistry_pdg::{CallString, RichLocation}; @@ -108,7 +108,7 @@ impl<'a, 'd> dot::Labeller<'a, CallString, GlobalEdge> for DotPrintableProgramDe fn node_label(&'a self, n: &CallString) -> LabelText<'a> { let (ctrl_id, nodes) = &self.call_sites[n]; let ctrl = &self.spdg.controllers[ctrl_id]; - let instruction = self.spdg.instruction_info[&n.leaf()]; + let instruction = &self.spdg.instruction_info[&n.leaf()]; let write_label = || { use std::fmt::Write; @@ -116,17 +116,17 @@ impl<'a, 'd> dot::Labeller<'a, CallString, GlobalEdge> for DotPrintableProgramDe write!(s, "{}|", self.format_call_string(*n))?; - match instruction { - InstructionInfo::Statement => s.push('S'), - InstructionInfo::FunctionCall(function) => { + match instruction.kind { + InstructionKind::Statement => s.push('S'), + InstructionKind::FunctionCall(function) => { let info = &self.spdg.def_info[&function.id]; write!(s, "{}", info.name)? } - InstructionInfo::Terminator => s.push('T'), - InstructionInfo::Start => { + InstructionKind::Terminator => s.push('T'), + InstructionKind::Start => { s.push('*'); } - InstructionInfo::Return => s.push_str("end"), + InstructionKind::Return => s.push_str("end"), }; for &n in nodes { diff --git a/crates/paralegal-spdg/src/lib.rs b/crates/paralegal-spdg/src/lib.rs index 2735365bea..5ba464d017 100644 --- a/crates/paralegal-spdg/src/lib.rs +++ b/crates/paralegal-spdg/src/lib.rs @@ -32,7 +32,7 @@ use internment::Intern; use itertools::Itertools; use rustc_portable::DefId; use serde::{Deserialize, Serialize}; -use std::{fmt, hash::Hash}; +use std::{fmt, hash::Hash, path::PathBuf}; use utils::serde_map_via_vec; @@ -125,6 +125,8 @@ pub struct DefInfo { pub path: Vec, /// Kind of object pub kind: DefKind, + /// Information about the span + pub src_info: Span, } /// Similar to `DefKind` in rustc but *not the same*! @@ -142,6 +144,64 @@ pub enum DefKind { Type, } +/// An interned [`SourceFileInfo`] +#[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)] +pub struct SourceFile(Intern); + +impl std::ops::Deref for SourceFile { + type Target = SourceFileInfo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/// Information about a source file +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)] +pub struct SourceFileInfo { + /// Printable location of the source code file - either an absolute path to library source code + /// or a path relative to within the compiled crate (e.g. `src/...`) + pub file_path: String, + /// Absolute path to source code file + pub abs_file_path: PathBuf, +} + +impl SourceFileInfo { + /// Intern the source file + pub fn intern(self) -> SourceFile { + SourceFile(Intern::new(self)) + } +} + +/// A "point" within a source file. Used to compose and compare spans. +/// +/// NOTE: The ordering of this type must be such that if point "a" is earlier in +/// the file than "b", then "a" < "b". +#[derive(Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Debug, PartialOrd, Ord)] +pub struct SpanCoord { + /// Line in the source file + pub line: u32, + /// Column of the line + pub col: u32, +} + +/// Encodes a source code location +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)] +pub struct Span { + /// Which file this comes from + pub source_file: SourceFile, + /// Starting coordinates of the span + pub start: SpanCoord, + /// Ending coordinates of the span, + pub end: SpanCoord, +} + +impl Span { + /// Is `other` completely contained within `self` + pub fn contains(&self, other: &Self) -> bool { + self.source_file == other.source_file && self.start <= other.start && self.end >= other.end + } +} + /// Metadata on a function call. #[derive(Debug, Clone, Copy, Serialize, Deserialize, Eq, Ord, PartialOrd, PartialEq)] pub struct FunctionCallInfo { @@ -156,7 +216,7 @@ pub struct FunctionCallInfo { #[derive( Debug, Clone, Copy, Serialize, Deserialize, Eq, Ord, PartialOrd, PartialEq, strum::EnumIs, )] -pub enum InstructionInfo { +pub enum InstructionKind { /// Some type of statement Statement, /// A function call @@ -169,16 +229,25 @@ pub enum InstructionInfo { Return, } -impl InstructionInfo { +impl InstructionKind { /// If this identifies a function call, return the information inside. pub fn as_function_call(self) -> Option { match self { - InstructionInfo::FunctionCall(d) => Some(d), + InstructionKind::FunctionCall(d) => Some(d), _ => None, } } } +/// Information about an instruction represented in the PDG +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct InstructionInfo { + /// Classification of the instruction + pub kind: InstructionKind, + /// The source code span + pub span: Span, +} + /// information about each encountered type. pub type TypeInfoMap = HashMap; @@ -543,6 +612,8 @@ pub struct NodeInfo { pub description: String, /// Additional information of how this node is used in the source. pub kind: NodeKind, + /// Span information for this node + pub span: Span, } impl Display for NodeInfo { diff --git a/props/Cargo.lock b/props/Cargo.lock index 2fe0d87fda..48dbb9fd20 100644 --- a/props/Cargo.lock +++ b/props/Cargo.lock @@ -153,6 +153,17 @@ dependencies = [ "os_str_bytes", ] +[[package]] +name = "colored" +version = "1.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a5f741c91823341bebf717d4c71bda820630ce065443b58bd1b7451af008355" +dependencies = [ + "is-terminal", + "lazy_static", + "winapi", +] + [[package]] name = "colored" version = "2.0.4" @@ -462,6 +473,7 @@ version = "0.1.0" dependencies = [ "anyhow", "bitvec", + "colored 1.9.4", "indexical", "itertools 0.12.1", "lazy_static", @@ -470,6 +482,7 @@ dependencies = [ "petgraph", "serde_json", "simple_logger", + "strum", ] [[package]] @@ -671,7 +684,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48047e77b528151aaf841a10a9025f9459da80ba820e425ff7eb005708a76dc7" dependencies = [ "atty", - "colored", + "colored 2.0.4", "log", "time", "winapi",