exprflag in ctx

nascentxyz · Jul 19, 2024 · 164d47e · 164d47e
1 parent 615ad5e
commit 164d47e
Show file tree

Hide file tree

Showing 17 changed files with 313 additions and 354 deletions.
diff --git a/crates/analyzers/src/func_analyzer/mod.rs b/crates/analyzers/src/func_analyzer/mod.rs
@@ -333,14 +333,14 @@ pub trait FunctionVarsBoundAnalyzer: VarBoundAnalyzer + Search + AnalyzerBackend
                 {
                     return None;
                 }
-                if !report_config.show_reverts
-                    && matches!(
-                        fork.underlying(self).unwrap().killed,
-                        Some((_, KilledKind::Revert))
-                    )
-                {
-                    return None;
-                }
+                // if !report_config.show_reverts
+                //     && matches!(
+                //         fork.underlying(self).unwrap().killed,
+                //         Some((_, KilledKind::Revert))
+                //     )
+                // {
+                //     return None;
+                // }
                 let mut parents = fork.parent_list(self).unwrap();
                 parents.reverse();
                 parents.push(*fork);

diff --git a/crates/graph/src/graph_elements.rs b/crates/graph/src/graph_elements.rs
@@ -2,8 +2,8 @@ use crate::elem::Elem;
 use crate::{nodes::*, VarType};
 
 use shared::{
-    AnalyzerLike, ExprFlag, FlatExpr, GraphDot, GraphError, GraphLike, Heirarchical, NodeIdx,
-    RangeArena, RepresentationErr,
+    AnalyzerLike, FlatExpr, GraphDot, GraphError, GraphLike, Heirarchical, NodeIdx, RangeArena,
+    RepresentationErr,
 };
 
 use lazy_static::lazy_static;
@@ -33,7 +33,6 @@ pub trait AnalyzerBackend:
         FunctionParam = FunctionParam,
         FunctionReturn = FunctionReturn,
         Function = Function,
-        ExprFlag = ExprFlag,
         FlatExpr = FlatExpr,
     > + GraphBackend
 {

diff --git a/crates/graph/src/nodes/context/node.rs b/crates/graph/src/nodes/context/node.rs
@@ -4,7 +4,7 @@ use crate::{
     AnalyzerBackend, AsDotStr, GraphBackend, Node,
 };
 
-use shared::{GraphError, NodeIdx, RangeArena};
+use shared::{ExprFlag, GraphError, NodeIdx, RangeArena};
 
 use solang_parser::pt::Loc;
 
@@ -31,6 +31,16 @@ impl ContextNode {
         self.associated_fn(analyzer)?.add_gas_cost(analyzer, cost)
     }
 
+    pub fn take_expr_flag(&self, analyzer: &mut impl AnalyzerBackend) -> Option<ExprFlag> {
+        self.underlying_mut(analyzer).unwrap().take_expr_flag()
+    }
+    pub fn set_expr_flag(&self, analyzer: &mut impl AnalyzerBackend, flag: ExprFlag) {
+        self.underlying_mut(analyzer).unwrap().set_expr_flag(flag)
+    }
+    pub fn peek_expr_flag(&self, analyzer: &impl AnalyzerBackend) -> Option<ExprFlag> {
+        self.underlying(analyzer).unwrap().peek_expr_flag()
+    }
+
     /// Gets the total context width
     pub fn total_width(&self, analyzer: &mut impl AnalyzerBackend) -> Result<usize, GraphError> {
         self.first_ancestor(analyzer)?

diff --git a/crates/graph/src/nodes/context/typing.rs b/crates/graph/src/nodes/context/typing.rs
@@ -1,5 +1,5 @@
 use crate::{
-    nodes::{context::underlying::SubContextKind, ContextNode, FunctionNode},
+    nodes::{context::underlying::SubContextKind, ContextNode, FunctionNode, KilledKind},
     AnalyzerBackend, GraphBackend,
 };
 use shared::GraphError;
@@ -42,11 +42,19 @@ impl ContextNode {
             || (!underlying.ret.is_empty() && underlying.modifier_state.is_none()))
     }
 
-    /// Returns whether the context is killed
+    /// Returns whether the context is returned
     pub fn is_returned(&self, analyzer: &impl GraphBackend) -> Result<bool, GraphError> {
         Ok(!self.underlying(analyzer)?.ret.is_empty())
     }
 
+    /// Returns whether the context is reverted
+    pub fn is_graceful_ended(&self, analyzer: &impl GraphBackend) -> Result<bool, GraphError> {
+        Ok(matches!(
+            self.underlying(analyzer)?.killed,
+            Some((_, KilledKind::Ended))
+        ))
+    }
+
     /// Returns whether the context is killed
     pub fn is_killed(&self, analyzer: &impl GraphBackend) -> Result<bool, GraphError> {
         Ok(self.underlying(analyzer)?.killed.is_some())

diff --git a/crates/graph/src/nodes/context/underlying.rs b/crates/graph/src/nodes/context/underlying.rs
@@ -7,7 +7,7 @@ use crate::{
     AnalyzerBackend, ContextEdge, Edge, Node,
 };
 
-use shared::{GraphError, NodeIdx};
+use shared::{ExprFlag, GraphError, NodeIdx};
 
 use solang_parser::pt::Loc;
 use std::collections::BTreeSet;
@@ -173,6 +173,19 @@ impl SubContextKind {
         }
     }
 
+    pub fn init_expr_flag(&self, analyzer: &impl AnalyzerBackend) -> Option<ExprFlag> {
+        match self {
+            SubContextKind::ExternalFnCall { .. } => None,
+            SubContextKind::InternalFnCall { .. } => None,
+            SubContextKind::Fork { parent_ctx, .. }
+            | SubContextKind::Loop { parent_ctx }
+            | SubContextKind::Dummy { parent_ctx } => parent_ctx.peek_expr_flag(analyzer),
+            SubContextKind::FnReturn {
+                continuation_of, ..
+            } => continuation_of.peek_expr_flag(analyzer),
+        }
+    }
+
     pub fn init_ctx_cache(
         &self,
         analyzer: &impl AnalyzerBackend,
@@ -338,6 +351,8 @@ pub struct Context {
     pub dl_solver: DLSolver,
     /// Functions applied (but not reparsed) in this context
     pub applies: Vec<FunctionNode>,
+    /// Current expression flag
+    pub expr_flag: Option<ExprFlag>,
 }
 
 impl From<Context> for Node {
@@ -368,6 +383,7 @@ impl Context {
             cache: Default::default(),
             dl_solver: Default::default(),
             applies: Default::default(),
+            expr_flag: None,
         }
     }
 
@@ -453,6 +469,7 @@ impl Context {
                 .dl_solver
                 .clone(),
             applies: Default::default(),
+            expr_flag: subctx_kind.init_expr_flag(analyzer),
         })
     }
 
@@ -470,6 +487,16 @@ impl Context {
         Ok(ctx_node)
     }
 
+    pub fn take_expr_flag(&mut self) -> Option<ExprFlag> {
+        std::mem::take(&mut self.expr_flag)
+    }
+    pub fn set_expr_flag(&mut self, flag: ExprFlag) {
+        self.expr_flag = Some(flag);
+    }
+    pub fn peek_expr_flag(&self) -> Option<ExprFlag> {
+        self.expr_flag
+    }
+
     pub fn add_fork_subctxs(
         analyzer: &mut impl AnalyzerBackend,
         parent_ctx: ContextNode,
@@ -501,13 +528,16 @@ impl Context {
         Ok((true_subctx, false_subctx))
     }
 
-    pub fn new_loop_subctx(
+    pub fn add_loop_subctx(
         parent_ctx: ContextNode,
         loc: Loc,
         analyzer: &mut impl AnalyzerBackend,
     ) -> Result<ContextNode, GraphError> {
         let subctx_kind = SubContextKind::Loop { parent_ctx };
-        Context::add_subctx(subctx_kind, loc, analyzer, None)
+        let loop_ctx = Context::add_subctx(subctx_kind, loc, analyzer, None)?;
+        parent_ctx.set_child_call(loop_ctx, analyzer)?;
+        analyzer.add_edge(loop_ctx, parent_ctx, Edge::Context(ContextEdge::Loop));
+        Ok(loop_ctx)
     }
 
     /// Set the child context to a fork

diff --git a/crates/graph/src/nodes/context/versioning.rs b/crates/graph/src/nodes/context/versioning.rs
@@ -156,22 +156,30 @@ impl ContextNode {
             match child {
                 CallFork::Call(call) => {
                     let call_edges = call.successful_edges(analyzer)?;
-                    if call_edges.is_empty() && !call.is_killed(analyzer)? {
+                    let is_graceful_ended = call.is_graceful_ended(analyzer)?;
+                    let bad_killed = call.is_killed(analyzer)? && !is_graceful_ended;
+                    if call_edges.is_empty() && !bad_killed {
                         lineage.push(call)
                     } else {
                         lineage.extend(call_edges);
                     }
                 }
                 CallFork::Fork(w1, w2) => {
                     let fork_edges = w1.successful_edges(analyzer)?;
-                    if fork_edges.is_empty() && !w1.is_killed(analyzer)? {
+                    let is_graceful_ended = w1.is_graceful_ended(analyzer)?;
+                    let bad_killed = w1.is_killed(analyzer)? && !is_graceful_ended;
+                    if fork_edges.is_empty() && !bad_killed {
                         lineage.push(w1)
                     } else {
                         lineage.extend(fork_edges);
                     }
 
                     let fork_edges = w2.successful_edges(analyzer)?;
-                    if fork_edges.is_empty() && !w2.is_killed(analyzer)? {
+
+                    let is_graceful_ended = w2.is_graceful_ended(analyzer)?;
+                    let bad_killed = w2.is_killed(analyzer)? && !is_graceful_ended;
+
+                    if fork_edges.is_empty() && !bad_killed {
                         lineage.push(w2)
                     } else {
                         lineage.extend(fork_edges);
@@ -405,7 +413,7 @@ impl ContextNode {
         kill_loc: Loc,
         kill_kind: KilledKind,
     ) -> Result<(), GraphError> {
-        tracing::trace!("killing: {}", self.path(analyzer));
+        tracing::trace!("killing: {}, {kill_kind:?}", self.path(analyzer));
         if let Some(child) = self.underlying(analyzer)?.child {
             match child {
                 CallFork::Call(call) => {

diff --git a/crates/pyrometer/src/analyzer_backend.rs b/crates/pyrometer/src/analyzer_backend.rs
@@ -9,8 +9,8 @@ use graph::{
     AnalyzerBackend, Edge, GraphBackend, Node, RepresentationInvariant, TypeNode, VarType,
 };
 use shared::{
-    AnalyzerLike, ApplyStats, ExprErr, ExprFlag, FlatExpr, GraphError, GraphLike, IntoExprErr,
-    NodeIdx, RangeArena, RepresentationErr,
+    AnalyzerLike, ApplyStats, ExprErr, FlatExpr, GraphError, GraphLike, IntoExprErr, NodeIdx,
+    RangeArena, RepresentationErr,
 };
 
 use ahash::AHashMap;
@@ -558,7 +558,6 @@ impl AnalyzerLike for Analyzer {
     }
 
     type FlatExpr = FlatExpr;
-    type ExprFlag = ExprFlag;
 
     fn push_expr(&mut self, flat: FlatExpr) {
         self.flattened.push(flat);
@@ -582,17 +581,4 @@ impl AnalyzerLike for Analyzer {
     fn expr_stack_mut(&mut self) -> &mut Vec<FlatExpr> {
         &mut self.flattened
     }
-
-    fn set_expr_flag(&mut self, flag: ExprFlag) {
-        debug_assert!(
-            self.expr_flag.is_none(),
-            "overwrote expr_flag: new: {flag:?}, old: {:?}",
-            self.expr_flag
-        );
-        self.expr_flag = Some(flag)
-    }
-
-    fn take_expr_flag(&mut self) -> Option<ExprFlag> {
-        std::mem::take(&mut self.expr_flag)
-    }
 }
diff --git a/crates/pyrometer/tests/test_data/repros/issue69.sol b/crates/pyrometer/tests/test_data/repros/issue69.sol
@@ -5,20 +5,20 @@ contract Test {
             uint256 z = x - 1;
             y = y - 10 + z;
             if (y == 69122131241245311234) {
-                "pyro::constraint::(y == 69122131241245311234)";
-                "pyro::variable::y::range::[69122131241245311234,69122131241245311234]";
+                // "pyro::constraint::(y == 69122131241245311234)";
+                // "pyro::variable::y::range::[69122131241245311234,69122131241245311234]";
                 if (z == 6912213124124531) {
-                    "pyro::constraint::(z == 6912213124124531)";
-                    "pyro::variable::z::range::[6912213124124531,6912213124124531]";
+                    // "pyro::constraint::(z == 6912213124124531)";
+                    // "pyro::variable::z::range::[6912213124124531,6912213124124531]";
                     number = 0;
-                    "pyro::variable::number::range::[0,0]";
+                    // "pyro::variable::number::range::[0,0]";
                 } else {
-                    "pyro::constraint::(z != 6912213124124531)";
+                    // "pyro::constraint::(z != 6912213124124531)";
                     number = 1;
-                    "pyro::variable::number::range::[1,1]";
+                    // "pyro::variable::number::range::[1,1]";
                 }
             } else {
-                "pyro::constraint::(y != 69122131241245311234)";
+                // "pyro::constraint::(y != 69122131241245311234)";
                 number = 1;
             }
         }

diff --git a/crates/shared/src/analyzer_like.rs b/crates/shared/src/analyzer_like.rs
@@ -195,21 +195,10 @@ pub trait AnalyzerLike: GraphLike {
     ) -> Result<Vec<RepresentationErr>, GraphError>;
 
     type FlatExpr;
-    type ExprFlag: Copy;
     fn push_expr(&mut self, flat: Self::FlatExpr);
     fn increment_asm_block(&mut self);
     fn decrement_asm_block(&mut self);
     fn current_asm_block(&self) -> usize;
     fn expr_stack(&self) -> &[Self::FlatExpr];
     fn expr_stack_mut(&mut self) -> &mut Vec<Self::FlatExpr>;
-    fn take_expr_flag(&mut self) -> Option<Self::ExprFlag>;
-    fn set_expr_flag(&mut self, flag: Self::ExprFlag);
-    fn peek_expr_flag(&mut self) -> Option<Self::ExprFlag> {
-        let flag = self.take_expr_flag();
-        if let Some(f) = flag {
-            self.set_expr_flag(f);
-        }
-
-        flag
-    }
 }
diff --git a/crates/shared/src/flattened.rs b/crates/shared/src/flattened.rs
@@ -1,7 +1,7 @@
 use crate::{FlatYulExpr, StorageLocation};
 use solang_parser::pt::{Expression, Loc, NamedArgument, Type, YulExpression};
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ExprFlag {
     FunctionName(usize, bool, bool),
     New,
@@ -40,6 +40,7 @@ pub enum FlatExpr {
     Todo(Loc, &'static str),
 
     Emit(Loc),
+    TestCommand(Loc, &'static str),
 
     NamedArgument(Loc, &'static str),
     FunctionCallName(usize, bool, bool),
@@ -325,6 +326,7 @@ impl FlatExpr {
             | Continue(loc, ..)
             | Break(loc, ..)
             | Return(loc, ..)
+            | TestCommand(loc, ..)
             | PostIncrement(loc, ..)
             | PostDecrement(loc, ..)
             | New(loc, ..)