Skip to content

Commit

Permalink
Mark main and serverAssert as weak symbols to be overridden (valkey-i…
Browse files Browse the repository at this point in the history
…o#1232)

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 <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
  • Loading branch information
madolson and zuiderkwast authored Oct 29, 2024
1 parent 8ee7a58 commit 5a4c064
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 1 addition & 10 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 5a4c064

Please sign in to comment.