Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD] Move from GCC to Clang #1816

Closed
wants to merge 15 commits into from
Closed
10 changes: 8 additions & 2 deletions .github/workflows/bedrock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:

- name: Install the Mold Linker
uses: rui314/setup-mold@v1

- name: Checkout Bedrock
uses: actions/[email protected]

Expand All @@ -46,14 +46,20 @@ jobs:
env:
OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}

- name: Install LLVM
run: |
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 18 all

- name: Install packages
run: |
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys BA9EF27F
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 4F4EA0AAE5267A6C
wget -qO - https://package.perforce.com/perforce.pubkey --no-check-certificate | sudo apt-key add -
echo "deb [arch=amd64] https://travis:${{ secrets.TRAVIS_APT_PASSWORD }}@$APT_MIRROR_URL/mirror/ppa.launchpad.net/ubuntu-toolchain-r/test/ubuntu focal main" | sudo tee -a /etc/apt/sources.list
sudo apt-get update -y
sudo -E apt-get -yq --no-install-suggests --no-install-recommends --force-yes install rsyslog cmake gcc-13 g++-13 libsodium-dev libgpgme11-dev libstdc++-13-dev
sudo -E apt-get -yq --no-install-suggests --no-install-recommends --force-yes install rsyslog cmake libsodium-dev libgpgme11-dev
sudo locale-gen "en_US.UTF-8"
sudo service rsyslog start

Expand Down
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
# to be set, but for the time being we need to override the defaults so that our existing dev environment works. This
# can be removed when that is resolved.
ifeq ($(CC),cc)
CC = gcc-13
CC = ccache /usr/bin/clang-18
endif
ifeq ($(CXX),g++)
CXX = g++-13
CXX = ccache /usr/bin/clang++-18
endif

# Set the optimization level from the environment, or default to -O2.
Expand All @@ -23,16 +23,16 @@ PROJECT = $(shell git rev-parse --show-toplevel)
INCLUDE = -I$(PROJECT) -I$(PROJECT)/mbedtls/include

# Set our standard C++ compiler flags
CXXFLAGS = -g -std=c++20 -fPIC -DSQLITE_ENABLE_NORMALIZE $(BEDROCK_OPTIM_COMPILE_FLAG) -Wall -Werror -Wformat-security -Wno-error=deprecated-declarations $(INCLUDE)
CXXFLAGS = -g -std=c++20 -gdwarf-4 -stdlib=libc++ -I/usr/include/c++/v1 -fPIC -DSQLITE_ENABLE_NORMALIZE $(BEDROCK_OPTIM_COMPILE_FLAG) -Wno-vla-cxx-extension -Wno-unqualified-std-cast-call -Werror -Wformat-security -Wno-error=deprecated-declarations $(INCLUDE) #-Wall -Werror
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-gdwarf-4 is here because otherwise clang outputs newer dwarf formats (version 5) than gnu ld will understand. We should be able get rid of this if we use clang's lld linker. For now, this outputs the older format.


# Amalgamation flags
AMALGAMATION_FLAGS = -Wno-unused-but-set-variable -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_STAT4 -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_SESSION -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT -DSQLITE_ENABLE_NOOP_UPDATE -DSQLITE_MUTEX_ALERT_MILLISECONDS=20 -DHAVE_USLEEP=1 -DSQLITE_MAX_MMAP_SIZE=17592186044416ull -DSQLITE_SHARED_MAPPING -DSQLITE_ENABLE_NORMALIZE -DSQLITE_MAX_PAGE_COUNT=4294967294 -DSQLITE_DISABLE_PAGECACHE_OVERFLOW_STATS
AMALGAMATION_FLAGS = -Wno-unused-const-variable -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_STAT4 -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_SESSION -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_UPDATE_DELETE_LIMIT -DSQLITE_ENABLE_NOOP_UPDATE -DSQLITE_MUTEX_ALERT_MILLISECONDS=20 -DHAVE_USLEEP=1 -DSQLITE_MAX_MMAP_SIZE=17592186044416ull -DSQLITE_SHARED_MAPPING -DSQLITE_ENABLE_NORMALIZE -DSQLITE_MAX_PAGE_COUNT=4294967294 -DSQLITE_DISABLE_PAGECACHE_OVERFLOW_STATS

# All our intermediate, dependency, object, etc files get hidden in here.
INTERMEDIATEDIR = .build

# We use the same library paths and required libraries for all binaries.
LIBPATHS =-L$(PROJECT) -Lmbedtls/library
LIBPATHS =-L$(PROJECT) -Lmbedtls/library -L/usr/lib/llvm-18/lib
LIBRARIES =-Wl,--start-group -lbedrock -lstuff -Wl,--end-group -ldl -lpcrecpp -lpthread -lmbedtls -lmbedx509 -lmbedcrypto -lz -lm

# These targets aren't actual files.
Expand Down Expand Up @@ -111,7 +111,7 @@ BINPREREQS = libbedrock.a libstuff.a mbedtls/library/libmbedcrypto.a

# All of our binaries build in the same way.
bedrock: $(BEDROCKOBJ) $(BINPREREQS)
$(CXX) -o $@ $(BEDROCKOBJ) $(LIBPATHS) -rdynamic $(LIBRARIES)
$(CXX) -stdlib=libc++ -o $@ $(BEDROCKOBJ) $(LIBPATHS) -rdynamic $(LIBRARIES)
johnmlee101 marked this conversation as resolved.
Show resolved Hide resolved
test/test: $(TESTOBJ) $(BINPREREQS)
$(CXX) -o $@ $(TESTOBJ) $(LIBPATHS) -rdynamic $(LIBRARIES)
test/clustertest/clustertest: $(CLUSTERTESTOBJ) $(BINPREREQS)
Expand Down
4 changes: 2 additions & 2 deletions ci_tests.sh
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#!/bin/bash
set -e

export CXX=g++-13
export CC=gcc-13
export CXX=/usr/bin/clang++-18
export CC=/usr/bin/clang-18

# Add the current working directory to $PATH so that tests can find bedrock.
export PATH=$PATH:`pwd`
Expand Down
4 changes: 3 additions & 1 deletion libstuff/SSignal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ void _SSignal_StackTrace(int signum, siginfo_t *info, void *ucontext) {
int status{0};
char* front = strchr(frame[0], '(') + 1;
char* end = strchr(front, '+');
char copy[end - front + 1]{0};
char* copy = (char*) malloc(end - front + 1);

strncpy(copy, front, end - front);
char* demangled = abi::__cxa_demangle(copy, 0, 0, &status);
char* tolog = status ? copy : demangled;
Expand All @@ -228,6 +229,7 @@ void _SSignal_StackTrace(int signum, siginfo_t *info, void *ucontext) {
}
SWARN("Frame #" << i << ": " << tolog);
free(frame);
free(copy);
}

// Done.
Expand Down
2 changes: 1 addition & 1 deletion sqlitecluster/SQLiteCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class SQLiteCommand {
SQLiteCommand(SQLiteCommand&& from);

// Default move assignment operator.
SQLiteCommand& operator=(SQLiteCommand&& from) = default;
// SQLiteCommand& operator=(SQLiteCommand&& from) = default;

// Destructor.
virtual ~SQLiteCommand() {}
Expand Down
14 changes: 7 additions & 7 deletions test/clustertest/testplugin/TestPlugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ class TestPluginCommand : public BedrockCommand {
public:
TestPluginCommand(SQLiteCommand&& baseCommand, BedrockPlugin_TestPlugin* plugin);
~TestPluginCommand();
virtual void prePeek(SQLite& db);
virtual bool peek(SQLite& db);
virtual void process(SQLite& db);
virtual void postProcess(SQLite& db);
virtual bool shouldPrePeek();
virtual bool shouldPostProcess();
virtual void prePeek(SQLite& db) override;
virtual bool peek(SQLite& db) override;
virtual void process(SQLite& db) override;
virtual void postProcess(SQLite& db) override;
virtual bool shouldPrePeek() override;
virtual bool shouldPostProcess() override;
virtual void reset(BedrockCommand::STAGE stage) override;
bool shouldEnableOnPrepareNotification(const SQLite& db, void (**handler)(SQLite& _db, int64_t tableID));
bool shouldEnableOnPrepareNotification(const SQLite& db, void (**handler)(SQLite& _db, int64_t tableID)) override;

private:
BedrockPlugin_TestPlugin& plugin() { return static_cast<BedrockPlugin_TestPlugin&>(*_plugin); }
Expand Down
7 changes: 4 additions & 3 deletions test/clustertest/tests/BadCommandTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
#include <test/clustertest/BedrockClusterTester.h>

struct BadCommandTest : tpunit::TestFixture {
BadCommandTest()
: tpunit::TestFixture("BadCommand", TEST(BadCommandTest::test)) { }
BadCommandTest() : tpunit::TestFixture("BadCommand") {
registerTests(TEST(BadCommandTest::test));
}

void test()
{
Expand Down Expand Up @@ -46,7 +47,7 @@ struct BadCommandTest : tpunit::TestFixture {
// This tests cases where keeping leader alive isn't feasible.
bool testFailed = false;
for (auto commandName : {"generatesegfaultpeek", "generateassertpeek", "generatesegfaultprocess"}) {

// Create the command with the current userID.
userID++;
SData command(commandName);
Expand Down
9 changes: 4 additions & 5 deletions test/clustertest/tests/BroadcastTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
#include <test/clustertest/BedrockClusterTester.h>

struct BroadcastCommandTest : tpunit::TestFixture {
BroadcastCommandTest()
: tpunit::TestFixture("BroadcastCommand",
BEFORE_CLASS(BroadcastCommandTest::setup),
BroadcastCommandTest() : tpunit::TestFixture("BroadcastCommand") {
registerTests(BEFORE_CLASS(BroadcastCommandTest::setup),
AFTER_CLASS(BroadcastCommandTest::teardown),
TEST(BroadcastCommandTest::test)
) { }
TEST(BroadcastCommandTest::test));
}

BedrockClusterTester* tester;

Expand Down
11 changes: 5 additions & 6 deletions test/clustertest/tests/ClusterUpgradeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
#include <test/clustertest/BedrockClusterTester.h>

struct ClusterUpgradeTest : tpunit::TestFixture {
ClusterUpgradeTest()
: tpunit::TestFixture("ClusterUpgrade",
BEFORE_CLASS(ClusterUpgradeTest::setup),
AFTER_CLASS(ClusterUpgradeTest::teardown),
TEST(ClusterUpgradeTest::test)
) { }
ClusterUpgradeTest() : tpunit::TestFixture("ClusterUpgrade") {
registerTests(BEFORE_CLASS(ClusterUpgradeTest::setup),
AFTER_CLASS(ClusterUpgradeTest::teardown),
TEST(ClusterUpgradeTest::test));
}

BedrockClusterTester* tester;
string prodBedrockName;
Expand Down
12 changes: 6 additions & 6 deletions test/clustertest/tests/ConflictSpamTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
#include <test/clustertest/BedrockClusterTester.h>

struct ConflictSpamTest : tpunit::TestFixture {
ConflictSpamTest()
: tpunit::TestFixture("ConflictSpam",
BEFORE_CLASS(ConflictSpamTest::setup),
AFTER_CLASS(ConflictSpamTest::teardown),
TEST(ConflictSpamTest::slow),
TEST(ConflictSpamTest::spam)) { }
ConflictSpamTest() : tpunit::TestFixture("ConflictSpam") {
registerTests(BEFORE_CLASS(ConflictSpamTest::setup),
AFTER_CLASS(ConflictSpamTest::teardown),
TEST(ConflictSpamTest::slow),
TEST(ConflictSpamTest::spam));
}

/* What's a conflict spam test? The main point of this test is to make sure we have lots of conflicting commits
* coming in to the whole cluster, so that we can make sure they all eventually get committed and replicated in a
Expand Down
11 changes: 6 additions & 5 deletions test/clustertest/tests/ControlCommandTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
#include <test/clustertest/BedrockClusterTester.h>

struct ControlCommandTest : tpunit::TestFixture {
ControlCommandTest()
: tpunit::TestFixture("ControlCommand",
BEFORE_CLASS(ControlCommandTest::setup),
AFTER_CLASS(ControlCommandTest::teardown),
TEST(ControlCommandTest::testPreventAttach)) { }
ControlCommandTest() : tpunit::TestFixture("ControlCommand") {
registerTests(BEFORE_CLASS(ControlCommandTest::setup),
AFTER_CLASS(ControlCommandTest::teardown),
TEST(ControlCommandTest::testPreventAttach));
}


BedrockClusterTester* tester;

Expand Down
10 changes: 6 additions & 4 deletions test/clustertest/tests/EscalateTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
#include <test/clustertest/BedrockClusterTester.h>

struct EscalateTest : tpunit::TestFixture {
EscalateTest() : tpunit::TestFixture("Escalate", BEFORE_CLASS(EscalateTest::setup),
AFTER_CLASS(EscalateTest::teardown),
TEST(EscalateTest::test),
TEST(EscalateTest::socketReuse)) { }
EscalateTest() : tpunit::TestFixture("Escalate") {
registerTests(BEFORE_CLASS(EscalateTest::setup),
AFTER_CLASS(EscalateTest::teardown),
TEST(EscalateTest::test),
TEST(EscalateTest::socketReuse));
}

BedrockClusterTester* tester = nullptr;

Expand Down
10 changes: 5 additions & 5 deletions test/clustertest/tests/FastStandDownTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@
* Unfortunately these are very hard to do, and the expectation is that these cases work very similarly to the cases we DO have tests for.
*/
struct FastStandDownTest : tpunit::TestFixture {
FastStandDownTest()
: tpunit::TestFixture("FastStandDown",
BEFORE_CLASS(FastStandDownTest::setup),
FastStandDownTest() : tpunit::TestFixture("FastStandDown") {
registerTests(BEFORE_CLASS(FastStandDownTest::setup),
AFTER_CLASS(FastStandDownTest::teardown),
TEST(FastStandDownTest::testHTTPSRequests),
TEST(FastStandDownTest::testFutureCommands)) { }
TEST(FastStandDownTest::testFutureCommands));
}

BedrockClusterTester* tester;

Expand Down Expand Up @@ -109,7 +109,7 @@ struct FastStandDownTest : tpunit::TestFixture {

// Verify the DB on the new leader contains the commit.
SData lookupQuery("Query");
lookupQuery["Query"] = "SELECT COUNT(*) FROM test WHERE id = " + insertQuery["commandExecuteTime"] + " AND value = 'escalated_in_the_past';";
lookupQuery["Query"] = "SELECT COUNT(*) FROM test WHERE id = " + insertQuery["commandExecuteTime"] + " AND value = 'escalated_in_the_past';";
httpsResult = tester->getTester(1).executeWaitMultipleData({lookupQuery}, 1);
ASSERT_EQUAL(httpsResult[0].content, "COUNT(*)\n1\n");
}
Expand Down
42 changes: 21 additions & 21 deletions test/clustertest/tests/FinishJobTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,26 @@
#include <test/tests/jobs/JobTestHelper.h>

struct FinishJobTest : tpunit::TestFixture {
FinishJobTest()
: tpunit::TestFixture("FinishJob",
BEFORE_CLASS(FinishJobTest::setupClass),
TEST(FinishJobTest::nonExistentJob),
TEST(FinishJobTest::notInRunningState),
TEST(FinishJobTest::parentIsNotPaused),
TEST(FinishJobTest::removeFinishedAndCancelledChildren),
TEST(FinishJobTest::updateData),
TEST(FinishJobTest::finishingParentUnPausesChildren),
TEST(FinishJobTest::deleteFinishedJobWithNoChildren),
TEST(FinishJobTest::hasRepeat),
TEST(FinishJobTest::inRunqueuedState),
TEST(FinishJobTest::hasRepeatWithDelay),
TEST(FinishJobTest::hasDelay),
TEST(FinishJobTest::hasRepeatWithNextRun),
TEST(FinishJobTest::hasDataDelete),
TEST(FinishJobTest::hasNextRun),
TEST(FinishJobTest::simpleFinishJobWithHttp),
AFTER(FinishJobTest::tearDown),
AFTER_CLASS(FinishJobTest::tearDownClass)) { }
FinishJobTest() : tpunit::TestFixture("FinishJob") {
registerTests(BEFORE_CLASS(FinishJobTest::setupClass),
TEST(FinishJobTest::nonExistentJob),
TEST(FinishJobTest::notInRunningState),
TEST(FinishJobTest::parentIsNotPaused),
TEST(FinishJobTest::removeFinishedAndCancelledChildren),
TEST(FinishJobTest::updateData),
TEST(FinishJobTest::finishingParentUnPausesChildren),
TEST(FinishJobTest::deleteFinishedJobWithNoChildren),
TEST(FinishJobTest::hasRepeat),
TEST(FinishJobTest::inRunqueuedState),
TEST(FinishJobTest::hasRepeatWithDelay),
TEST(FinishJobTest::hasDelay),
TEST(FinishJobTest::hasRepeatWithNextRun),
TEST(FinishJobTest::hasDataDelete),
TEST(FinishJobTest::hasNextRun),
TEST(FinishJobTest::simpleFinishJobWithHttp),
AFTER(FinishJobTest::tearDown),
AFTER_CLASS(FinishJobTest::tearDownClass));
}

BedrockClusterTester* clusterTester;
BedrockTester* tester;
Expand Down Expand Up @@ -267,7 +267,7 @@ struct FinishJobTest : tpunit::TestFixture {
ASSERT_EQUAL(row[1], "QUEUED");
} else if (row[0] == cancelledChildID) {
ASSERT_EQUAL(row[1], "QUEUED");
} else {
} else {
ASSERT_TRUE(false);
}
}
Expand Down
5 changes: 3 additions & 2 deletions test/clustertest/tests/ForkCheckTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
#include <test/clustertest/BedrockClusterTester.h>

struct ForkCheckTest : tpunit::TestFixture {
ForkCheckTest()
: tpunit::TestFixture("ForkCheck", TEST(ForkCheckTest::test)) {}
ForkCheckTest() : tpunit::TestFixture("ForkCheck") {
registerTests(TEST(ForkCheckTest::test));
}

pair<uint64_t, string> getMaxJournalCommit(BedrockTester& tester, bool online = true) {
SQResult journals;
Expand Down
5 changes: 3 additions & 2 deletions test/clustertest/tests/ForkedNodeApprovalTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
#include <test/clustertest/BedrockClusterTester.h>

struct ForkedNodeApprovalTest : tpunit::TestFixture {
ForkedNodeApprovalTest()
: tpunit::TestFixture("ForkedNodeApproval", TEST(ForkedNodeApprovalTest::test)) {}
ForkedNodeApprovalTest() : tpunit::TestFixture("ForkedNodeApproval") {
registerTests(TEST(ForkedNodeApprovalTest::test));
}

pair<uint64_t, string> getMaxJournalCommit(BedrockTester& tester, bool online = true) {
SQResult journals;
Expand Down
14 changes: 7 additions & 7 deletions test/clustertest/tests/FutureExecutionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
#include <test/clustertest/BedrockClusterTester.h>

struct FutureExecutionTest : tpunit::TestFixture {
FutureExecutionTest()
: tpunit::TestFixture("FutureExecution",
BEFORE_CLASS(FutureExecutionTest::setup),
AFTER_CLASS(FutureExecutionTest::teardown),
TEST(FutureExecutionTest::FutureExecution),
TEST(FutureExecutionTest::FutureExecutionTimeout)) { }
FutureExecutionTest() : tpunit::TestFixture("FutureExecution") {
registerTests(BEFORE_CLASS(FutureExecutionTest::setup),
AFTER_CLASS(FutureExecutionTest::teardown),
TEST(FutureExecutionTest::FutureExecution),
TEST(FutureExecutionTest::FutureExecutionTimeout));
}

BedrockClusterTester* tester;

Expand All @@ -31,7 +31,7 @@ struct FutureExecutionTest : tpunit::TestFixture {
// Three seconds from now.
query["commandExecuteTime"] = to_string(STimeNow() + 3000000);
query["Query"] = "INSERT INTO test VALUES(" + SQ(50011) + ", " + SQ("sent_by_leader") + ");";
string result = brtester.executeWaitVerifyContent(query, "202");
string result = brtester.executeWaitVerifyContent(query, "202");

// Ok, Now let's wait a second
sleep(1);
Expand Down
11 changes: 5 additions & 6 deletions test/clustertest/tests/GracefulFailoverTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
#include <test/clustertest/BedrockClusterTester.h>

struct GracefulFailoverTest : tpunit::TestFixture {
GracefulFailoverTest()
: tpunit::TestFixture("GracefulFailover",
BEFORE_CLASS(GracefulFailoverTest::setup),
AFTER_CLASS(GracefulFailoverTest::teardown),
TEST(GracefulFailoverTest::test)
) { }
GracefulFailoverTest() : tpunit::TestFixture("GracefulFailover") {
registerTests(BEFORE_CLASS(GracefulFailoverTest::setup),
AFTER_CLASS(GracefulFailoverTest::teardown),
TEST(GracefulFailoverTest::test));
}

BedrockClusterTester* tester;

Expand Down
Loading
Loading