Merge pull request #5089 from sfc-gh-tclinkenbeard/fix-ub

Enable UB to fail tests in ctest when using UBSAN
This commit is contained in:
Trevor Clinkenbeard 2021-07-14 12:58:56 -07:00 committed by GitHub
commit a6f57d181c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 65 additions and 74 deletions

View File

@ -136,6 +136,7 @@ function(add_fdb_test)
${VALGRIND_OPTION}
${ADD_FDB_TEST_TEST_FILES}
WORKING_DIRECTORY ${PROJECT_BINARY_DIR})
set_tests_properties("${test_name}" PROPERTIES ENVIRONMENT UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1)
get_filename_component(test_dir_full ${first_file} DIRECTORY)
if(NOT ${test_dir_full} STREQUAL "")
get_filename_component(test_dir ${test_dir_full} NAME)
@ -419,6 +420,7 @@ function(add_fdbclient_test)
--
${T_COMMAND})
set_tests_properties("${T_NAME}" PROPERTIES TIMEOUT 60)
set_tests_properties("${T_NAME}" PROPERTIES ENVIRONMENT UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1)
endfunction()
# Creates 3 distinct clusters before running the specified command.

View File

@ -25,11 +25,3 @@
Reference<IConfigTransaction> IConfigTransaction::createTestSimple(ConfigTransactionInterface const& cti) {
return makeReference<SimpleConfigTransaction>(cti);
}
Reference<IConfigTransaction> IConfigTransaction::createSimple(Database const& cx) {
return makeReference<SimpleConfigTransaction>(cx);
}
Reference<IConfigTransaction> IConfigTransaction::createPaxos(Database const& cx) {
return makeReference<PaxosConfigTransaction>(cx);
}

View File

@ -40,8 +40,6 @@ public:
virtual ~IConfigTransaction() = default;
static Reference<IConfigTransaction> createTestSimple(ConfigTransactionInterface const&);
static Reference<IConfigTransaction> createSimple(Database const&);
static Reference<IConfigTransaction> createPaxos(Database const&);
// Not implemented:
void setVersion(Version) override { throw client_invalid_operation(); }

View File

@ -26,33 +26,15 @@
ISingleThreadTransaction* ISingleThreadTransaction::allocateOnForeignThread(Type type) {
if (type == Type::RYW) {
auto tr =
(ReadYourWritesTransaction*)(ReadYourWritesTransaction::operator new(sizeof(ReadYourWritesTransaction)));
tr->preinitializeOnForeignThread();
auto tr = new ReadYourWritesTransaction;
return tr;
} else if (type == Type::SIMPLE_CONFIG) {
auto tr = (SimpleConfigTransaction*)(SimpleConfigTransaction::operator new(sizeof(SimpleConfigTransaction)));
auto tr = new SimpleConfigTransaction;
return tr;
} else if (type == Type::PAXOS_CONFIG) {
auto tr = (PaxosConfigTransaction*)(PaxosConfigTransaction::operator new(sizeof(PaxosConfigTransaction)));
auto tr = new PaxosConfigTransaction;
return tr;
}
ASSERT(false);
return nullptr;
}
void ISingleThreadTransaction::create(ISingleThreadTransaction* tr, Type type, Database db) {
switch (type) {
case Type::RYW:
new (tr) ReadYourWritesTransaction(db);
break;
case Type::SIMPLE_CONFIG:
new (tr) SimpleConfigTransaction(db);
break;
case Type::PAXOS_CONFIG:
new (tr) PaxosConfigTransaction(db);
break;
default:
ASSERT(false);
}
}

View File

@ -45,7 +45,7 @@ public:
};
static ISingleThreadTransaction* allocateOnForeignThread(Type type);
static void create(ISingleThreadTransaction* tr, Type type, Database db);
virtual void setDatabase(Database const&) = 0;
virtual void setVersion(Version v) = 0;
virtual Future<Version> getReadVersion() = 0;

View File

@ -113,7 +113,7 @@ ThreadFuture<RangeResult> DLTransaction::getRange(const KeySelectorRef& begin,
end.offset,
limits.rows,
limits.bytes,
FDBStreamingModes::EXACT,
FDB_STREAMING_MODE_EXACT,
0,
snapshot,
reverse);
@ -207,12 +207,12 @@ ThreadFuture<Standalone<VectorRef<KeyRef>>> DLTransaction::getRangeSplitPoints(c
void DLTransaction::addReadConflictRange(const KeyRangeRef& keys) {
throwIfError(api->transactionAddConflictRange(
tr, keys.begin.begin(), keys.begin.size(), keys.end.begin(), keys.end.size(), FDBConflictRangeTypes::READ));
tr, keys.begin.begin(), keys.begin.size(), keys.end.begin(), keys.end.size(), FDB_CONFLICT_RANGE_TYPE_READ));
}
void DLTransaction::atomicOp(const KeyRef& key, const ValueRef& value, uint32_t operationType) {
api->transactionAtomicOp(
tr, key.begin(), key.size(), value.begin(), value.size(), (FDBMutationTypes::Option)operationType);
tr, key.begin(), key.size(), value.begin(), value.size(), static_cast<FDBMutationType>(operationType));
}
void DLTransaction::set(const KeyRef& key, const ValueRef& value) {
@ -239,7 +239,7 @@ ThreadFuture<Void> DLTransaction::watch(const KeyRef& key) {
void DLTransaction::addWriteConflictRange(const KeyRangeRef& keys) {
throwIfError(api->transactionAddConflictRange(
tr, keys.begin.begin(), keys.begin.size(), keys.end.begin(), keys.end.size(), FDBConflictRangeTypes::WRITE));
tr, keys.begin.begin(), keys.begin.size(), keys.end.begin(), keys.end.size(), FDB_CONFLICT_RANGE_TYPE_WRITE));
}
ThreadFuture<Void> DLTransaction::commit() {
@ -269,8 +269,10 @@ ThreadFuture<int64_t> DLTransaction::getApproximateSize() {
}
void DLTransaction::setOption(FDBTransactionOptions::Option option, Optional<StringRef> value) {
throwIfError(api->transactionSetOption(
tr, option, value.present() ? value.get().begin() : nullptr, value.present() ? value.get().size() : 0));
throwIfError(api->transactionSetOption(tr,
static_cast<FDBTransactionOption>(option),
value.present() ? value.get().begin() : nullptr,
value.present() ? value.get().size() : 0));
}
ThreadFuture<Void> DLTransaction::onError(Error const& e) {
@ -309,8 +311,10 @@ Reference<ITransaction> DLDatabase::createTransaction() {
}
void DLDatabase::setOption(FDBDatabaseOptions::Option option, Optional<StringRef> value) {
throwIfError(api->databaseSetOption(
db, option, value.present() ? value.get().begin() : nullptr, value.present() ? value.get().size() : 0));
throwIfError(api->databaseSetOption(db,
static_cast<FDBDatabaseOption>(option),
value.present() ? value.get().begin() : nullptr,
value.present() ? value.get().size() : 0));
}
ThreadFuture<int64_t> DLDatabase::rebootWorker(const StringRef& address, bool check, int duration) {
@ -504,7 +508,7 @@ void DLApi::selectApiVersion(int apiVersion) {
init();
throwIfError(api->selectApiVersion(apiVersion, headerVersion));
throwIfError(api->setNetworkOption(FDBNetworkOptions::EXTERNAL_CLIENT, nullptr, 0));
throwIfError(api->setNetworkOption(static_cast<FDBNetworkOption>(FDBNetworkOptions::EXTERNAL_CLIENT), nullptr, 0));
}
const char* DLApi::getClientVersion() {
@ -516,8 +520,9 @@ const char* DLApi::getClientVersion() {
}
void DLApi::setNetworkOption(FDBNetworkOptions::Option option, Optional<StringRef> value) {
throwIfError(api->setNetworkOption(
option, value.present() ? value.get().begin() : nullptr, value.present() ? value.get().size() : 0));
throwIfError(api->setNetworkOption(static_cast<FDBNetworkOption>(option),
value.present() ? value.get().begin() : nullptr,
value.present() ? value.get().size() : 0));
}
void DLApi::setupNetwork() {

View File

@ -22,6 +22,7 @@
#define FDBCLIENT_MULTIVERSIONTRANSACTION_H
#pragma once
#include "bindings/c/foundationdb/fdb_c_options.g.h"
#include "fdbclient/FDBOptions.g.h"
#include "fdbclient/FDBTypes.h"
#include "fdbclient/IClientApi.h"
@ -31,10 +32,10 @@
// FdbCApi is used as a wrapper around the FoundationDB C API that gets loaded from an external client library.
// All of the required functions loaded from that external library are stored in function pointers in this struct.
struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
typedef struct future FDBFuture;
typedef struct cluster FDBCluster;
typedef struct database FDBDatabase;
typedef struct transaction FDBTransaction;
typedef struct FDB_future FDBFuture;
typedef struct FDB_cluster FDBCluster;
typedef struct FDB_database FDBDatabase;
typedef struct FDB_transaction FDBTransaction;
#pragma pack(push, 4)
typedef struct key {
@ -57,16 +58,16 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
// Network
fdb_error_t (*selectApiVersion)(int runtimeVersion, int headerVersion);
const char* (*getClientVersion)();
fdb_error_t (*setNetworkOption)(FDBNetworkOptions::Option option, uint8_t const* value, int valueLength);
fdb_error_t (*setNetworkOption)(FDBNetworkOption option, uint8_t const* value, int valueLength);
fdb_error_t (*setupNetwork)();
fdb_error_t (*runNetwork)();
fdb_error_t (*stopNetwork)();
fdb_error_t* (*createDatabase)(const char* clusterFilePath, FDBDatabase** db);
fdb_error_t (*createDatabase)(const char* clusterFilePath, FDBDatabase** db);
// Database
fdb_error_t (*databaseCreateTransaction)(FDBDatabase* database, FDBTransaction** tr);
fdb_error_t (*databaseSetOption)(FDBDatabase* database,
FDBDatabaseOptions::Option option,
FDBDatabaseOption option,
uint8_t const* value,
int valueLength);
void (*databaseDestroy)(FDBDatabase* database);
@ -86,7 +87,7 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
// Transaction
fdb_error_t (*transactionSetOption)(FDBTransaction* tr,
FDBTransactionOptions::Option option,
FDBTransactionOption option,
uint8_t const* value,
int valueLength);
void (*transactionDestroy)(FDBTransaction* tr);
@ -113,7 +114,7 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
int endOffset,
int limit,
int targetBytes,
FDBStreamingModes::Option mode,
FDBStreamingMode mode,
int iteration,
fdb_bool_t snapshot,
fdb_bool_t reverse);
@ -135,7 +136,7 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
int keyNameLength,
uint8_t const* param,
int paramLength,
FDBMutationTypes::Option operationType);
FDBMutationType operationType);
FDBFuture* (*transactionGetEstimatedRangeSizeBytes)(FDBTransaction* tr,
uint8_t const* begin_key_name,
@ -163,7 +164,7 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
int beginKeyNameLength,
uint8_t const* endKeyName,
int endKeyNameLength,
FDBConflictRangeTypes::Option);
FDBConflictRangeType);
// Future
fdb_error_t (*futureGetDatabase)(FDBFuture* f, FDBDatabase** outDb);

View File

@ -244,8 +244,6 @@ public:
explicit Transaction(Database const& cx);
~Transaction();
void preinitializeOnForeignThread() { committedVersion = invalidVersion; }
void setVersion(Version v);
Future<Version> getReadVersion() { return getReadVersion(0); }
Future<Version> getRawReadVersion();
@ -421,7 +419,7 @@ private:
Database cx;
double backoff;
Version committedVersion;
Version committedVersion{ invalidVersion };
CommitTransactionRequest tr;
Future<Version> readVersion;
Promise<Optional<Value>> metadataVersion;

View File

@ -122,9 +122,14 @@ void PaxosConfigTransaction::checkDeferredError() const {
ASSERT(false);
}
PaxosConfigTransaction::PaxosConfigTransaction(Database const& cx) {
PaxosConfigTransaction::PaxosConfigTransaction() {
// TODO: Implement
ASSERT(false);
}
PaxosConfigTransaction::~PaxosConfigTransaction() = default;
void PaxosConfigTransaction::setDatabase(Database const& cx) {
// TODO: Implement
ASSERT(false);
}

View File

@ -33,8 +33,9 @@ class PaxosConfigTransaction final : public IConfigTransaction, public FastAlloc
PaxosConfigTransactionImpl& impl() { return *_impl; }
public:
PaxosConfigTransaction(Database const&);
PaxosConfigTransaction();
~PaxosConfigTransaction();
void setDatabase(Database const&) override;
Future<Version> getReadVersion() override;
Optional<Version> getCachedReadVersion() const override;

View File

@ -1293,6 +1293,10 @@ ReadYourWritesTransaction::ReadYourWritesTransaction(Database const& cx)
applyPersistentOptions();
}
void ReadYourWritesTransaction::setDatabase(Database const& cx) {
*this = ReadYourWritesTransaction(cx);
}
ACTOR Future<Void> timebomb(double endTime, Promise<Void> resetPromise) {
while (now() < endTime) {
wait(delayUntil(std::min(endTime + 0.0001, now() + CLIENT_KNOBS->TRANSACTION_TIMEOUT_DELAY_INTERVAL)));
@ -1735,10 +1739,6 @@ void ReadYourWritesTransaction::getWriteConflicts(KeyRangeMap<bool>* result) {
}
}
void ReadYourWritesTransaction::preinitializeOnForeignThread() {
tr.preinitializeOnForeignThread();
}
void ReadYourWritesTransaction::setTransactionID(uint64_t id) {
tr.setTransactionID(id);
}

View File

@ -68,6 +68,7 @@ public:
explicit ReadYourWritesTransaction(Database const& cx);
~ReadYourWritesTransaction();
void setDatabase(Database const&) override;
void setVersion(Version v) override { tr.setVersion(v); }
Future<Version> getReadVersion() override;
Optional<Version> getCachedReadVersion() const override { return tr.getCachedReadVersion(); }
@ -153,8 +154,6 @@ public:
void getWriteConflicts(KeyRangeMap<bool>* result) override;
void preinitializeOnForeignThread();
Database getDatabase() const { return tr.getDatabase(); }
const TransactionInfo& getTransactionInfo() const { return tr.info; }

View File

@ -290,10 +290,13 @@ void SimpleConfigTransaction::checkDeferredError() const {
impl().checkDeferredError(deferredError);
}
SimpleConfigTransaction::SimpleConfigTransaction(Database const& cx)
: _impl(std::make_unique<SimpleConfigTransactionImpl>(cx)) {}
void SimpleConfigTransaction::setDatabase(Database const& cx) {
_impl = std::make_unique<SimpleConfigTransactionImpl>(cx);
}
SimpleConfigTransaction::SimpleConfigTransaction(ConfigTransactionInterface const& cti)
: _impl(std::make_unique<SimpleConfigTransactionImpl>(cti)) {}
SimpleConfigTransaction::SimpleConfigTransaction() = default;
SimpleConfigTransaction::~SimpleConfigTransaction() = default;

View File

@ -43,6 +43,8 @@ class SimpleConfigTransaction final : public IConfigTransaction, public FastAllo
public:
SimpleConfigTransaction(ConfigTransactionInterface const&);
SimpleConfigTransaction(Database const&);
SimpleConfigTransaction();
void setDatabase(Database const&) override;
~SimpleConfigTransaction();
Future<Version> getReadVersion() override;
Optional<Version> getCachedReadVersion() const override;

View File

@ -149,9 +149,9 @@ ThreadSafeTransaction::ThreadSafeTransaction(DatabaseContext* cx, ISingleThreadT
auto tr = this->tr = ISingleThreadTransaction::allocateOnForeignThread(type);
// No deferred error -- if the construction of the RYW transaction fails, we have no where to put it
onMainThreadVoid(
[tr, type, cx]() {
[tr, cx]() {
cx->addref();
ISingleThreadTransaction::create(tr, type, Database(cx));
tr->setDatabase(Database(cx));
},
nullptr);
}

View File

@ -17,6 +17,7 @@
#include "../allocators.h"
#include "swap.h"
#include <cstddef>
#if defined(__clang__)
RAPIDJSON_DIAG_PUSH
@ -106,7 +107,7 @@ public:
template <typename T>
RAPIDJSON_FORCEINLINE void Reserve(size_t count = 1) {
// Expand the stack if needed
if (RAPIDJSON_UNLIKELY(stackTop_ + sizeof(T) * count > stackEnd_))
if (RAPIDJSON_UNLIKELY(static_cast<std::ptrdiff_t>(sizeof(T) * count) > (stackEnd_ - stackTop_)))
Expand<T>(count);
}
@ -118,7 +119,7 @@ public:
template <typename T>
RAPIDJSON_FORCEINLINE T* PushUnsafe(size_t count = 1) {
RAPIDJSON_ASSERT(stackTop_ + sizeof(T) * count <= stackEnd_);
RAPIDJSON_ASSERT(static_cast<std::ptrdiff_t>(sizeof(T) * count) <= (stackEnd_ - stackTop_));
T* ret = reinterpret_cast<T*>(stackTop_);
stackTop_ += sizeof(T) * count;
return ret;

View File

@ -52,7 +52,7 @@ namespace vexillographer
{
string parameterComment = "";
if (o.scope.ToString().EndsWith("Option"))
parameterComment = String.Format("{0}/* {1} */\n", indent, "Parameter: " + o.getParameterComment());
parameterComment = String.Format("{0}/* {1} {2}*/\n", indent, "Parameter: " + o.getParameterComment(), o.hidden ? "This is a hidden parameter and should not be used directly by applications." : "");
return String.Format("{0}/* {2} */\n{5}{0}{1}{3}={4}", indent, prefix, o.comment, o.name.ToUpper(), o.code, parameterComment);
}
@ -64,7 +64,7 @@ namespace vexillographer
options = new Option[] { new Option{ scope = scope,
comment = "This option is only a placeholder for C compatibility and should not be used",
code = -1, name = "DUMMY_DO_NOT_USE", paramDesc = null } };
outFile.WriteLine(string.Join(",\n\n", options.Where(f => !f.hidden).Select(f => getCLine(f, " ", prefix)).ToArray()));
outFile.WriteLine(string.Join(",\n\n", options.Select(f => getCLine(f, " ", prefix)).ToArray()));
outFile.WriteLine("}} FDB{0};", scope.ToString());
outFile.WriteLine();
}

View File

@ -45,7 +45,7 @@ static HistogramRegistry* globalHistograms = nullptr;
#pragma region HistogramRegistry
HistogramRegistry& GetHistogramRegistry() {
ISimulator::ProcessInfo* info = g_simulator.getCurrentProcess();
ISimulator::ProcessInfo* info = g_network && g_network->isSimulated() ? g_simulator.getCurrentProcess() : nullptr;
if (info) {
// in simulator; scope histograms to simulated process

View File

@ -275,10 +275,12 @@ if(WITH_PYTHON)
NAME multiversion_client/unit_tests
COMMAND $<TARGET_FILE:fdbserver> -r unittests -f /fdbclient/multiversionclient/
)
set_tests_properties("multiversion_client/unit_tests" PROPERTIES ENVIRONMENT UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1)
add_test(
NAME threadsafe_threadfuture_to_future/unit_tests
COMMAND $<TARGET_FILE:fdbserver> -r unittests -f /flow/safeThreadFutureToFuture/
)
set_tests_properties("threadsafe_threadfuture_to_future/unit_tests" PROPERTIES ENVIRONMENT UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1)
endif()
verify_testing()