Commit 1c15b5bb authored by Giuseppe Ottaviano's avatar Giuseppe Ottaviano Committed by Facebook Github Bot 9

Fix warning in MicroLock initialization

Summary:The `init()` function uses the previous value of `lock_`, but that is
uninitialized and the compiler can issue a warning about it. It is
also potentially undefined behavior because it is not guaranteed that
the address of `lock_` is taken before `init()` (in which case the it
would be just an indeterminate value).

Since it is not very useful to initialize only one slot and leave the
others uninitialized, we can just have a single `init()` that
zero-initializes all the slots.

Reviewed By: dcolascione

Differential Revision: D3042629

fb-gh-sync-id: de1633b02eb1c891e310f2d5d2cfc5376cd41d5f
shipit-source-id: de1633b02eb1c891e310f2d5d2cfc5376cd41d5f
parent b29f38a4
...@@ -102,8 +102,8 @@ class MicroLockCore { ...@@ -102,8 +102,8 @@ class MicroLockCore {
public: public:
inline void unlock(unsigned slot); inline void unlock(unsigned slot);
inline void unlock() { unlock(0); } inline void unlock() { unlock(0); }
inline void init(unsigned slot) { lock_ &= ~(3U << (2 * slot)); } // Initializes all the slots.
inline void init() { init(0); } inline void init() { lock_ = 0; }
}; };
inline detail::Futex<>* MicroLockCore::word() const { inline detail::Futex<>* MicroLockCore::word() const {
......
...@@ -211,7 +211,7 @@ struct SimpleBarrier { ...@@ -211,7 +211,7 @@ struct SimpleBarrier {
}; };
} }
static void runMicroLockTest() { TEST(SmallLocks, MicroLock) {
volatile uint64_t counters[4] = {0, 0, 0, 0}; volatile uint64_t counters[4] = {0, 0, 0, 0};
std::vector<std::thread> threads; std::vector<std::thread> threads;
static const unsigned nrThreads = 20; static const unsigned nrThreads = 20;
...@@ -225,10 +225,7 @@ static void runMicroLockTest() { ...@@ -225,10 +225,7 @@ static void runMicroLockTest() {
struct { struct {
uint8_t a; uint8_t a;
volatile uint8_t b; volatile uint8_t b;
union {
MicroLock alock; MicroLock alock;
uint8_t c;
};
volatile uint8_t d; volatile uint8_t d;
} x; } x;
...@@ -237,7 +234,7 @@ static void runMicroLockTest() { ...@@ -237,7 +234,7 @@ static void runMicroLockTest() {
x.a = 'a'; x.a = 'a';
x.b = origB; x.b = origB;
x.c = 0; x.alock.init();
x.d = origD; x.d = origD;
// This thread touches other parts of the host word to show that // This thread touches other parts of the host word to show that
...@@ -282,17 +279,16 @@ static void runMicroLockTest() { ...@@ -282,17 +279,16 @@ static void runMicroLockTest() {
EXPECT_EQ(x.a, 'a'); EXPECT_EQ(x.a, 'a');
EXPECT_EQ(x.b, (uint8_t)(origB + iterPerThread / 2)); EXPECT_EQ(x.b, (uint8_t)(origB + iterPerThread / 2));
EXPECT_EQ(x.c, 0);
EXPECT_EQ(x.d, (uint8_t)(origD + iterPerThread / 2)); EXPECT_EQ(x.d, (uint8_t)(origD + iterPerThread / 2));
for (unsigned i = 0; i < 4; ++i) { for (unsigned i = 0; i < 4; ++i) {
EXPECT_EQ(counters[i], ((uint64_t)nrThreads * iterPerThread) / 4); EXPECT_EQ(counters[i], ((uint64_t)nrThreads * iterPerThread) / 4);
} }
} }
TEST(SmallLocks, MicroLock) { runMicroLockTest(); }
TEST(SmallLocks, MicroLockTryLock) { TEST(SmallLocks, MicroLockTryLock) {
MicroLock lock; MicroLock lock;
lock.init(); lock.init();
EXPECT_TRUE(lock.try_lock()); EXPECT_TRUE(lock.try_lock());
EXPECT_FALSE(lock.try_lock()); EXPECT_FALSE(lock.try_lock());
lock.unlock();
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment