From 451d6449631139a8b3647895eae5cf1bda15984e Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Mon, 13 Jan 2025 13:33:44 +0000 Subject: [PATCH 1/6] Remove alloc_dealloc_mismatch option and pass-through --- CMakeLists.txt | 7 ------- cmake/common.cmake | 7 ------- tests/e2e_operations.py | 3 +-- tests/infra/remote.py | 5 +---- 4 files changed, 2 insertions(+), 20 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b2142ca5fea7..0acbef712323 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -935,13 +935,6 @@ if(BUILD_TESTS) PROPERTY LABELS unit_test ) - # https://github.com/microsoft/CCF/issues/5198 - set_property( - TEST csr_test - APPEND - PROPERTY ENVIRONMENT "ASAN_OPTIONS=alloc_dealloc_mismatch=0" - ) - add_test(NAME versionifier_test COMMAND ${PYTHON} ${CMAKE_SOURCE_DIR}/python/src/ccf/_versionifier.py diff --git a/cmake/common.cmake b/cmake/common.cmake index ac50801edb92..877ca7fd346c 100644 --- a/cmake/common.cmake +++ b/cmake/common.cmake @@ -26,13 +26,6 @@ function(add_unit_test name) "TSAN_OPTIONS=suppressions=${CCF_DIR}/tsan_env_suppressions" ) - # https://github.com/microsoft/CCF/issues/5198 - set_property( - TEST ${name} - APPEND - PROPERTY ENVIRONMENT "ASAN_OPTIONS=alloc_dealloc_mismatch=0" - ) - endfunction() # Test binary wrapper diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 7655432e00fb..573aa6e894d0 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -395,7 +395,6 @@ def run_config_timeout_check(args): env = {} if args.enclave_platform == "snp": env = snp.get_aci_env() - env["ASAN_OPTIONS"] = "alloc_dealloc_mismatch=0" proc = subprocess.Popen( [ @@ -466,7 +465,7 @@ def run_configuration_file_checks(args): for config in config_files_to_check: cmd = [bin_path, f"--config={config}", "--check"] rc = infra.proc.ccall( - *cmd, env={"ASAN_OPTIONS": "alloc_dealloc_mismatch=0"} + *cmd, ).returncode assert rc == 0, f"Failed to check configuration: {rc}" LOG.success(f"Successfully check sample configuration file {config}") diff --git a/tests/infra/remote.py b/tests/infra/remote.py index e7522d0734ef..99bc708656d2 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -361,10 +361,7 @@ def __init__( if ubsan_opts: env["UBSAN_OPTIONS"] += ":" + ubsan_opts env["TSAN_OPTIONS"] = os.environ.get("TSAN_OPTIONS", "") - # https://github.com/microsoft/CCF/issues/5198 - env["ASAN_OPTIONS"] = os.environ.get( - "ASAN_OPTIONS", "alloc_dealloc_mismatch=0" - ) + env["ASAN_OPTIONS"] = os.environ.get("ASAN_OPTIONS", "") elif enclave_platform == "snp": env = snp.get_aci_env() snp_security_context_directory_envvar = ( From e4f59ba6d419b070d40e1786e2b0d303d8e6714e Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Mon, 13 Jan 2025 13:34:51 +0000 Subject: [PATCH 2/6] Change std lib in ASAN build --- .azure-pipelines-templates/daily-matrix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azure-pipelines-templates/daily-matrix.yml b/.azure-pipelines-templates/daily-matrix.yml index 5dafafda7eba..7a362aa62228 100644 --- a/.azure-pipelines-templates/daily-matrix.yml +++ b/.azure-pipelines-templates/daily-matrix.yml @@ -33,7 +33,7 @@ parameters: cmake_args: "-DCMAKE_BUILD_TYPE=Debug -DLVI_MITIGATIONS=OFF" cmake_env: "" ASAN: - cmake_args: "-DSAN=ON" + cmake_args: "-DSAN=ON -DUSELIBCXX=OFF" cmake_env: "" TSAN: cmake_args: "-DTSAN=ON -DWORKER_THREADS=2" From df6df24a715c7008611e634992293724dcfaa2fd Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Mon, 13 Jan 2025 14:16:20 +0000 Subject: [PATCH 3/6] Format --- .azure-pipelines-templates/daily-matrix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azure-pipelines-templates/daily-matrix.yml b/.azure-pipelines-templates/daily-matrix.yml index 7a362aa62228..6d7dae22f294 100644 --- a/.azure-pipelines-templates/daily-matrix.yml +++ b/.azure-pipelines-templates/daily-matrix.yml @@ -33,7 +33,7 @@ parameters: cmake_args: "-DCMAKE_BUILD_TYPE=Debug -DLVI_MITIGATIONS=OFF" cmake_env: "" ASAN: - cmake_args: "-DSAN=ON -DUSELIBCXX=OFF" + cmake_args: "-DSAN=ON -DUSE_LIBCXX=OFF" cmake_env: "" TSAN: cmake_args: "-DTSAN=ON -DWORKER_THREADS=2" From fb733adbfb9bdeb136c9d39a04376775b10448ae Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Tue, 14 Jan 2025 17:43:48 +0000 Subject: [PATCH 4/6] How did I miss this... --- .github/workflows/long-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/long-test.yml b/.github/workflows/long-test.yml index d216aa08c507..8a992c5e80d0 100644 --- a/.github/workflows/long-test.yml +++ b/.github/workflows/long-test.yml @@ -53,7 +53,7 @@ jobs: git config --global --add safe.directory "$GITHUB_WORKSPACE" mkdir build cd build - cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TESTS=ON -DLVI_MITIGATIONS=OFF -DSAN=ON .. + cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TESTS=ON -DLVI_MITIGATIONS=OFF -DSAN=ON -DUSE_LIBCXX=OFF .. ninja - name: "Test" From 3ebb7161c42a1e0c255e65affb3a4d29d48999b7 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Wed, 15 Jan 2025 11:36:51 +0000 Subject: [PATCH 5/6] Avoid deref unaligned pointer --- src/ds/ring_buffer.h | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ds/ring_buffer.h b/src/ds/ring_buffer.h index ea662a403c4c..eb7536298c77 100644 --- a/src/ds/ring_buffer.h +++ b/src/ds/ring_buffer.h @@ -145,19 +145,24 @@ namespace ringbuffer { static inline uint64_t read64_impl(const BufferDef& bd, size_t index) { + auto src = bd.data + index; + auto src_64 = reinterpret_cast(src); + #ifdef __cpp_lib_atomic_ref - auto& ref = *(reinterpret_cast(bd.data + index)); - std::atomic_ref slot(ref); - return slot.load(std::memory_order_acquire); -#else - // __atomic_load is used instead of std::atomic_ref since it's not - // supported by libc++ yet. + if (Const::is_aligned(src, 8)) + { + auto& ref = *src_64; + std::atomic_ref slot(ref); + return slot.load(std::memory_order_acquire); + } +#endif + + // __atomic_load is used instead of std::atomic_ref when std::atomic_ref + // is unavailable, or the src pointer is not aligned // https://en.cppreference.com/w/Template:cpp/compiler_support/20 uint64_t r = 0; - __atomic_load( - reinterpret_cast(bd.data + index), &r, __ATOMIC_ACQUIRE); + __atomic_load(src_64, &r, __ATOMIC_ACQUIRE); return r; -#endif } static inline Message message(uint64_t header) From c860dd08a6539da0f7b10661bcc61665a193e2a2 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 23 Jan 2025 15:23:59 +0000 Subject: [PATCH 6/6] Ok --- include/ccf/http_accept.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ccf/http_accept.h b/include/ccf/http_accept.h index fc5f175e5f2c..53093bfde33b 100644 --- a/include/ccf/http_accept.h +++ b/include/ccf/http_accept.h @@ -46,7 +46,7 @@ namespace ccf::http bool operator<(const AcceptHeaderField& other) const { static constexpr auto float_comp_epsilon = 0.0000001f; - if (abs(q_factor - other.q_factor) > float_comp_epsilon) + if (std::abs(q_factor - other.q_factor) > float_comp_epsilon) { return q_factor < other.q_factor; }