added undefined san test

twhuang-utah · twhuang-utah · commit fd5a54feaf6b · 2021-09-06T15:12:54.000-06:00
diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
@@ -25,8 +25,19 @@ jobs:
     - name: test
       run: cd build ; ctest -j 10 --output-on-failure
   
+  undefined-test:
+    runs-on: macos-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: cmake
+      run: cmake -S . -B build -DCMAKE_CXX_FLAGS="-fsanitize=undefined -g"
+    - name: build
+      run: cmake --build build --parallel 10
+    - name: test
+      run: cd build ; ctest -j 10 --output-on-failure
+  
   leak-test:
-    runs-on: ubuntu-latest
+    runs-on: macos-latest
     steps:
     - uses: actions/checkout@v2
     - name: cmake
diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml
@@ -36,6 +36,17 @@ jobs:
     - name: test
       run: cd build ; ctest -j 10 --output-on-failure
   
+  undefined-test:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: cmake
+      run: cmake -S . -B build -DCMAKE_CXX_FLAGS="-fsanitize=undefined -g"
+    - name: build
+      run: cmake --build build --parallel 10
+    - name: test
+      run: cd build ; ctest -j 10 --output-on-failure
+  
   tsan-test:
     runs-on: ubuntu-latest
     steps:
@@ -46,3 +57,4 @@ jobs:
       run: cmake --build build --parallel 10
     - name: test
       run: cd build ; ctest -j 10 --output-on-failure
+
diff --git a/taskflow/core/algorithm/for_each.hpp b/taskflow/core/algorithm/for_each.hpp
@@ -2,7 +2,6 @@
 // - gomp: https://github.com/gcc-mirror/gcc/blob/master/libgomp/iter.c
 // - komp: https://github.com/llvm-mirror/openmp/blob/master/runtime/src/kmp_dispatch.cpp
 
-
 #pragma once
 
 #include "../executor.hpp"
diff --git a/taskflow/core/executor.hpp b/taskflow/core/executor.hpp
@@ -270,9 +270,6 @@ class Executor {
 
   private:
 
-    //inline static thread_local PerThread _per_thread;
-    //inline static thread_local Worker* _this_worker {nullptr};
-
     std::condition_variable _topology_cv;
     std::mutex _taskflow_mutex;
     std::mutex _topology_mutex;
@@ -365,9 +362,6 @@ inline Executor::~Executor() {
   for(auto& t : _threads){
     t.join();
   } 
-  
-  // flush the default observer
-  //_flush_tfprof();
 }
 
 // Function: num_workers
@@ -440,7 +434,6 @@ void Executor::silent_async(F&& f, ArgsT&&... args) {
 
 // Function: this_worker_id
 inline int Executor::this_worker_id() const {
-  //auto worker = _per_thread.worker;
   Worker* worker = this_worker().worker;
   return worker ? static_cast<int>(worker->_id) : -1;
 }
@@ -456,7 +449,6 @@ inline void Executor::_spawn(size_t N) {
     
     _threads.emplace_back([this] (Worker& w) -> void {
 
-      //_per_thread.worker = &w;
       this_worker().worker = &w;
 
       Node* t = nullptr;
@@ -641,7 +633,6 @@ inline void Executor::_schedule(Node* node) {
   node->_state.fetch_or(Node::READY, std::memory_order_release);
 
   // caller is a worker to this pool
-  //auto worker = _per_thread.worker;
   auto worker = this_worker().worker;
 
   if(worker != nullptr && worker->_executor == this) {
@@ -679,7 +670,6 @@ inline void Executor::_schedule(const std::vector<Node*>& nodes) {
   }
 
   // worker thread
-  //auto worker = _per_thread.worker;
   auto worker = this_worker().worker;
 
   if(worker != nullptr && worker->_executor == this) {
@@ -921,7 +911,6 @@ inline void Executor::_invoke_dynamic_task(Worker& w, Node* node) {
 // Procedure: _invoke_dynamic_task_external
 inline void Executor::_invoke_dynamic_task_external(Node*p, Graph& g, bool detach) {
 
-  //auto worker = _per_thread.worker;
   auto worker = this_worker().worker;
 
   //assert(worker && worker->_executor == this);
@@ -1206,7 +1195,6 @@ inline void Executor::_set_up_topology(Topology* tpg) {
   for(auto node : tpg->_taskflow._graph._nodes) {
     
     node->_topology = tpg;
-    //node->_clear_state();
     node->_state.store(0, std::memory_order_relaxed);
 
     if(node->num_dependents() == 0) {
@@ -1230,10 +1218,9 @@ inline void Executor::_tear_down_topology(Topology* tpg) {
   // case 1: we still need to run the topology again
   if(!tpg->_is_cancelled && !tpg->_pred()) {
     //assert(tpg->_join_counter == 0);
-    f._mutex.lock();
+    std::lock_guard<std::mutex> lock(f._mutex);
     tpg->_join_counter = tpg->_sources.size();
     _schedule(tpg->_sources); 
-    f._mutex.unlock();
   }
   // case 2: the final run of this topology
   else {
@@ -1244,11 +1231,8 @@ inline void Executor::_tear_down_topology(Topology* tpg) {
       tpg->_call();
     }
 
-    f._mutex.lock();
-
     // If there is another run (interleave between lock)
-    if(f._topologies.size() > 1) {
-
+    if(std::unique_lock<std::mutex> lock(f._mutex); f._topologies.size()>1) {
       //assert(tpg->_join_counter == 0);
 
       // Set the promise
@@ -1259,12 +1243,10 @@ inline void Executor::_tear_down_topology(Topology* tpg) {
       // decrement the topology but since this is not the last we don't notify
       _decrement_topology();
       
-      _set_up_topology(tpg);
-      
       // set up topology needs to be under the lock or it can
       // introduce memory order error with pop
-      f._mutex.unlock();
-    }
+      _set_up_topology(tpg);
+    } 
     else {
       //assert(f._topologies.size() == 1);
 
@@ -1283,7 +1265,8 @@ inline void Executor::_tear_down_topology(Topology* tpg) {
       // Now we remove the topology from this taskflow
       f._topologies.pop();
 
-      f._mutex.unlock();
+      //f._mutex.unlock();
+      lock.unlock();
       
       // We set the promise in the end in case taskflow leaves the scope.
       // After set_value, the caller will return from wait
@@ -1298,7 +1281,6 @@ inline void Executor::_tear_down_topology(Topology* tpg) {
         std::scoped_lock<std::mutex> lock(_taskflow_mutex);
         _taskflows.erase(*s);
       } 
-
     }
   }
 }
diff --git a/taskflow/core/flow_builder.hpp b/taskflow/core/flow_builder.hpp
@@ -134,6 +134,9 @@ class FlowBuilder {
     @brief removes a task from a taskflow
 
     @param task task to remove
+
+    Removes a task and its input and output dependencies from this graph. 
+    If the task does not belong to this graph, nothing will happen.
     */
     void erase(Task task);
 
@@ -552,23 +555,22 @@ auto FlowBuilder::emplace(C&&... cs) {
 
 // Function: erase
 inline void FlowBuilder::erase(Task task) {
+
   if (!task._node) {
     return;
   }
 
   task.for_each_dependent([&] (Task dependent) {
-    auto& successors = dependent._node->_successors;
-    auto I = ::std::find(successors.begin(), successors.end(), task._node);
-    if (I != successors.end()) {
-      successors.erase(I);
+    auto& S = dependent._node->_successors;
+    if(auto I = std::find(S.begin(), S.end(), task._node); I != S.end()) {
+      S.erase(I);
     }
   });
 
   task.for_each_successor([&] (Task dependent) {
-    auto& dependents = dependent._node->_dependents;
-    auto I = ::std::find(dependents.begin(), dependents.end(), task._node);
-    if (I != dependents.end()) {
-      dependents.erase(I);
+    auto& D = dependent._node->_dependents;
+    if(auto I = std::find(D.begin(), D.end(), task._node); I != D.end()) {
+      D.erase(I);
     }
   });
   
diff --git a/taskflow/core/graph.hpp b/taskflow/core/graph.hpp
@@ -558,12 +558,10 @@ inline Node* Graph::emplace_back() {
 
 // Function: erase
 inline void Graph::erase(Node* node) {
-  auto I = ::std::find(_nodes.begin(), _nodes.end(), node);
-  if (I != _nodes.end()) {
+  if(auto I = std::find(_nodes.begin(), _nodes.end(), node); I != _nodes.end()) {
     _nodes.erase(I);
+    node_pool.recycle(node);
   }
-
-  node_pool.recycle(node);
 }
 
 }  // end of namespace tf. ---------------------------------------------------

Original file line number	Diff line number	Diff line change
`@@ -558,12 +558,10 @@ inline Node* Graph::emplace_back() {`
`558`	`558`
`559`	`559`	`// Function: erase`
`560`	`560`	`inline void Graph::erase(Node* node) {`
`561`		`- auto I = ::std::find(_nodes.begin(), _nodes.end(), node);`
`562`		`- if (I != _nodes.end()) {`
	`561`	`+ if(auto I = std::find(_nodes.begin(), _nodes.end(), node); I != _nodes.end()) {`
`563`	`562`	`_nodes.erase(I);`
	`563`	`+ node_pool.recycle(node);`
`564`	`564`	`}`
`565`		`-`
`566`		`- node_pool.recycle(node);`
`567`	`565`	`}`
`568`	`566`
`569`	`567`	`} // end of namespace tf. ---------------------------------------------------`