asan for all tests (#748)

* docker/Dockerfile: add odyssey build type To check bugs in asan builds of existing tests Signed-off-by: rkhapov <[email protected]> * test_scram.sh: fix asan run Signed-off-by: rkhapov <[email protected]> * sources/storage.c: fix strtol usage The msg string can be not null-terminated. Signed-off-by: rkhapov <[email protected]> * docker/Dockerfile: add vim Useful for in-container debugging Signed-off-by: rkhapov <[email protected]> * test-lag.sh: add odyssey start waiting To ensure odyssey started in some slow builds, like asan Signed-off-by: rkhapov <[email protected]> * sources/storage.c: rewrite watchdog It contained use-after-free bug because of spagetti-code Preformed little refactor A little bit slower (for each repl lag query we allocate new client), but doesnt significant because replication lag checking is performed once per second. Signed-off-by: rkhapov <[email protected]> * test-lag.sh: add ody-stop seems like it was missed Signed-off-by: rkhapov <[email protected]> --------- Signed-off-by: rkhapov <[email protected]> Co-authored-by: rkhapov <[email protected]>
yandex · Jan 11, 2025 · 104550b · 104550b
1 parent 7f18c34
commit 104550b
Show file tree

Hide file tree

Showing 13 changed files with 208 additions and 124 deletions.
diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml
@@ -32,11 +32,16 @@ jobs:
         submodules: recursive
     - name: Format
       run: make fmt
-    - name: Test
+    - name: Release Build Test
       env:
         CC: ${{ matrix.compiler }}
         CC_FOR_BUILD: ${{ matrix.compiler }}
       run: make run_test
+    - name: Asan Build Test
+      env:
+        CC: ${{ matrix.compiler }}
+        CC_FOR_BUILD: ${{ matrix.compiler }}
+      run: make run_test_asan
     - name: Prepare for release
       run: tar -zcf odyssey.linux-amd64.$(git rev-parse --short HEAD).tar.gz sources
     - name: Release

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -62,9 +62,9 @@ elseif("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
         set(CMAKE_C_FLAGS "-std=${CLANGCOMPILERSTANDART} -pedantic -Wall -Wextra -Wstrict-aliasing -g -O0 -pthread -D_GNU_SOURCE -DLDAP_DBG")
         set(OD_DEVEL_LVL 1)
 elseif("${CMAKE_BUILD_TYPE}" STREQUAL "ASAN")
-        set(CMAKE_C_FLAGS "-std=${CLANGCOMPILERSTANDART} -g -fsanitize=address,undefined -fno-sanitize-recover=all -pedantic -Wall -Wextra -Wstrict-aliasing -g -O0 -pthread -D_GNU_SOURCE")
+        set(CMAKE_C_FLAGS "-std=${CLANGCOMPILERSTANDART} -g -fsanitize=address,undefined,leak -fno-sanitize-recover=all -fno-omit-frame-pointer -pedantic -Wall -Wextra -Wstrict-aliasing -g -O0 -pthread -D_GNU_SOURCE")
 elseif("${CMAKE_BUILD_TYPE}" STREQUAL "TSAN")
-        set(CMAKE_C_FLAGS "-std=${CLANGCOMPILERSTANDART} -g -fsanitize=thread -fno-sanitize-recover=all -pedantic -Wall -Wextra -Wstrict-aliasing -g -O0 -pthread -D_GNU_SOURCE")
+        set(CMAKE_C_FLAGS "-std=${CLANGCOMPILERSTANDART} -g -fsanitize=thread -fno-sanitize-recover=all -fno-omit-frame-pointer -pedantic -Wall -Wextra -Wstrict-aliasing -g -O0 -pthread -D_GNU_SOURCE")
 endif()
 
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CC_FLAGS}")

diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
-BUILD_TEST_DIR=build-dbg
+BUILD_TEST_DIR=build
 BUILD_REL_DIR=build
-BUILD_TEST_ASAN_DIR=build-asan
+BUILD_TEST_ASAN_DIR=build
 ODY_DIR=$(PWD)
 TMP_BIN:=$(ODY_DIR)/tmp
 
@@ -57,11 +57,7 @@ apply_fmt:
 build_asan:
 	rm -rf $(BUILD_TEST_ASAN_DIR)
 	mkdir -p $(BUILD_TEST_ASAN_DIR)
-	cd $(BUILD_TEST_ASAN_DIR) && $(CMAKE_BIN) -DCMAKE_BUILD_TYPE=ASAN $(ODY_DIR) && make -j$(CONCURRENCY)
-
-copy_asan_bin:
-	cp $(BUILD_TEST_ASAN_DIR)/sources/odyssey ./docker/bin/odyssey-asan
-	cp $(BUILD_TEST_ASAN_DIR)/test/odyssey_test ./docker/bin/odyssey_test_asan
+	cd $(BUILD_TEST_ASAN_DIR) && $(CMAKE_BIN) -DCMAKE_BUILD_TYPE=ASAN $(ODY_DIR) $(CMAKE_FLAGS) && make -j$(CONCURRENCY)
 
 build_release:
 	rm -rf $(BUILD_REL_DIR)
@@ -79,7 +75,11 @@ gdb: build_dbg
 run_test:
 	# change dir, test would not work with absolute path
 	./cleanup-docker.sh
-	docker compose -f ./docker-compose-test.yml up --exit-code-from odyssey
+	ODYSSEY_TEST_BUILD_TYPE=build_release docker compose -f ./docker-compose-test.yml up --exit-code-from odyssey
+
+run_test_asan:
+	./cleanup-docker.sh
+	ODYSSEY_TEST_BUILD_TYPE=build_asan docker compose -f ./docker-compose-test.yml up --exit-code-from odyssey
 
 submit-cov:
 	mkdir cov-build && cd cov-build

diff --git a/docker-compose-test.yml b/docker-compose-test.yml
@@ -12,6 +12,8 @@ services:
     build:
       dockerfile: ./docker/Dockerfile
       context: .
+      args:
+        odyssey_build_type: "${ODYSSEY_TEST_BUILD_TYPE:-build_release}"
     environment:
       CMAKE_BUILD_TYPE: "${CMAKE_BUILD_TYPE:-Debug}"
     volumes:

diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -16,13 +16,13 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 
 FROM odyssey-build-env AS odyssey-build
 
+ARG odyssey_build_type
+
 RUN mkdir build_dir
 COPY . /build_dir
 
 WORKDIR /build_dir
-RUN make build_release
-RUN make build_dbg
-RUN make build_asan
+RUN make ${odyssey_build_type}
 
 FROM golang:1.23-alpine AS golang-tests-builder
 
@@ -70,6 +70,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
     postgresql-server-dev-16 \
     sudo \
     git \
+    vim \
     docker-ce docker-ce-cli containerd.io
 
 RUN mkdir /tmp/odyssey
@@ -88,13 +89,7 @@ COPY --from=golang-tests-builder /config-validation/pkg/config-validation /confi
 COPY --from=odyssey-build /build_dir/build/sources/odyssey /usr/bin/odyssey
 COPY --from=odyssey-build /build_dir/build/test/odyssey_test /usr/bin/odyssey_test
 
-COPY --from=odyssey-build /build_dir/build-dbg/sources/odyssey /usr/bin/odyssey-dbg
-COPY --from=odyssey-build /build_dir/build-dbg/test/odyssey_test /usr/bin/odyssey_test-dbg
-
-COPY --from=odyssey-build /build_dir/build-asan/sources/odyssey /usr/bin/odyssey-asan
-COPY --from=odyssey-build /build_dir/build-asan/test/odyssey_test /usr/bin/odyssey_test_asan
-
-COPY --from=odyssey-build /build_dir/build-asan/test/machinarium /machinarium
+COPY --from=odyssey-build /build_dir/build/test/machinarium /machinarium
 
 COPY ./docker/bin/ody-restart /usr/bin/ody-restart
 COPY ./docker/bin/ody-start  /usr/bin/ody-start

diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
@@ -4,7 +4,9 @@ set -ex
 
 /usr/bin/odyssey_test
 
-/usr/bin/odyssey_test_asan
+/usr/bin/odyssey /etc/odyssey/odyssey.conf
+sleep 5
+ody-stop
 
 setup
 
@@ -106,10 +108,6 @@ then
 fi
 echo "" > /var/log/odyssey.log
 
-/usr/bin/odyssey-asan /etc/odyssey/odyssey.conf
-sleep 5
-ody-stop
-
 # TODO: rewrite
 #/shell-test/test.sh
 /shell-test/console_role_test.sh

diff --git a/docker/lagpolling/test-lag.sh b/docker/lagpolling/test-lag.sh
@@ -2,6 +2,7 @@
 
 
 /usr/bin/odyssey /etc/odyssey/lag-conf.conf
+sleep 3
 
 for _ in $(seq 1 3); do
 	PGPASSWORD=lolol psql -h localhost -p6432 -dpostgres -Uuser1 -c 'select 3' && exit 1 
@@ -26,3 +27,5 @@ PGPASSWORD=lolol psql -h localhost -p6432 -dpostgres -Uuser1 -c 'select 3' || tr
 for _ in $(seq 1 3); do
 	PGPASSWORD=lolol psql -h localhost -p6432 -dpostgres -Uuser1 -c 'select 3' || exit 1 
 done
+
+ody-stop
diff --git a/docker/scram/test_scram.sh b/docker/scram/test_scram.sh
@@ -1,6 +1,7 @@
 #!/bin/bash -x
 
 /usr/bin/odyssey /scram/config.conf
+sleep 3
 
 /scram/test_scram_backend.sh
 if [ $? -eq 1 ]

diff --git a/sources/extension.h b/sources/extension.h
@@ -12,6 +12,10 @@ struct od_extension {
 static inline od_retcode_t od_extensions_init(od_extension_t *extensions)
 {
 	extensions->modules = malloc(sizeof(od_module_t));
+	if (extensions->modules == NULL) {
+		return 1;
+	}
+
 	od_modules_init(extensions->modules);
 
 	return OK_RESPONSE;

diff --git a/sources/instance.c b/sources/instance.c
@@ -61,7 +61,11 @@ void od_config_testing(od_instance_t *instance)
 	od_error_init(&error);
 	od_router_init(&router, &global);
 	od_hba_init(&hba);
-	od_extensions_init(&extensions);
+	if (od_extensions_init(&extensions) != 0) {
+		od_error(&instance->logger, "config", NULL, NULL,
+			 "failed to init extensions");
+		goto error;
+	};
 
 	int rc;
 	rc = od_config_reader_import(&instance->config, &router.rules, &error,
@@ -144,7 +148,13 @@ int od_instance_main(od_instance_t *instance, int argc, char **argv)
 	od_router_init(&router, &global);
 	od_cron_init(&cron);
 	od_worker_pool_init(&worker_pool);
-	od_extensions_init(&extensions);
+
+	if (od_extensions_init(&extensions) != 0) {
+		od_error(&instance->logger, "config", NULL, NULL,
+			 "failed to extensions init");
+		goto error;
+	}
+
 	od_hba_init(&hba);
 	od_global_init(&global, instance, &system, &router, &cron, &worker_pool,
 		       &extensions, &hba);

diff --git a/sources/sighandler.c b/sources/sighandler.c
@@ -58,11 +58,11 @@ od_attribute_noreturn() void od_system_shutdown(od_system_t *system,
 	/* Prevent OpenSSL usage during deinitialization */
 	od_worker_pool_wait();
 
+	od_extension_free(&instance->logger, system->global->extensions);
+
 #ifdef OD_SYSTEM_SHUTDOWN_CLEANUP
 	od_router_free(system->global->router);
 
-	od_extension_free(&instance->logger, system->global->extensions);
-
 	od_system_cleanup(system);
 
 	/* stop machinaruim and free */