From 1555554a743a69644432215413e0dad83a18a235 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Fri, 6 Jul 2012 11:44:46 -0700 Subject: [PATCH] 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. --- datarace.cc | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/datarace.cc b/datarace.cc index da9a3fc1..1040a3b0 100644 --- a/datarace.cc +++ b/datarace.cc @@ -25,6 +25,21 @@ static uint64_t * lookupAddressEntry(void * address) { 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; @@ -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]; - if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) { + if (clock_may_race(currClock, thread, readClock, readThread)) { /* 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; - if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) { + if (clock_may_race(currClock, thread, writeClock, writeThread)) { /* 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)); - if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) { + if (clock_may_race(currClock, thread, readClock, readThread)) { /* 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)); - if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) { + if (clock_may_race(currClock, thread, writeClock, writeThread)) { /* 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; - if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) { + if (clock_may_race(currClock, thread, writeClock, writeThread)) { /* 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]; + /* 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) { @@ -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)); - if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) { + if (clock_may_race(currClock, thread, writeClock, writeThread)) { /* 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)); - 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); -- 2.34.1