Add NodeManagerScopes to fix use-after-free issues (#4768)
authorAndres Noetzli <andres.noetzli@gmail.com>
Fri, 17 Jul 2020 22:25:54 +0000 (15:25 -0700)
committerGitHub <noreply@github.com>
Fri, 17 Jul 2020 22:25:54 +0000 (15:25 -0700)
This commit fixes our current ASan issues. Some methods in `NodeManager`
were not creating a `NodeManagerScope` for `this` but were indirectly
calling methods that get the `NodeManager` from the current scope, so we
ended up calling methods on a `NodeManager` that had already been
destroyed.

src/expr/node_manager.cpp
src/expr/node_manager.h

index c68b0df86aede23543fc204b88d86629c9bb35df..e9f56bf3fdb59ccbbfc7c6aa180ab1cb2abe376a 100644 (file)
@@ -106,6 +106,10 @@ NodeManager::NodeManager(ExprManager* exprManager)
 }
 
 void NodeManager::init() {
+  // `mkConst()` indirectly needs the correct NodeManager in scope because we
+  // call `NodeValue::inc()` which uses `NodeManager::curentNM()`
+  NodeManagerScope nms(this);
+
   poolInsert( &expr::NodeValue::null() );
 
   for(unsigned i = 0; i < unsigned(kind::LAST_KIND); ++i) {
index 1a28a16eb2b85587832fac6f91f7a64dd7e1bedf..84c4b44e08aa0e4c0c0d185a9a30b79434577c43 100644 (file)
@@ -1484,6 +1484,9 @@ TypeNode NodeManager::mkTypeConst(const T& val) {
 
 template <class NodeClass, class T>
 NodeClass NodeManager::mkConstInternal(const T& val) {
+  // This method indirectly calls `NodeValue::inc()`, which relies on having
+  // the correct `NodeManager` in scope.
+  NodeManagerScope nms(this);
 
   // typedef typename kind::metakind::constantMap<T>::OwningTheory theory_t;
   NVStorage<1> nvStorage;