Skip to content

Commit 94bb78d

Browse files
Jaesoo Leefacebook-github-bot
authored andcommitted
fixing flaky unit tests NvmItemTest
Summary: This change fixes several flaky tests cases in NvmItemTest; e.g., for NvmItemTest.TotalSize, we were hitting unsigned overflow error for getting total size (i.e., NvmItem::totalSize) when numBlobs_ is 0 (e.g., https://fburl.com/testinfra/h7trhoi3). NvmItem.h:129:53: runtime error: addition of unsigned offset to 0x60300001f942 overflowed to 0x60300001f93a This is a test case error and this change fixes it. This change also fixes multiple instances of buffer overflow errors in constructing NvmItem; NvmItem buffer should include sizeof(NvmItem) itself in addition to the blob sizes. Reviewed By: therealgymmy Differential Revision: D49323745 fbshipit-source-id: fda63a00b472e328b1e1d8afab291d00a461cc86
1 parent 4af5e24 commit 94bb78d

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

cachelib/allocator/nvmcache/tests/NvmItemTests.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ TEST(NvmItemTest, SingleBlob) {
3939
uint32_t origSize = 5;
4040
Blob blob{origSize, data};
4141
size_t bufSize = NvmItem::estimateVariableSize(blob);
42-
auto nvmItem = std::unique_ptr<NvmItem>(new (bufSize) NvmItem(1, 1, 1, blob));
42+
auto nvmItem = std::unique_ptr<NvmItem>(new (bufSize + sizeof(NvmItem))
43+
NvmItem(1, 1, 1, blob));
4344
ASSERT_EQ(1, nvmItem->getNumBlobs());
4445
ASSERT_EQ(data, nvmItem->getBlob(0).data);
4546
ASSERT_EQ(origSize, nvmItem->getBlob(0).origAllocSize);
@@ -48,7 +49,7 @@ TEST(NvmItemTest, SingleBlob) {
4849
}
4950

5051
TEST(NvmItemTest, MultipleBlobs) {
51-
int nBlobs = folly::Random::rand32(0, 100);
52+
int nBlobs = folly::Random::rand32(1, 100);
5253
std::vector<Blob> blobs;
5354
std::vector<std::string> strings;
5455
const uint32_t extra = 10;
@@ -60,8 +61,8 @@ TEST(NvmItemTest, MultipleBlobs) {
6061
}
6162

6263
size_t bufSize = NvmItem::estimateVariableSize(blobs);
63-
auto nvmItem =
64-
std::unique_ptr<NvmItem>(new (bufSize) NvmItem(1, 1, 1, blobs));
64+
auto nvmItem = std::unique_ptr<NvmItem>(new (bufSize + sizeof(NvmItem))
65+
NvmItem(1, 1, 1, blobs));
6566

6667
ASSERT_EQ(nBlobs, nvmItem->getNumBlobs());
6768
for (int i = 0; i < nBlobs; i++) {
@@ -73,7 +74,8 @@ TEST(NvmItemTest, MultipleBlobs) {
7374
}
7475

7576
TEST(NvmItemTest, TotalSize) {
76-
int nBlobs = folly::Random::rand32(0, 100);
77+
// nBlobs should be greater than 0
78+
int nBlobs = folly::Random::rand32(1, 100);
7779
std::vector<Blob> blobs;
7880
std::vector<std::string> strings;
7981
for (int i = 0; i < nBlobs; i++) {
@@ -85,14 +87,14 @@ TEST(NvmItemTest, TotalSize) {
8587
}
8688

8789
size_t bufSize = NvmItem::estimateVariableSize(blobs);
88-
auto nvmItem =
89-
std::unique_ptr<NvmItem>(new (bufSize) NvmItem(1, 1, 1, blobs));
90+
auto nvmItem = std::unique_ptr<NvmItem>(new (bufSize + sizeof(NvmItem))
91+
NvmItem(1, 1, 1, blobs));
9092

9193
ASSERT_EQ(bufSize + sizeof(NvmItem), nvmItem->totalSize());
9294
}
9395

9496
TEST(NvmItemTest, MultipleBlobsOverFlow) {
95-
int nBlobs = folly::Random::rand32(0, 100);
97+
int nBlobs = folly::Random::rand32(1, 100);
9698
std::vector<Blob> blobs;
9799
std::vector<std::string> strings;
98100
const uint32_t extra = 10;
@@ -110,7 +112,8 @@ TEST(NvmItemTest, MultipleBlobsOverFlow) {
110112
folly::StringPiece{buf.get(), maxLen}});
111113

112114
size_t bufSize = NvmItem::estimateVariableSize(blobs);
113-
ASSERT_THROW(std::unique_ptr<NvmItem>(new (bufSize) NvmItem(1, 1, 1, blobs)),
115+
ASSERT_THROW(std::unique_ptr<NvmItem>(new (bufSize + sizeof(NvmItem))
116+
NvmItem(1, 1, 1, blobs)),
114117
std::out_of_range);
115118
}
116119

@@ -121,7 +124,8 @@ TEST(NvmItemTest, SingleBlobOverflow) {
121124
folly::StringPiece{buf.get(), maxLen}};
122125

123126
size_t bufSize = NvmItem::estimateVariableSize(blob);
124-
ASSERT_THROW(std::unique_ptr<NvmItem>(new (bufSize) NvmItem(1, 1, 1, blob)),
127+
ASSERT_THROW(std::unique_ptr<NvmItem>(new (bufSize + sizeof(NvmItem))
128+
NvmItem(1, 1, 1, blob)),
125129
std::out_of_range)
126130
<< maxLen;
127131
}

0 commit comments

Comments
 (0)