Add -Wshift-sign-overflow

This catches the bug fixed in #4656 at compile time

cmake/ConfigureCompiler.cmake

@@ -280,7 +280,12 @@ else()
         -Wno-unknown-attributes)
     endif()
     add_compile_options(
-      -Wall -Wextra
+      -Wall
+      -Wextra
+      -Wredundant-move
+      -Wpessimizing-move
+      -Woverloaded-virtual
+      -Wshift-sign-overflow
       # Here's the current set of warnings we need to explicitly disable to compile warning-free with clang 10
       -Wno-comment
       -Wno-dangling-else
@@ -288,16 +293,12 @@ else()
       -Wno-format
       -Wno-mismatched-tags
       -Wno-missing-field-initializers
-      -Wno-overloaded-virtual
       -Wno-reorder
       -Wno-reorder-ctor
       -Wno-sign-compare
       -Wno-tautological-pointer-compare
       -Wno-undefined-var-template
       -Wno-tautological-pointer-compare
-      -Wredundant-move
-      -Wpessimizing-move
-      -Woverloaded-virtual
       -Wno-unknown-pragmas
       -Wno-unknown-warning-option
       -Wno-unused-function

fdbclient/CoordinationInterface.h

@@ -107,8 +107,9 @@ private:
 
 struct LeaderInfo {
 	constexpr static FileIdentifier file_identifier = 8338794;
+	// The first 7 bits of changeID represent cluster controller process class fitness, the lower the better
 	UID changeID;
-	static const uint64_t mask = ~(127ll << 57);
+	static const uint64_t changeIDMask = ~(uint64_t(0b1111111) << 57);
 	Value serializedInfo;
 	bool forward; // If true, serializedInfo is a connection string instead!
 
@@ -125,13 +126,13 @@ struct LeaderInfo {
 	// The first 7 bits of ChangeID represent cluster controller process class fitness, the lower the better
 	void updateChangeID(ClusterControllerPriorityInfo info) {
 		changeID = UID(((uint64_t)info.processClassFitness << 57) | ((uint64_t)info.isExcluded << 60) |
-		                   ((uint64_t)info.dcFitness << 61) | (changeID.first() & mask),
+		                   ((uint64_t)info.dcFitness << 61) | (changeID.first() & changeIDMask),
 		               changeID.second());
 	}
 
 	// All but the first 7 bits are used to represent process id
 	bool equalInternalId(LeaderInfo const& leaderInfo) const {
-		return ((changeID.first() & mask) == (leaderInfo.changeID.first() & mask)) &&
+		return ((changeID.first() & changeIDMask) == (leaderInfo.changeID.first() & changeIDMask)) &&
 		       changeID.second() == leaderInfo.changeID.second();
 	}
 
@@ -139,8 +140,10 @@ struct LeaderInfo {
 	// 1. the candidate has better process class fitness and the candidate is not the leader
 	// 2. the leader process class fitness becomes worse
 	bool leaderChangeRequired(LeaderInfo const& candidate) const {
-		return ((changeID.first() & ~mask) > (candidate.changeID.first() & ~mask) && !equalInternalId(candidate)) ||
-		       ((changeID.first() & ~mask) < (candidate.changeID.first() & ~mask) && equalInternalId(candidate));
+		return ((changeID.first() & ~changeIDMask) > (candidate.changeID.first() & ~changeIDMask) &&
+		        !equalInternalId(candidate)) ||
+		       ((changeID.first() & ~changeIDMask) < (candidate.changeID.first() & ~changeIDMask) &&
+		        equalInternalId(candidate));
 	}
 
 	ClusterControllerPriorityInfo getPriorityInfo() const {

fdbclient/MonitorLeader.actor.cpp

@@ -432,7 +432,8 @@ Optional<std::pair<LeaderInfo, bool>> getLeader(const vector<Optional<LeaderInfo
 	for (int i = 0; i < nominees.size(); i++) {
 		if (nominees[i].present()) {
 			maskedNominees.push_back(std::make_pair(
-			    UID(nominees[i].get().changeID.first() & LeaderInfo::mask, nominees[i].get().changeID.second()), i));
+			    UID(nominees[i].get().changeID.first() & LeaderInfo::changeIDMask, nominees[i].get().changeID.second()),
+			    i));
 		}
 	}
 

fdbserver/workloads/FileSystem.actor.cpp

@@ -103,7 +103,7 @@ struct FileSystemWorkload : TestWorkload {
 
 		tr->set(key, path);
 		std::string keyStr(key.toString());
-		tr->set(keyStr + "/size", format("%d", deterministicRandom()->randomInt(0, 2 << 30)));
+		tr->set(keyStr + "/size", format("%d", deterministicRandom()->randomInt(0, std::numeric_limits<int>::max())));
 		tr->set(keyStr + "/server", format("%d", deterministicRandom()->randomInt(0, self->serverCount)));
 		tr->set(keyStr + "/deleted", deleted ? LiteralStringRef("1") : LiteralStringRef("0"));
 		tr->set(keyStr + "/server", format("%d", serverID));
@@ -236,7 +236,7 @@ struct FileSystemWorkload : TestWorkload {
 		loop {
 			state int fileID = deterministicRandom()->randomInt(0, self->fileCount);
 			state bool isDeleting = deterministicRandom()->random01() < 0.25;
-			state int size = isDeleting ? 0 : deterministicRandom()->randomInt(0, 2 << 30);
+			state int size = isDeleting ? 0 : deterministicRandom()->randomInt(0, std::numeric_limits<int>::max());
 			state std::string keyStr = self->keyForFileID(fileID).toString();
 			state double tstart = now();
 			state Transaction tr(cx);