datarace: factor subtle clock-vector logic into a reusable function

These comparisons are performed many times throughout datarace.cc, featuring a
lot of code duplication without strong documentation. This factors the
duplicated code into a single function and documents that single function more
clearly.

Brian Demsky: please review this fix (it should make no actual logical change)
and take a look at the TODO I have inserted; I'm not sure if the TODO is
warranted.

diff --git a/datarace.cc b/datarace.cc
index da9a3fc15c9716a5a0ec7320bf23bccfb5445ef1..1040a3b0df2d4871f21515e62d2d94e5a6651313 100644 (file)
--- a/datarace.cc
+++ b/datarace.cc
@@ -25,6 +25,21 @@ static uint64_t * lookupAddressEntry(void * address) {
        return &basetable->array[((uintptr_t)address)&MASK16BIT];
 }
 
        return &basetable->array[((uintptr_t)address)&MASK16BIT];
 }
 

+/**
+ * Compares a current clock-vector/thread-ID pair with a clock/thread-ID pair
+ * to check the potential for a data race.
+ * @param clock1 The current clock vector
+ * @param tid1 The current thread; paired with clock1
+ * @param clock2 The clock value for the potentially-racing action
+ * @param tid2 The thread ID for the potentially-racing action
+ * @return true if the current clock allows a race with the event at clock2/tid2
+ */
+static bool clock_may_race(ClockVector *clock1, thread_id_t tid1,
+                           modelclock_t clock2, thread_id_t tid2)
+{
+       return tid1 != tid2 && clock2 != 0 && clock1->getClock(tid2) <= clock2;
+}
+

 static void expandRecord(uint64_t * shadow) {
        uint64_t shadowval=*shadow;
 
 static void expandRecord(uint64_t * shadow) {
        uint64_t shadowval=*shadow;
 


@@ -61,7 +76,7 @@ void fullRaceCheckWrite(thread_id_t thread, uint64_t * shadow, ClockVector *curr
                modelclock_t readClock = record->readClock[i];
                thread_id_t readThread = record->thread[i];
 
                modelclock_t readClock = record->readClock[i];
                thread_id_t readThread = record->thread[i];
 

-               if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+               if (clock_may_race(currClock, thread, readClock, readThread)) {

                        /* We have a datarace */
                        reportDataRace();
                }
                        /* We have a datarace */
                        reportDataRace();
                }


@@ -72,7 +87,7 @@ void fullRaceCheckWrite(thread_id_t thread, uint64_t * shadow, ClockVector *curr
        modelclock_t writeClock = record->writeClock;
        thread_id_t writeThread = record->writeThread;
 
        modelclock_t writeClock = record->writeClock;
        thread_id_t writeThread = record->writeThread;
 

-       if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+       if (clock_may_race(currClock, thread, writeClock, writeThread)) {

                /* We have a datarace */
                reportDataRace();
        }
                /* We have a datarace */
                reportDataRace();
        }


@@ -108,7 +123,7 @@ void raceCheckWrite(thread_id_t thread, void *location, ClockVector *currClock)
        modelclock_t readClock = READVECTOR(shadowval);
        thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 
        modelclock_t readClock = READVECTOR(shadowval);
        thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 

-       if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+       if (clock_may_race(currClock, thread, readClock, readThread)) {

                /* We have a datarace */
                reportDataRace();
        }
                /* We have a datarace */
                reportDataRace();
        }


@@ -118,7 +133,7 @@ void raceCheckWrite(thread_id_t thread, void *location, ClockVector *currClock)
        modelclock_t writeClock = WRITEVECTOR(shadowval);
        thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 
        modelclock_t writeClock = WRITEVECTOR(shadowval);
        thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 

-       if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+       if (clock_may_race(currClock, thread, writeClock, writeThread)) {

                /* We have a datarace */
                reportDataRace();
        }
                /* We have a datarace */
                reportDataRace();
        }


@@ -133,7 +148,7 @@ void fullRaceCheckRead(thread_id_t thread, uint64_t * shadow, ClockVector *currC
        modelclock_t writeClock = record->writeClock;
        thread_id_t writeThread = record->writeThread;
 
        modelclock_t writeClock = record->writeClock;
        thread_id_t writeThread = record->writeThread;
 

-       if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+       if (clock_may_race(currClock, thread, writeClock, writeThread)) {

                /* We have a datarace */
                reportDataRace();
        }
                /* We have a datarace */
                reportDataRace();
        }


@@ -146,6 +161,8 @@ void fullRaceCheckRead(thread_id_t thread, uint64_t * shadow, ClockVector *currC
                modelclock_t readClock = record->readClock[i];
                thread_id_t readThread = record->thread[i];
 
                modelclock_t readClock = record->readClock[i];
                thread_id_t readThread = record->thread[i];
 

+               /* TODO: should this have different logic than all the other
+                * 'clock_may_race' calls? */

                if (readThread != thread && currClock->getClock(readThread) <= readClock) {
                        /* Still need this read in vector */
                        if (copytoindex!=i) {
                if (readThread != thread && currClock->getClock(readThread) <= readClock) {
                        /* Still need this read in vector */
                        if (copytoindex!=i) {


@@ -201,7 +218,7 @@ void raceCheckRead(thread_id_t thread, void *location, ClockVector *currClock) {
        modelclock_t writeClock = WRITEVECTOR(shadowval);
        thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 
        modelclock_t writeClock = WRITEVECTOR(shadowval);
        thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 

-       if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+       if (clock_may_race(currClock, thread, writeClock, writeThread)) {

                /* We have a datarace */
                reportDataRace();
        }
                /* We have a datarace */
                reportDataRace();
        }


@@ -209,7 +226,7 @@ void raceCheckRead(thread_id_t thread, void *location, ClockVector *currClock) {
        modelclock_t readClock = READVECTOR(shadowval);
        thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 
        modelclock_t readClock = READVECTOR(shadowval);
        thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 

-       if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+       if (clock_may_race(currClock, thread, readClock, readThread)) {

                /* We don't subsume this read... Have to expand record. */
                expandRecord(shadow);
                fullRaceCheckRead(thread, shadow, currClock);
                /* We don't subsume this read... Have to expand record. */
                expandRecord(shadow);
                fullRaceCheckRead(thread, shadow, currClock);