From 5a4c0640cebe922de563e03ff2a683b89612f522 Mon Sep 17 00:00:00 2001 From: Madelyn Olson Date: Tue, 29 Oct 2024 14:26:17 -0700 Subject: [PATCH] Mark main and serverAssert as weak symbols to be overridden (#1232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At some point unit tests stopped building on MacOS because of duplicate symbols. I had originally solved this problem by using a flag that overrides symbols, but the much better solution is to mark the duplicate symbols as weak and they can be overridden during linking. (Symbols by default are strong, strong symbols override weak symbols) I also added macos unit build to the CI, so that this doesn't silently break in the future again. --------- Signed-off-by: Madelyn Olson Co-authored-by: Viktor Söderqvist --- .github/workflows/ci.yml | 2 +- src/Makefile | 11 +---------- src/debug.c | 2 +- src/server.c | 3 ++- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1d23f40fa..48a94ef984 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,7 +83,7 @@ jobs: steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 - name: make - run: make -j3 SERVER_CFLAGS='-Werror' + run: make -j3 all-with-unit-tests SERVER_CFLAGS='-Werror' build-32bit: runs-on: ubuntu-latest diff --git a/src/Makefile b/src/Makefile index 020b70d6d5..ae2de1c626 100644 --- a/src/Makefile +++ b/src/Makefile @@ -98,15 +98,6 @@ ifeq ($(USE_JEMALLOC),no) MALLOC=libc endif -# Some unit tests compile files a second time to get access to static functions, the "--allow-multiple-definition" flag -# allows us to do that without an error, by using the first instance of function. This behavior can also be used -# to tweak behavior of code just for unit tests. The version of ld on MacOS apparently always does this. -ifneq ($(uname_S),Darwin) - ALLOW_DUPLICATE_FLAG=-Wl,--allow-multiple-definition -else - ALLOW_DUPLICATE_FLAG= -endif - ifdef SANITIZER ifeq ($(SANITIZER),address) MALLOC=libc @@ -494,7 +485,7 @@ $(ENGINE_LIB_NAME): $(ENGINE_SERVER_OBJ) # valkey-unit-tests $(ENGINE_UNIT_TESTS): $(ENGINE_TEST_OBJ) $(ENGINE_LIB_NAME) - $(SERVER_LD) $(ALLOW_DUPLICATE_FLAG) -o $@ $^ ../deps/hiredis/libhiredis.a ../deps/lua/src/liblua.a ../deps/hdr_histogram/libhdrhistogram.a ../deps/fpconv/libfpconv.a $(FINAL_LIBS) + $(SERVER_LD) -o $@ $^ ../deps/hiredis/libhiredis.a ../deps/lua/src/liblua.a ../deps/hdr_histogram/libhdrhistogram.a ../deps/fpconv/libfpconv.a $(FINAL_LIBS) # valkey-sentinel $(ENGINE_SENTINEL_NAME): $(SERVER_NAME) diff --git a/src/debug.c b/src/debug.c index 98512fd436..d221a884ee 100644 --- a/src/debug.c +++ b/src/debug.c @@ -1023,7 +1023,7 @@ void debugCommand(client *c) { /* =========================== Crash handling ============================== */ -__attribute__((noinline)) void _serverAssert(const char *estr, const char *file, int line) { +__attribute__((noinline, weak)) void _serverAssert(const char *estr, const char *file, int line) { int new_report = bugReportStart(); serverLog(LL_WARNING, "=== %sASSERTION FAILED ===", new_report ? "" : "RECURSIVE "); serverLog(LL_WARNING, "==> %s:%d '%s' is not true", file, line, estr); diff --git a/src/server.c b/src/server.c index e95012eefa..508edc7112 100644 --- a/src/server.c +++ b/src/server.c @@ -6810,7 +6810,8 @@ serverTestProc *getTestProcByName(const char *name) { } #endif -int main(int argc, char **argv) { +/* Main is marked as weak so that unit tests can use their own main function. */ +__attribute__((weak)) int main(int argc, char **argv) { struct timeval tv; int j; char config_from_stdin = 0;