model: bugfix - w_modification_order handling current action
[model-checker.git] / model.cc
index cdce1028c141b914dcf4f0d7623df3c452eeb461..483e83232ffb2ec7272e1eae1c7283af3b4eac84 100644 (file)
--- a/model.cc
+++ b/model.cc
@@ -265,11 +265,11 @@ ModelAction * ModelChecker::get_next_backtrack()
 /**
  * Processes a read or rmw model action.
  * @param curr is the read model action to process.
- * @param th is the thread
  * @param second_part_of_rmw is boolean that is true is this is the second action of a rmw.
  * @return True if processing this read updates the mo_graph.
  */
-bool ModelChecker::process_read(ModelAction *curr, Thread * th, bool second_part_of_rmw) {
+bool ModelChecker::process_read(ModelAction *curr, bool second_part_of_rmw)
+{
        uint64_t value;
        bool updated = false;
        while (true) {
@@ -281,7 +281,7 @@ bool ModelChecker::process_read(ModelAction *curr, Thread * th, bool second_part
                        bool r_status = false;
 
                        if (!second_part_of_rmw) {
-                               check_recency(curr,false);
+                               check_recency(curr);
                                r_status = r_modification_order(curr, reads_from);
                        }
 
@@ -303,11 +303,36 @@ bool ModelChecker::process_read(ModelAction *curr, Thread * th, bool second_part
                        Promise *valuepromise = new Promise(curr, value, expiration);
                        promises->push_back(valuepromise);
                }
-               th->set_return_value(value);
+               get_thread(curr)->set_return_value(value);
                return updated;
        }
 }
 
+/**
+ * Process a write ModelAction
+ * @param curr The ModelAction to process
+ * @return True if the mo_graph was updated or promises were resolved
+ */
+bool ModelChecker::process_write(ModelAction *curr)
+{
+       bool updated_mod_order = w_modification_order(curr);
+       bool updated_promises = resolve_promises(curr);
+
+       if (promises->size() == 0) {
+               for (unsigned int i = 0; i<futurevalues->size(); i++) {
+                       struct PendingFutureValue pfv = (*futurevalues)[i];
+                       if (pfv.act->get_node()->add_future_value(pfv.value, pfv.expiration) &&
+                                       (!priv->next_backtrack || *pfv.act > *priv->next_backtrack))
+                               priv->next_backtrack = pfv.act;
+               }
+               futurevalues->resize(0);
+       }
+
+       mo_graph->commitChanges();
+       get_thread(curr)->set_return_value(VALUE_NONE);
+       return updated_mod_order || updated_promises;
+}
+
 /**
  * This is the heart of the model checker routine. It performs model-checking
  * actions corresponding to a given "current action." Among other processes, it
@@ -395,37 +420,40 @@ Thread * ModelChecker::check_current_action(ModelAction *curr)
                break;
        }
 
-       bool updated = false;
+       /* Add current action to lists before work_queue loop */
+       if (!second_part_of_rmw)
+               add_action_to_lists(curr);
 
-       if (curr->is_read()) {
-               updated = process_read(curr, get_thread(curr), second_part_of_rmw);
-       }
+       work_queue_t work_queue(1, CheckCurrWorkEntry(curr));
 
-       if (curr->is_write()) {
-               bool updated_mod_order = w_modification_order(curr);
-               bool updated_promises = resolve_promises(curr);
-               updated = updated || updated_mod_order || updated_promises;
-
-               if (promises->size()==0) {
-                       for (unsigned int i = 0; i<futurevalues->size(); i++) {
-                               struct PendingFutureValue pfv=(*futurevalues)[i];
-                               if (pfv.act->get_node()->add_future_value(pfv.value, pfv.expiration) &&
-                                               (!priv->next_backtrack || *pfv.act > *priv->next_backtrack))
-                                       priv->next_backtrack = pfv.act;
-                       }
-                       futurevalues->resize(0);
-               }
+       while (!work_queue.empty()) {
+               WorkQueueEntry work = work_queue.front();
+               work_queue.pop_front();
 
-               mo_graph->commitChanges();
-               get_thread(curr)->set_return_value(VALUE_NONE);
-       }
+               switch (work.type) {
+               case WORK_CHECK_CURR_ACTION: {
+                       ModelAction *act = work.action;
+                       bool updated = false;
+                       if (act->is_read() && process_read(act, second_part_of_rmw))
+                               updated = true;
 
-       if (updated)
-               resolve_release_sequences(curr->get_location());
+                       if (act->is_write() && process_write(act))
+                               updated = true;
 
-       /* Add action to list.  */
-       if (!second_part_of_rmw)
-               add_action_to_lists(curr);
+                       if (updated)
+                               work_queue.push_back(CheckRelSeqWorkEntry(act->get_location()));
+                       break;
+               }
+               case WORK_CHECK_RELEASE_SEQ:
+                       resolve_release_sequences(work.location, &work_queue);
+                       break;
+               case WORK_CHECK_MO_EDGES:
+                       /** @todo Perform follow-up mo_graph checks */
+               default:
+                       ASSERT(false);
+                       break;
+               }
+       }
 
        check_curr_backtracking(curr);
 
@@ -503,7 +531,7 @@ ModelAction * ModelChecker::process_rmw(ModelAction *act) {
  *
  * If so, we decide that the execution is no longer feasible.
  */
-void ModelChecker::check_recency(ModelAction *curr, bool already_added) {
+void ModelChecker::check_recency(ModelAction *curr) {
        if (params.maxreads != 0) {
                if (curr->get_node()->get_read_from_size() <= 1)
                        return;
@@ -524,12 +552,10 @@ void ModelChecker::check_recency(ModelAction *curr, bool already_added) {
 
                action_list_t::reverse_iterator rit = list->rbegin();
                /* Skip past curr */
-               if (already_added) {
-                       for (; (*rit) != curr; rit++)
-                               ;
-                       /* go past curr now */
-                       rit++;
-               }
+               for (; (*rit) != curr; rit++)
+                       ;
+               /* go past curr now */
+               rit++;
 
                action_list_t::reverse_iterator ritcopy = rit;
                //See if we have enough reads from the same value
@@ -620,10 +646,13 @@ bool ModelChecker::r_modification_order(ModelAction *curr, const ModelAction *rf
                for (rit = list->rbegin(); rit != list->rend(); rit++) {
                        ModelAction *act = *rit;
 
-                       /* Include at most one act per-thread that "happens before" curr */
-                       if (act->happens_before(curr)) {
+                       /*
+                        * Include at most one act per-thread that "happens
+                        * before" curr. Don't consider reflexively.
+                        */
+                       if (act->happens_before(curr) && act != curr) {
                                if (act->is_write()) {
-                                       if (rf != act && act != curr) {
+                                       if (rf != act) {
                                                mo_graph->addEdge(act, rf);
                                                added = true;
                                        }
@@ -634,7 +663,6 @@ bool ModelChecker::r_modification_order(ModelAction *curr, const ModelAction *rf
                                                added = true;
                                        }
                                }
-
                                break;
                        }
                }
@@ -727,7 +755,7 @@ bool ModelChecker::w_modification_order(ModelAction *curr)
        if (curr->is_seqcst()) {
                /* We have to at least see the last sequentially consistent write,
                         so we are initialized. */
-               ModelAction *last_seq_cst = get_last_seq_cst(curr->get_location());
+               ModelAction *last_seq_cst = get_last_seq_cst(curr);
                if (last_seq_cst != NULL) {
                        mo_graph->addEdge(last_seq_cst, curr);
                        added = true;
@@ -741,8 +769,23 @@ bool ModelChecker::w_modification_order(ModelAction *curr)
                action_list_t::reverse_iterator rit;
                for (rit = list->rbegin(); rit != list->rend(); rit++) {
                        ModelAction *act = *rit;
+                       if (act == curr) {
+                               /*
+                                * If RMW, we already have all relevant edges,
+                                * so just skip to next thread.
+                                * If normal write, we need to look at earlier
+                                * actions, so continue processing list.
+                                */
+                               if (curr->is_rmw())
+                                       break;
+                               else
+                                       continue;
+                       }
 
-                       /* Include at most one act per-thread that "happens before" curr */
+                       /*
+                        * Include at most one act per-thread that "happens
+                        * before" curr
+                        */
                        if (act->happens_before(curr)) {
                                /*
                                 * Note: if act is RMW, just add edge:
@@ -750,11 +793,9 @@ bool ModelChecker::w_modification_order(ModelAction *curr)
                                 * The following edge should be handled elsewhere:
                                 *   readfrom(act) --mo--> act
                                 */
-                               if (act->is_write()) {
-                                       //RMW shouldn't have an edge to themselves
-                                       if (act!=curr)
-                                               mo_graph->addEdge(act, curr);
-                               } else if (act->is_read() && act->get_reads_from() != NULL)
+                               if (act->is_write())
+                                       mo_graph->addEdge(act, curr);
+                               else if (act->is_read() && act->get_reads_from() != NULL)
                                        mo_graph->addEdge(act->get_reads_from(), curr);
                                added = true;
                                break;
@@ -954,9 +995,12 @@ void ModelChecker::get_release_seq_heads(ModelAction *act,
  *
  * @param location The location/object that should be checked for release
  * sequence resolutions
- * @return True if any updates occurred (new synchronization, new mo_graph edges)
+ * @param work_queue The work queue to which to add work items as they are
+ * generated
+ * @return True if any updates occurred (new synchronization, new mo_graph
+ * edges)
  */
-bool ModelChecker::resolve_release_sequences(void *location)
+bool ModelChecker::resolve_release_sequences(void *location, work_queue_t *work_queue)
 {
        std::list<ModelAction *> *list;
        list = lazy_sync_with_release->getptr(location);
@@ -979,14 +1023,18 @@ bool ModelChecker::resolve_release_sequences(void *location)
                }
 
                if (updated) {
+                       /* Re-check act for mo_graph edges */
+                       work_queue->push_back(MOEdgeWorkEntry(act));
+
                        /* propagate synchronization to later actions */
                        action_list_t::reverse_iterator it = action_trace->rbegin();
                        while ((*it) != act) {
                                ModelAction *propagate = *it;
-                               if (act->happens_before(propagate))
-                                       /** @todo new mo_graph edges along with
-                                        * this synchronization? */
+                               if (act->happens_before(propagate)) {
                                        propagate->synchronize_with(act);
+                                       /* Re-check 'propagate' for mo_graph edges */
+                                       work_queue->push_back(MOEdgeWorkEntry(propagate));
+                               }
                        }
                }
                if (complete) {
@@ -1037,18 +1085,21 @@ ModelAction * ModelChecker::get_last_action(thread_id_t tid)
 }
 
 /**
- * Gets the last memory_order_seq_cst action (in the total global sequence)
- * performed on a particular object (i.e., memory location).
- * @param location The object location to check
- * @return The last seq_cst action performed
+ * Gets the last memory_order_seq_cst write (in the total global sequence)
+ * performed on a particular object (i.e., memory location), not including the
+ * current action.
+ * @param curr The current ModelAction; also denotes the object location to
+ * check
+ * @return The last seq_cst write
  */
-ModelAction * ModelChecker::get_last_seq_cst(const void *location)
+ModelAction * ModelChecker::get_last_seq_cst(ModelAction *curr)
 {
+       void *location = curr->get_location();
        action_list_t *list = obj_map->get_safe_ptr(location);
        /* Find: max({i in dom(S) | seq_cst(t_i) && isWrite(t_i) && samevar(t_i, t)}) */
        action_list_t::reverse_iterator rit;
        for (rit = list->rbegin(); rit != list->rend(); rit++)
-               if ((*rit)->is_write() && (*rit)->is_seqcst())
+               if ((*rit)->is_write() && (*rit)->is_seqcst() && (*rit) != curr)
                        return *rit;
        return NULL;
 }
@@ -1162,7 +1213,7 @@ void ModelChecker::build_reads_from_past(ModelAction *curr)
        bool initialized = false;
 
        if (curr->is_seqcst()) {
-               last_seq_cst = get_last_seq_cst(curr->get_location());
+               last_seq_cst = get_last_seq_cst(curr);
                /* We have to at least see the last sequentially consistent write,
                         so we are initialized. */
                if (last_seq_cst != NULL)