wbtiger
/
foundationdb
mirror of https://github.com/apple/foundationdb.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4909 from apple/anoyes/fix-ub 

Fix several instances of undefined behavior
This commit is contained in:
Andrew Noyes 2021-06-07 08:58:45 -07:00 committed by GitHub
parent 838d847d4e d6a6a8b3dd
commit 402622ace9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 68 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
fdbclient/FDBTypes.h
View File

@ -483,7 +483,9 @@ inline Key keyAfter(const KeyRef& key) {
Standalone<StringRef> r;
uint8_t* s = new (r.arena()) uint8_t[key.size() + 1];
memcpy(s, key.begin(), key.size());
if (key.size() > 0) {
memcpy(s, key.begin(), key.size());
}
s[key.size()] = 0;
((StringRef&)r) = StringRef(s, key.size() + 1);
return r;

8
fdbclient/ManagementAPI.actor.cpp
View File

@ -1315,7 +1315,7 @@ struct AutoQuorumChange final : IQuorumChange {
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile> ccf,
CoordinatorsResult& err) override {
return getDesired(this, tr, oldCoordinators, ccf, &err);
return getDesired(Reference<AutoQuorumChange>::addRef(this), tr, oldCoordinators, ccf, &err);
}
ACTOR static Future<int> getRedundancy(AutoQuorumChange* self, Transaction* tr) {
@ -1378,7 +1378,7 @@ struct AutoQuorumChange final : IQuorumChange {
return true; // The status quo seems fine
}
ACTOR static Future<vector<NetworkAddress>> getDesired(AutoQuorumChange* self,
ACTOR static Future<vector<NetworkAddress>> getDesired(Reference<AutoQuorumChange> self,
Transaction* tr,
vector<NetworkAddress> oldCoordinators,
Reference<ClusterConnectionFile> ccf,
@ -1386,7 +1386,7 @@ struct AutoQuorumChange final : IQuorumChange {
state int desiredCount = self->desired;
if (desiredCount == -1) {
int redundancy = wait(getRedundancy(self, tr));
int redundancy = wait(getRedundancy(self.getPtr(), tr));
desiredCount = redundancy * 2 - 1;
}
@ -1415,7 +1415,7 @@ struct AutoQuorumChange final : IQuorumChange {
}
if (checkAcceptable) {
bool ok = wait(isAcceptable(self, tr, oldCoordinators, ccf, desiredCount, &excluded));
bool ok = wait(isAcceptable(self.getPtr(), tr, oldCoordinators, ccf, desiredCount, &excluded));
if (ok)
return oldCoordinators;
}

11
fdbrpc/Stats.h
View File

@ -20,6 +20,7 @@
#ifndef FDBRPC_STATS_H
#define FDBRPC_STATS_H
#include <type_traits>
#pragma once
// Yet another performance statistics interface
@ -136,7 +137,15 @@ struct SpecialCounter final : ICounter, FastAllocated<SpecialCounter<F>>, NonCop
void remove() override { delete this; }
std::string const& getName() const override { return name; }
int64_t getValue() const override { return f(); }
int64_t getValue() const override {
auto result = f();
// Disallow conversion from floating point to int64_t, since this has
// been a source of confusion - e.g. a percentage represented as a
// fraction between 0 and 1 is not meaningful after conversion to
// int64_t.
static_assert(!std::is_floating_point_v<decltype(result)>);
return result;
}
void resetInterval() override {}

4
fdbserver/DataDistributionTracker.actor.cpp
View File

@ -176,8 +176,8 @@ ShardSizeBounds getShardSizeBounds(KeyRangeRef shard, int64_t maxShardSize) {
}
int64_t getMaxShardSize(double dbSizeEstimate) {
return std::min((SERVER_KNOBS->MIN_SHARD_BYTES +
(int64_t)std::sqrt(dbSizeEstimate) * SERVER_KNOBS->SHARD_BYTES_PER_SQRT_BYTES) *
return std::min((SERVER_KNOBS->MIN_SHARD_BYTES + (int64_t)std::sqrt(std::max<double>(dbSizeEstimate, 0)) *
SERVER_KNOBS->SHARD_BYTES_PER_SQRT_BYTES) *
SERVER_KNOBS->SHARD_BYTES_RATIO,
(int64_t)SERVER_KNOBS->MAX_SHARD_BYTES);
}

2
fdbserver/DeltaTree.h
View File

@ -832,7 +832,7 @@ public:
int count = end - begin;
numItems = count;
nodeBytesDeleted = 0;
initialHeight = (uint8_t)log2(count) + 1;
initialHeight = count ? (uint8_t)log2(count) + 1 : 0;
maxHeight = 0;
// The boundary leading to the new page acts as the last time we branched right

25
fdbserver/GrvProxyServer.actor.cpp
View File

@ -109,15 +109,18 @@ struct GrvProxyStats {
SERVER_KNOBS->LATENCY_SAMPLE_SIZE),
grvLatencyBands("GRVLatencyMetrics", id, SERVER_KNOBS->STORAGE_LOGGING_DELAY) {
// The rate at which the limit(budget) is allowed to grow.
specialCounter(cc, "SystemAndDefaultTxnRateAllowed", [this]() { return this->transactionRateAllowed; });
specialCounter(cc, "BatchTransactionRateAllowed", [this]() { return this->batchTransactionRateAllowed; });
specialCounter(cc, "SystemAndDefaultTxnLimit", [this]() { return this->transactionLimit; });
specialCounter(cc, "BatchTransactionLimit", [this]() { return this->batchTransactionLimit; });
specialCounter(cc, "PercentageOfDefaultGRVQueueProcessed", [this]() {
return this->percentageOfDefaultGRVQueueProcessed;
});
specialCounter(
cc, "PercentageOfBatchGRVQueueProcessed", [this]() { return this->percentageOfBatchGRVQueueProcessed; });
cc, "SystemAndDefaultTxnRateAllowed", [this]() { return int64_t(this->transactionRateAllowed); });
specialCounter(
cc, "BatchTransactionRateAllowed", [this]() { return int64_t(this->batchTransactionRateAllowed); });
specialCounter(cc, "SystemAndDefaultTxnLimit", [this]() { return int64_t(this->transactionLimit); });
specialCounter(cc, "BatchTransactionLimit", [this]() { return int64_t(this->batchTransactionLimit); });
specialCounter(cc, "PercentageOfDefaultGRVQueueProcessed", [this]() {
return int64_t(100 * this->percentageOfDefaultGRVQueueProcessed);
});
specialCounter(cc, "PercentageOfBatchGRVQueueProcessed", [this]() {
return int64_t(100 * this->percentageOfBatchGRVQueueProcessed);
});
logger = traceCounters("GrvProxyMetrics", id, SERVER_KNOBS->WORKER_LOGGING_INTERVAL, &cc, "GrvProxyMetrics");
for (int i = 0; i < FLOW_KNOBS->BASIC_LOAD_BALANCE_BUCKETS; i++) {
@ -831,8 +834,10 @@ ACTOR static Future<Void> transactionStarter(GrvProxyInterface proxy,
}
span = Span(span.location);
grvProxyData->stats.percentageOfDefaultGRVQueueProcessed = (double)defaultGRVProcessed / defaultQueueSize;
grvProxyData->stats.percentageOfBatchGRVQueueProcessed = (double)batchGRVProcessed / batchQueueSize;
grvProxyData->stats.percentageOfDefaultGRVQueueProcessed =
defaultQueueSize ? (double)defaultGRVProcessed / defaultQueueSize : 1;
grvProxyData->stats.percentageOfBatchGRVQueueProcessed =
batchQueueSize ? (double)batchGRVProcessed / batchQueueSize : 1;
}
}

8
fdbserver/LogRouter.actor.cpp
View File

@ -175,22 +175,22 @@ struct LogRouterData {
specialCounter(cc, "WaitForVersionMS", [this]() {
double val = this->waitForVersionTime;
this->waitForVersionTime = 0;
return 1000 * val;
return int64_t(1000 * val);
});
specialCounter(cc, "WaitForVersionMaxMS", [this]() {
double val = this->maxWaitForVersionTime;
this->maxWaitForVersionTime = 0;
return 1000 * val;
return int64_t(1000 * val);
});
specialCounter(cc, "GetMoreMS", [this]() {
double val = this->getMoreTime;
this->getMoreTime = 0;
return 1000 * val;
return int64_t(1000 * val);
});
specialCounter(cc, "GetMoreMaxMS", [this]() {
double val = this->maxGetMoreTime;
this->maxGetMoreTime = 0;
return 1000 * val;
return int64_t(1000 * val);
});
specialCounter(cc, "Generation", [this]() { return this->generation; });
logger = traceCounters("LogRouterMetrics",

8
fdbserver/RadixTree.h
View File

@ -54,7 +54,9 @@ StringRef radix_join(const StringRef& key1, const StringRef& key2, Arena& arena)
uint8_t* s = new (arena) uint8_t[rsize];
memcpy(s, key1.begin(), key1.size());
memcpy(s + key1.size(), key2.begin(), key2.size());
if (key2.size() > 0) {
memcpy(s + key1.size(), key2.begin(), key2.size());
}
return StringRef(s, rsize);
}
@ -591,7 +593,9 @@ StringRef radix_tree::iterator::getKey(uint8_t* content) const {
auto node = m_pointee;
uint32_t pos = m_pointee->m_depth;
while (true) {
memcpy(content + pos, node->getKey().begin(), node->getKeySize());
if (node->getKeySize() > 0) {
memcpy(content + pos, node->getKey().begin(), node->getKeySize());
}
node = node->m_parent;
if (node == nullptr || pos <= 0)
break;

6
fdbserver/SkipList.cpp
View File

@ -355,8 +355,10 @@ public:
// pre: !finished()
force_inline void prefetch() {
Node* next = x->getNext(level - 1);
_mm_prefetch((const char*)next, _MM_HINT_T0);
_mm_prefetch((const char*)next + 64, _MM_HINT_T0);
if (next) {
_mm_prefetch((const char*)next, _MM_HINT_T0);
_mm_prefetch((const char*)next + 64, _MM_HINT_T0);
}
}
// pre: !finished()

5
fdbserver/VersionedBTree.actor.cpp
View File

@ -1004,7 +1004,7 @@ struct RedwoodMetrics {
if (*m.first == '\0') {
*s += "\n";
} else if (!skipZeroes || m.second != 0) {
*s += format("%-15s %-8u %8u/s ", m.first, m.second, int(m.second / elapsed));
*s += format("%-15s %-8u %8" PRId64 "/s ", m.first, m.second, int64_t(m.second / elapsed));
}
}
}
@ -2871,7 +2871,8 @@ struct RedwoodRecordRef {
case 1:
return *(int32_t*)r;
case 2:
return (((int64_t)((int48_t*)r)->high) << 16) | (((int48_t*)r)->low & 0xFFFF);
return ((int64_t) static_cast<uint32_t>(reinterpret_cast<const int48_t*>(r)->high) << 16) |
(((int48_t*)r)->low & 0xFFFF);
case 3:
default:
return *(int64_t*)r;

2
fdbserver/sqlite/sqlite3.amalgamation.c
View File

@ -43143,7 +43143,7 @@ SQLITE_PRIVATE void sqlite3VdbeMakeReady(
p->pFree = sqlite3DbMallocZero(db, nByte);
}
zCsr = p->pFree;
zEnd = &zCsr[nByte];
zEnd = zCsr ? &zCsr[nByte] : NULL;
}while( nByte && !db->mallocFailed );
p->nCursor = (u16)nCursor;

2
fdbserver/storageserver.actor.cpp
View File

@ -729,7 +729,7 @@ public:
specialCounter(cc, "DurableVersion", [self]() { return self->durableVersion.get(); });
specialCounter(cc, "DesiredOldestVersion", [self]() { return self->desiredOldestVersion.get(); });
specialCounter(cc, "VersionLag", [self]() { return self->versionLag; });
specialCounter(cc, "LocalRate", [self] { return self->currentRate() * 100; });
specialCounter(cc, "LocalRate", [self] { return int64_t(self->currentRate() * 100); });
specialCounter(cc, "BytesReadSampleCount", [self]() { return self->metrics.bytesReadSample.queue.size(); });

12
flow/Arena.h
View File

@ -444,8 +444,18 @@ public:
StringRef substr(int start) const { return StringRef(data + start, length - start); }
StringRef substr(int start, int size) const { return StringRef(data + start, size); }
bool startsWith(const StringRef& s) const { return size() >= s.size() && !memcmp(begin(), s.begin(), s.size()); }
bool startsWith(const StringRef& s) const {
// Avoid UB - can't pass nullptr to memcmp
if (s.size() == 0) {
return true;
}
return size() >= s.size() && !memcmp(begin(), s.begin(), s.size());
}
bool endsWith(const StringRef& s) const {
// Avoid UB - can't pass nullptr to memcmp
if (s.size() == 0) {
return true;
}
return size() >= s.size() && !memcmp(end() - s.size(), s.begin(), s.size());
}

2
flow/FastAlloc.cpp
View File

@ -119,7 +119,7 @@ void setFastAllocatorThreadInitFunction(ThreadInitFunction f) {
std::atomic<int64_t> g_hugeArenaMemory(0);
double hugeArenaLastLogged = 0;
std::map<std::string, std::pair<int, int>> hugeArenaTraces;
std::map<std::string, std::pair<int, int64_t>> hugeArenaTraces;
void hugeArenaSample(int size) {
if (TraceEvent::isNetworkThread()) {

3
flow/Histogram.cpp
View File

@ -28,6 +28,7 @@
// either we pull g_simulator into flow, or flow (and the I/O path) will be unable to log performance
// metrics.
#include <fdbrpc/simulator.h>
#include <limits>
// pull in some global pointers too: These types are implemented in fdbrpc/sim2.actor.cpp, which is not available here.
// Yuck. If you're not using the simulator, these will remain null, and all should be well.
@ -117,7 +118,7 @@ void Histogram::writeToLog() {
e.detail("Group", group).detail("Op", op).detail("Unit", UnitToStringMapper.at(unit));
for (uint32_t i = 0; i < 32; i++) {
uint32_t value = ((uint32_t)1) << (i + 1);
uint64_t value = uint64_t(1) << (i + 1);
if (buckets[i]) {
switch (unit) {