Skip to content

Commit

Permalink
fix address sanitize reported stack-use-after-scope (#1721)
Browse files Browse the repository at this point in the history
* fix address sanitize

Signed-off-by: lizhen6 <[email protected]>

* run unit test in build_macos ci task

Signed-off-by: lizhen6 <[email protected]>

---------

Signed-off-by: lizhen6 <[email protected]>
  • Loading branch information
tedli authored Jul 21, 2023
1 parent 514d7ab commit 8bf77bc
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 46 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/pika.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ jobs:
cmake -B ${{github.workspace}}/build -S . -DCMAKE_C_COMPILER=/usr/local/opt/gcc@10/bin/gcc-10 -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}
cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}}
# - name: Test
# working-directory: ${{github.workspace}}/build
# run: |
# export CC=/usr/local/opt/gcc@10/bin/gcc-10
# ctest -C ${{env.BUILD_TYPE}}
- name: Test
working-directory: ${{github.workspace}}/build
run: |
export CC=/usr/local/opt/gcc@10/bin/gcc-10
ctest -C ${{env.BUILD_TYPE}}
- name: Unit Test
working-directory: ${{github.workspace}}
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ execute_process(COMMAND uname -p OUTPUT_VARIABLE HOST_ARCH)
string(TOLOWER ${HOST_ARCH} HOST_ARCH)

if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE RElEASE)
set(CMAKE_BUILD_TYPE RELEASE)
endif()

string(TOUPPER ${CMAKE_BUILD_TYPE} BUILD_TYPE)
Expand All @@ -50,7 +50,7 @@ elseif(${BUILD_TYPE} STREQUAL MINSIZEREL)
elseif(${BUILD_TYPE} STREQUAL RELWITHDEBINFO)
set(LIB_BUILD_TYPE RELWITHDEBINFO)
else()
set(LIB_BUILD_TYPE RElEASE)
set(LIB_BUILD_TYPE RELEASE)
set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNDEBUG")
endif()

Expand Down
51 changes: 24 additions & 27 deletions src/storage/src/redis_hashes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ Status RedisHashes::HIncrby(const Slice& key, const Slice& field, int64_t value,
std::string meta_value;

Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
char value_buf[32] = {0};
char meta_value_buf[4] = {0};
if (s.ok()) {
ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
if (parsed_hashes_meta_value.IsStale() || parsed_hashes_meta_value.count() == 0) {
Expand All @@ -325,9 +327,8 @@ Status RedisHashes::HIncrby(const Slice& key, const Slice& field, int64_t value,
parsed_hashes_meta_value.set_timestamp(0);
batch.Put(handles_[0], key, meta_value);
HashesDataKey hashes_data_key(key, version, field);
char buf[32];
Int64ToStr(buf, 32, value);
batch.Put(handles_[1], hashes_data_key.Encode(), buf);
Int64ToStr(value_buf, 32, value);
batch.Put(handles_[1], hashes_data_key.Encode(), value_buf);
*ret = value;
} else {
version = parsed_hashes_meta_value.version();
Expand All @@ -342,32 +343,28 @@ Status RedisHashes::HIncrby(const Slice& key, const Slice& field, int64_t value,
return Status::InvalidArgument("Overflow");
}
*ret = ival + value;
char buf[32];
Int64ToStr(buf, 32, *ret);
batch.Put(handles_[1], hashes_data_key.Encode(), buf);
Int64ToStr(value_buf, 32, *ret);
batch.Put(handles_[1], hashes_data_key.Encode(), value_buf);
statistic++;
} else if (s.IsNotFound()) {
char buf[32];
Int64ToStr(buf, 32, value);
Int64ToStr(value_buf, 32, value);
parsed_hashes_meta_value.ModifyCount(1);
batch.Put(handles_[0], key, meta_value);
batch.Put(handles_[1], hashes_data_key.Encode(), buf);
batch.Put(handles_[1], hashes_data_key.Encode(), value_buf);
*ret = value;
} else {
return s;
}
}
} else if (s.IsNotFound()) {
char str[4];
EncodeFixed32(str, 1);
HashesMetaValue hashes_meta_value(std::string(str, sizeof(int32_t)));
EncodeFixed32(meta_value_buf, 1);
HashesMetaValue hashes_meta_value(Slice(meta_value_buf, sizeof(int32_t)));
version = hashes_meta_value.UpdateVersion();
batch.Put(handles_[0], key, hashes_meta_value.Encode());
HashesDataKey hashes_data_key(key, version, field);

char buf[32];
Int64ToStr(buf, 32, value);
batch.Put(handles_[1], hashes_data_key.Encode(), buf);
Int64ToStr(value_buf, 32, value);
batch.Put(handles_[1], hashes_data_key.Encode(), value_buf);
*ret = value;
} else {
return s;
Expand All @@ -393,6 +390,7 @@ Status RedisHashes::HIncrbyfloat(const Slice& key, const Slice& field, const Sli
}

Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
char meta_value_buf[4] = {0};
if (s.ok()) {
ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
if (parsed_hashes_meta_value.IsStale() || parsed_hashes_meta_value.count() == 0) {
Expand Down Expand Up @@ -431,9 +429,8 @@ Status RedisHashes::HIncrbyfloat(const Slice& key, const Slice& field, const Sli
}
}
} else if (s.IsNotFound()) {
char str[4];
EncodeFixed32(str, 1);
HashesMetaValue hashes_meta_value(std::string(str, sizeof(int32_t)));
EncodeFixed32(meta_value_buf, 1);
HashesMetaValue hashes_meta_value(Slice(meta_value_buf, sizeof(int32_t)));
version = hashes_meta_value.UpdateVersion();
batch.Put(handles_[0], key, hashes_meta_value.Encode());

Expand Down Expand Up @@ -559,6 +556,7 @@ Status RedisHashes::HMSet(const Slice& key, const std::vector<FieldValue>& fvs)
int32_t version = 0;
std::string meta_value;
Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
char meta_value_buf[4] = {0};
if (s.ok()) {
ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
if (parsed_hashes_meta_value.IsStale() || parsed_hashes_meta_value.count() == 0) {
Expand Down Expand Up @@ -590,9 +588,8 @@ Status RedisHashes::HMSet(const Slice& key, const std::vector<FieldValue>& fvs)
batch.Put(handles_[0], key, meta_value);
}
} else if (s.IsNotFound()) {
char str[4];
EncodeFixed32(str, filtered_fvs.size());
HashesMetaValue hashes_meta_value(std::string(str, sizeof(int32_t)));
EncodeFixed32(meta_value_buf, filtered_fvs.size());
HashesMetaValue hashes_meta_value(Slice(meta_value_buf, sizeof(int32_t)));
version = hashes_meta_value.UpdateVersion();
batch.Put(handles_[0], key, hashes_meta_value.Encode());
for (const auto& fv : filtered_fvs) {
Expand All @@ -613,6 +610,7 @@ Status RedisHashes::HSet(const Slice& key, const Slice& field, const Slice& valu
uint32_t statistic = 0;
std::string meta_value;
Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
char meta_value_buf[4] = {0};
if (s.ok()) {
ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
if (parsed_hashes_meta_value.IsStale() || parsed_hashes_meta_value.count() == 0) {
Expand Down Expand Up @@ -645,9 +643,8 @@ Status RedisHashes::HSet(const Slice& key, const Slice& field, const Slice& valu
}
}
} else if (s.IsNotFound()) {
char str[4];
EncodeFixed32(str, 1);
HashesMetaValue meta_value(Slice(str, sizeof(int32_t)));
EncodeFixed32(meta_value_buf, 1);
HashesMetaValue meta_value(Slice(meta_value_buf, sizeof(int32_t)));
version = meta_value.UpdateVersion();
batch.Put(handles_[0], key, meta_value.Encode());
HashesDataKey data_key(key, version, field);
Expand All @@ -668,6 +665,7 @@ Status RedisHashes::HSetnx(const Slice& key, const Slice& field, const Slice& va
int32_t version = 0;
std::string meta_value;
Status s = db_->Get(default_read_options_, handles_[0], key, &meta_value);
char meta_value_buf[4] = {0};
if (s.ok()) {
ParsedHashesMetaValue parsed_hashes_meta_value(&meta_value);
if (parsed_hashes_meta_value.IsStale() || parsed_hashes_meta_value.count() == 0) {
Expand All @@ -694,9 +692,8 @@ Status RedisHashes::HSetnx(const Slice& key, const Slice& field, const Slice& va
}
}
} else if (s.IsNotFound()) {
char str[4];
EncodeFixed32(str, 1);
HashesMetaValue hashes_meta_value(std::string(str, sizeof(int32_t)));
EncodeFixed32(meta_value_buf, 1);
HashesMetaValue hashes_meta_value(Slice(meta_value_buf, sizeof(int32_t)));
version = hashes_meta_value.UpdateVersion();
batch.Put(handles_[0], key, hashes_meta_value.Encode());
HashesDataKey hashes_data_key(key, version, field);
Expand Down
24 changes: 12 additions & 12 deletions src/storage/tests/lists_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
#include "src/redis.h"
#include "storage/storage.h"

using storage::Status;
using storage::Slice;
using storage::EncodeFixed64;
using storage::ListsMetaValue;
using storage::ListsDataFilter;
using storage::ListsDataKey;
using storage::ListsMetaValue;
using storage::Slice;
using storage::Status;

class ListsFilterTest : public ::testing::Test {
public:
Expand Down Expand Up @@ -77,7 +77,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) {

// Timeout timestamp is not set, but it's an empty list.
EncodeFixed64(str, 0);
ListsMetaValue lists_meta_value1(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value1(Slice(str, sizeof(uint64_t)));
lists_meta_value1.UpdateVersion();
std::this_thread::sleep_for(std::chrono::milliseconds(1000));
filter_result =
Expand All @@ -86,7 +86,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) {

// Timeout timestamp is not set, it's not an empty list.
EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value2(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value2(Slice(str, sizeof(uint64_t)));
lists_meta_value2.UpdateVersion();
std::this_thread::sleep_for(std::chrono::milliseconds(1000));
filter_result =
Expand All @@ -95,7 +95,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) {

// Timeout timestamp is set, but not expired.
EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value3(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value3(Slice(str, sizeof(uint64_t)));
lists_meta_value3.UpdateVersion();
lists_meta_value3.SetRelativeTimestamp(3);
std::this_thread::sleep_for(std::chrono::milliseconds(1000));
Expand All @@ -105,7 +105,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) {

// Timeout timestamp is set, already expired.
EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value4(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value4(Slice(str, sizeof(uint64_t)));
lists_meta_value4.UpdateVersion();
lists_meta_value4.SetRelativeTimestamp(1);
std::this_thread::sleep_for(std::chrono::milliseconds(2000));
Expand All @@ -128,7 +128,7 @@ TEST_F(ListsFilterTest, DataFilterTest) {
ASSERT_TRUE(lists_data_filter1 != nullptr);

EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value1(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value1(Slice(str, sizeof(uint64_t)));
version = lists_meta_value1.UpdateVersion();
s = meta_db->Put(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY", lists_meta_value1.Encode());
ASSERT_TRUE(s.ok());
Expand All @@ -145,7 +145,7 @@ TEST_F(ListsFilterTest, DataFilterTest) {
ASSERT_TRUE(lists_data_filter2 != nullptr);

EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value2(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value2(Slice(str, sizeof(uint64_t)));
version = lists_meta_value2.UpdateVersion();
lists_meta_value2.SetRelativeTimestamp(1);
s = meta_db->Put(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY", lists_meta_value2.Encode());
Expand All @@ -162,7 +162,7 @@ TEST_F(ListsFilterTest, DataFilterTest) {
ASSERT_TRUE(lists_data_filter3 != nullptr);

EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value3(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value3(Slice(str, sizeof(uint64_t)));
version = lists_meta_value3.UpdateVersion();
lists_meta_value3.SetRelativeTimestamp(1);
s = meta_db->Put(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY", lists_meta_value3.Encode());
Expand All @@ -180,7 +180,7 @@ TEST_F(ListsFilterTest, DataFilterTest) {
ASSERT_TRUE(lists_data_filter4 != nullptr);

EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value4(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value4(Slice(str, sizeof(uint64_t)));
version = lists_meta_value4.UpdateVersion();
s = meta_db->Put(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY", lists_meta_value4.Encode());
ASSERT_TRUE(s.ok());
Expand All @@ -199,7 +199,7 @@ TEST_F(ListsFilterTest, DataFilterTest) {
ASSERT_TRUE(lists_data_filter5 != nullptr);

EncodeFixed64(str, 1);
ListsMetaValue lists_meta_value5(std::string(str, sizeof(uint64_t)));
ListsMetaValue lists_meta_value5(Slice(str, sizeof(uint64_t)));
version = lists_meta_value5.UpdateVersion();
s = meta_db->Put(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY", lists_meta_value5.Encode());
ASSERT_TRUE(s.ok());
Expand Down

0 comments on commit 8bf77bc

Please sign in to comment.