Skip to content

Commit

Permalink
feat(core): minimize debug and bug fixes (#86)
Browse files Browse the repository at this point in the history
* struct fixes & minimize debug

* cleanup

* clippy lint fixes and silence

* cleanup

* lint

* tiny fixes

* remove print

* remove print

* remove prints

* remove prints

* fix panic in mermaid node

* ignore mini.sol
  • Loading branch information
brockelmore authored Jul 11, 2024
1 parent 44ec8f1 commit e030f38
Show file tree
Hide file tree
Showing 73 changed files with 3,375 additions and 1,606 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ name: Rust

on:
push:
branches: [ "master" ]
branches: ["master"]
pull_request:
branches: [ "master" ]
branches: ["master"]

env:
CARGO_TERM_COLOR: always
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@
**/.swp
.vscode
.env
notes.md
notes.md
mini.sol
2 changes: 2 additions & 0 deletions crates/analyzers/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::too_many_arguments)]

pub mod bounds;

use ariadne::{Cache, Label, Report, ReportKind, Span};
Expand Down
20 changes: 14 additions & 6 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ struct Args {
/// Post pyrometer debugging information to debugging site
#[clap(long)]
pub debug_site: bool,

#[clap(long)]
pub minimize_debug: Option<String>,
}

pub fn subscriber() {
Expand Down Expand Up @@ -238,10 +241,10 @@ fn main() {
let mut analyzer = Analyzer {
max_depth: args.max_stack_depth,
root: Root::RemappingsDirectory(env::current_dir().unwrap()),
debug_panic: args.debug_panic || args.minimize_debug.is_some(),
minimize_debug: args.minimize_debug,
..Default::default()
};
println!("debug panic: {}", args.debug_panic);
analyzer.debug_panic = args.debug_panic;

let (current_path, sol) = if args.path.ends_with(".sol") {
let sol = fs::read_to_string(args.path.clone()).expect("Could not find file");
Expand Down Expand Up @@ -490,10 +493,15 @@ fn main() {
}
}
} else if let Some(ctx) = FunctionNode::from(func).maybe_body_ctx(&mut analyzer) {
let analysis = analyzer
.bounds_for_all(arena, &file_mapping, ctx, config)
.as_cli_compat(&file_mapping);
analysis.print_reports(&mut source_map, &analyzer, arena);
if !matches!(
ctx.underlying(&analyzer).unwrap().killed,
Some((_, graph::nodes::KilledKind::DebugIgnored))
) {
let analysis = analyzer
.bounds_for_all(arena, &file_mapping, ctx, config)
.as_cli_compat(&file_mapping);
analysis.print_reports(&mut source_map, &analyzer, arena);
}
}
}
} else {
Expand Down
6 changes: 5 additions & 1 deletion crates/graph/src/graph_elements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ pub enum Edge {
LibraryFunction(NodeIdx),
/// A connection for a builtin function
BuiltinFunction,
/// A connection from one contract to another contract
UsingContract(Loc),
}

impl Heirarchical for Edge {
Expand All @@ -272,7 +274,7 @@ impl Heirarchical for Edge {

Contract | Ty | Field | Enum | Struct | Error | Event | Var | InheritedContract
| Modifier | FallbackFunc | Constructor | ReceiveFunc | LibraryFunction(_)
| BuiltinFunction | Func => 2,
| BuiltinFunction | Func | UsingContract(_) => 2,

Context(_) | ErrorParam | FunctionParam | FunctionReturn | FuncModifier(_) => 3,
}
Expand Down Expand Up @@ -333,6 +335,8 @@ pub enum ContextEdge {
InheritedStorageVariable,
/// A connection to the calldata variable
CalldataVariable,
/// A contract variable (storage, consts, immutables, etc)
ContractVariable,

/// A connection between a variable and a parent variable where the child is some attribute on the parent
/// (i.e. `.length`)
Expand Down
3 changes: 3 additions & 0 deletions crates/graph/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![allow(clippy::too_many_arguments)]
#![allow(clippy::type_complexity)]

mod graph_elements;
mod range;
mod var_type;
Expand Down
3 changes: 3 additions & 0 deletions crates/graph/src/nodes/context/expr_ret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub enum KilledKind {
Revert,
/// Unexpected parse error. This is likely a bug or invalid solidity. See the `errors` section of the CLI output or rerun with `--debug` for more information
ParseError,
/// This context was not evaluated because it was not on the path to analyzing the requested context to debug
DebugIgnored,
}

impl KilledKind {
Expand All @@ -27,6 +29,7 @@ impl KilledKind {
Unreachable => "Unsatisifiable bounds, therefore dead code",
Revert => "Execution guaranteed to revert here!",
ParseError => "Unexpected parse error. This is likely a bug or invalid solidity. See the `errors` section of the CLI output or rerun with `--debug` for more information",
DebugIgnored => "Ignored due to debug_ctx_path being set",
}
}
}
Expand Down
31 changes: 29 additions & 2 deletions crates/graph/src/nodes/context/querying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
AnalyzerBackend, ContextEdge, Edge, GraphBackend,
};

use shared::{GraphError, Search};
use shared::{GraphError, NodeIdx, Search};
use std::collections::{BTreeMap, BTreeSet};

impl ContextNode {
Expand Down Expand Up @@ -68,6 +68,32 @@ impl ContextNode {
}
}

pub fn keep_inscope_tys(
&self,
idxs: &mut Vec<NodeIdx>,
analyzer: &mut impl AnalyzerBackend,
) -> Result<(), GraphError> {
let mut tys = self
.visible_structs(analyzer)?
.iter()
.map(|i| i.0.into())
.collect::<Vec<_>>();
if let Some(contract) = self.maybe_associated_contract(analyzer)? {
tys.extend(contract.visible_nodes(analyzer));
}

if let Some(source) = self.maybe_associated_source(analyzer) {
tys.extend(source.visible_nodes(analyzer)?);
}

tys.sort();
tys.dedup();

idxs.retain(|i| tys.contains(i));

Ok(())
}

/// Gets visible functions
pub fn visible_modifiers(
&self,
Expand Down Expand Up @@ -226,7 +252,8 @@ impl ContextNode {

let mut structs = source.visible_structs(analyzer)?;
let contract = self.associated_contract(analyzer)?;
structs.extend(contract.visible_structs(analyzer));
let contract_visible = contract.visible_structs(analyzer);
structs.extend(contract_visible);

structs.sort();
structs.dedup();
Expand Down
2 changes: 1 addition & 1 deletion crates/graph/src/nodes/context/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl ContextNode {
Ok(underlying.fn_call.is_none() && underlying.ext_fn_call.is_none() && !underlying.is_fork)
}

pub fn has_continuation(&self, analyzer: &mut impl GraphBackend) -> Result<bool, GraphError> {
pub fn has_continuation(&self, analyzer: &impl GraphBackend) -> Result<bool, GraphError> {
Ok(self.underlying(analyzer)?.continuation_of.is_some())
}

Expand Down
5 changes: 5 additions & 0 deletions crates/graph/src/nodes/context/underlying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ pub struct Context {
pub cache: ContextCache,
/// A difference logic solver used for determining reachability
pub dl_solver: DLSolver,
/// Functions applied (but not reparsed) in this context
pub applies: Vec<FunctionNode>,
}

impl Context {
Expand Down Expand Up @@ -89,6 +91,7 @@ impl Context {
number_of_live_edges: 0,
cache: Default::default(),
dl_solver: Default::default(),
applies: Default::default(),
}
}

Expand Down Expand Up @@ -238,6 +241,7 @@ impl Context {
associated_contract: None,
},
dl_solver: parent_ctx.underlying(analyzer)?.dl_solver.clone(),
applies: Default::default(),
})
}

Expand Down Expand Up @@ -300,6 +304,7 @@ impl Context {
associated_contract: None,
},
dl_solver: parent_ctx.underlying(analyzer)?.dl_solver.clone(),
applies: Default::default(),
})
}

Expand Down
49 changes: 44 additions & 5 deletions crates/graph/src/nodes/context/var/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,32 @@ impl ContextVarNode {
)
}

pub fn maybe_ctx(&self, analyzer: &impl GraphBackend) -> Option<ContextNode> {
pub fn is_struct_field(&self, analyzer: &impl GraphBackend) -> bool {
self.struct_parent(analyzer).is_some()
}

pub fn struct_parent(&self, analyzer: &impl GraphBackend) -> Option<ContextVarNode> {
let first = self.first_version(analyzer);
analyzer
.graph()
.edges_directed(first.0.into(), Direction::Outgoing)
.filter(|edge| *edge.weight() == Edge::Context(ContextEdge::Variable))
.map(|edge| ContextNode::from(edge.target()))
.take(1)
.next()
.find(|edge| *edge.weight() == Edge::Context(ContextEdge::AttrAccess("field")))
.map(|edge| edge.target().into())
}

pub fn maybe_ctx(&self, analyzer: &impl GraphBackend) -> Option<ContextNode> {
if let Some(struct_parent) = self.struct_parent(analyzer) {
struct_parent.maybe_ctx(analyzer)
} else {
let first = self.first_version(analyzer);
analyzer
.graph()
.edges_directed(first.0.into(), Direction::Outgoing)
.filter(|edge| *edge.weight() == Edge::Context(ContextEdge::Variable))
.map(|edge| ContextNode::from(edge.target()))
.take(1)
.next()
}
}

pub fn maybe_storage_var(&self, analyzer: &impl GraphBackend) -> Option<VarNode> {
Expand Down Expand Up @@ -212,6 +229,10 @@ impl ContextVarNode {
Ok(self.underlying(analyzer)?.tmp_of())
}

pub fn is_struct(&self, analyzer: &impl GraphBackend) -> Result<bool, GraphError> {
Ok(!self.struct_to_fields(analyzer)?.is_empty())
}

pub fn struct_to_fields(
&self,
analyzer: &impl GraphBackend,
Expand All @@ -229,6 +250,24 @@ impl ContextVarNode {
}
}

pub fn field_of_struct(
&self,
name: &str,
analyzer: &impl GraphBackend,
) -> Result<Option<ContextVarNode>, GraphError> {
let fields = self.struct_to_fields(analyzer)?;
Ok(fields
.iter()
.find(|field| {
if let Ok(field_name) = field.name(analyzer) {
field_name.ends_with(&format!(".{}", name))
} else {
false
}
})
.copied())
}

pub fn array_to_len_var(&self, analyzer: &impl GraphBackend) -> Option<ContextVarNode> {
if let Some(len) = analyzer
.graph()
Expand Down
66 changes: 63 additions & 3 deletions crates/graph/src/nodes/context/var/typing.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::{
elem::Elem,
nodes::{Builtin, Concrete, ContextNode, ContextVarNode},
nodes::{
Builtin, Concrete, ContextNode, ContextVarNode, EnumNode, ErrorNode, StructNode, TyNode,
},
range::{
elem::{RangeElem, RangeExpr, RangeOp},
RangeEval,
},
AnalyzerBackend, ContextEdge, Edge, GraphBackend, Node, VarType,
AnalyzerBackend, ContextEdge, Edge, GraphBackend, Node, TypeNode, VarType,
};

use shared::{GraphError, RangeArena, Search, StorageLocation};
Expand Down Expand Up @@ -89,6 +91,58 @@ impl ContextVarNode {
))
}

pub fn maybe_struct(
&self,
analyzer: &impl GraphBackend,
) -> Result<Option<StructNode>, GraphError> {
if let VarType::User(TypeNode::Struct(ut), _) = self.ty(analyzer)? {
Ok(Some(*ut))
} else {
Ok(None)
}
}

pub fn maybe_enum(&self, analyzer: &impl GraphBackend) -> Result<Option<EnumNode>, GraphError> {
if let VarType::User(TypeNode::Enum(ut), _) = self.ty(analyzer)? {
Ok(Some(*ut))
} else {
Ok(None)
}
}

pub fn maybe_ty_node(
&self,
analyzer: &impl GraphBackend,
) -> Result<Option<TyNode>, GraphError> {
if let VarType::User(TypeNode::Ty(ut), _) = self.ty(analyzer)? {
Ok(Some(*ut))
} else {
Ok(None)
}
}

pub fn maybe_err_node(
&self,
analyzer: &impl GraphBackend,
) -> Result<Option<ErrorNode>, GraphError> {
if let VarType::User(TypeNode::Error(ut), _) = self.ty(analyzer)? {
Ok(Some(*ut))
} else {
Ok(None)
}
}

pub fn maybe_usertype(
&self,
analyzer: &impl GraphBackend,
) -> Result<Option<TypeNode>, GraphError> {
if let VarType::User(ut, _) = self.ty(analyzer)? {
Ok(Some(*ut))
} else {
Ok(None)
}
}

pub fn is_return_assignment(&self, analyzer: &impl GraphBackend) -> bool {
analyzer
.graph()
Expand Down Expand Up @@ -554,7 +608,13 @@ impl ContextVarNode {
if let Some(to_range) = to_ty.range(analyzer)? {
let mut min_expr = (*self)
.range_min(analyzer)?
.unwrap()
.unwrap_or_else(|| {
panic!(
"{:?}, {} had no min",
self.loc(analyzer).unwrap(),
self.display_name(analyzer).unwrap()
)
})
.cast(to_range.min.clone())
.min(
(*self)
Expand Down
Loading

0 comments on commit e030f38

Please sign in to comment.