From e860f8840a9e0b7bf3df8aff1d96b20df983223b Mon Sep 17 00:00:00 2001 From: sdong Date: Thu, 9 Apr 2020 11:20:33 -0700 Subject: [PATCH] Fix memory corruption caused by new test in options_settable_test (#6676) Summary: https://github.com/facebook/rocksdb/pull/6668 added some new test code but it has a risk of memory corruption. Fix it Pull Request resolved: https://github.com/facebook/rocksdb/pull/6676 Test Plan: Run the test under ASAN and see it passes. Reviewed By: ajkr Differential Revision: D20937108 fbshipit-source-id: 22cc96bb02030df0a37a02e67a2cc37ca31ba22d --- options/options_settable_test.cc | 36 +++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 94ca5be1e..55d144d26 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -511,27 +511,39 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { sizeof(std::vector)}, }; - // Construct two pieces of memory with controlled base. This is needed as - // otherwise padding bytes might not be filled. + // For all memory used for options, pre-fill every char. Otherwise, the + // padding bytes might be different so that byte-wise comparison doesn't + // general equal results even if objects are equal. + const char kMySpecialChar = 'x'; char* mcfo1_ptr = new char[sizeof(MutableCFOptions)]; - MutableCFOptions* mcfo1 = new (mcfo1_ptr) MutableCFOptions(); FillWithSpecialChar(mcfo1_ptr, sizeof(MutableCFOptions), - kMutableCFOptionsBlacklist, 'x'); + kMutableCFOptionsBlacklist, kMySpecialChar); char* mcfo2_ptr = new char[sizeof(MutableCFOptions)]; - MutableCFOptions* mcfo2 = new (mcfo2_ptr) MutableCFOptions(); FillWithSpecialChar(mcfo2_ptr, sizeof(MutableCFOptions), - kMutableCFOptionsBlacklist, 'x'); + kMutableCFOptionsBlacklist, kMySpecialChar); + // A clean column family options is constructed after filling the same special + // char as the initial one. So that the padding bytes are the same. + char* cfo_clean_ptr = new char[sizeof(ColumnFamilyOptions)]; + FillWithSpecialChar(cfo_clean_ptr, sizeof(ColumnFamilyOptions), + kColumnFamilyOptionsBlacklist); rnd_filled_options.num_levels = 66; - *mcfo1 = MutableCFOptions(rnd_filled_options); - ColumnFamilyOptions cfo_back = - BuildColumnFamilyOptions(ColumnFamilyOptions(), *mcfo1); - *mcfo2 = MutableCFOptions(cfo_back); + ColumnFamilyOptions* cfo_clean = new (cfo_clean_ptr) ColumnFamilyOptions(); + + MutableCFOptions* mcfo1 = + new (mcfo1_ptr) MutableCFOptions(rnd_filled_options); + ColumnFamilyOptions cfo_back = BuildColumnFamilyOptions(*cfo_clean, *mcfo1); + MutableCFOptions* mcfo2 = new (mcfo2_ptr) MutableCFOptions(cfo_back); ASSERT_TRUE(CompareBytes(mcfo1_ptr, mcfo2_ptr, sizeof(MutableCFOptions), kMutableCFOptionsBlacklist)); - delete[] mcfo1; - delete[] mcfo2; + + cfo_clean->~ColumnFamilyOptions(); + mcfo1->~MutableCFOptions(); + mcfo2->~MutableCFOptions(); + delete[] mcfo1_ptr; + delete[] mcfo2_ptr; + delete[] cfo_clean_ptr; } #endif // !__clang__ #endif // OS_LINUX || OS_WIN