From 21f979ad36def5fd0f7163540e161b6e0db697ed Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Fri, 6 Sep 2024 19:28:07 +0000
Subject: [PATCH 01/26] ci: add ctest memcheck test

* create a buildspec file which contains ctest memcheck command
---
 codebuild/spec/buildspec_valgrind.yml | 116 ++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
 create mode 100644 codebuild/spec/buildspec_valgrind.yml

diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
new file mode 100644
index 00000000000..b4b81a8656e
--- /dev/null
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -0,0 +1,116 @@
+---
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use
+# this file except in compliance with the License. A copy of the License is
+# located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied. See the License for the specific language governing permissions and
+# limitations under the License.
+version: 0.2
+
+# This buildspec runs on an Ubuntu22 image. That configuration is a property of
+# the codebuild job itself.
+
+# Codebuild's matrix jobs have non-differentiated names so use batch-list
+# instead.
+
+# Parameter motivation
+
+# COMPILERS
+# We run asan on both gcc and clang because of different features sets for their
+# address sanitizers. Specifically there was a case where GCC was able to detect
+# a memcpy-param-overlap that Clang did not.
+
+# LIBCRYPTOS
+# awslc: happy path libcrypto for s2n-tls
+# openssl 3: s2n-tls takes different code paths for ossl3, so make sure we run 
+#            asan on it. See pr 4033 for a historical motivating example.
+# openssl 1.1.1: a widely deployed version of openssl.
+# openssl 1.0.2: the default libcrypto on AL2, and AL2 is still widely deployed.
+
+# CMAKE_BUILD_TYPE
+# RelWithDebInfo: This instructs CMake to do all optimizations (Rel -> Release)
+# along with debug info (DebInfo). Debug info is necessary to get line numbers 
+# in the stack traces that ASAN reports.
+batch:
+  build-list:
+    - identifier: clang_awslc
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: awslc
+          COMPILER: clang
+    - identifier: clang_openssl_3_0
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: openssl-3.0
+          COMPILER: clang
+    - identifier: clang_openssl_1_1_1
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: openssl-1.1.1
+          COMPILER: clang
+    - identifier: clang_openssl_1_0_2
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: openssl-1.0.2
+          COMPILER: clang
+    - identifier: gcc_awslc
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: awslc
+          COMPILER: gcc
+    - identifier: gcc_openssl_3_0
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        variables:
+          S2N_LIBCRYPTO: openssl-3.0
+          COMPILER: gcc
+
+phases:
+  pre_build:
+    commands:
+      - |
+        if [ -d "third-party-src" ]; then
+          cd third-party-src;
+          ln -s /usr/local $CODEBUILD_SRC_DIR/third-party-src/test-deps;
+        fi
+      - /usr/bin/$COMPILER --version
+  build:
+    on-failure: ABORT
+    commands:
+      - |
+        cmake . -Bbuild \
+          -DCMAKE_C_COMPILER=/usr/bin/$COMPILER \
+          -DCMAKE_PREFIX_PATH=/usr/local/$S2N_LIBCRYPTO \
+          -DCMAKE_BUILD_TYPE=RelWithDebInfo
+      - cmake --build ./build -- -j $(nproc)
+  post_build:
+    on-failure: ABORT
+    commands:
+      - |    
+        if [[ "$S2N_LIBCRYPTO" == "openssl-1.1.1" ]]; then
+            echo "running task pedantic_valgrind"
+            S2N_DEBUG=true\ 
+            CTEST_OUTPUT_ON_FAILURE=1 \
+            CTEST_MEMORYCHECK_COMMAND_OPTIONS="${CTEST_MEMORYCHECK_COMMAND_OPTIONS_PEDANTIC}" \
+            S2N_VALGRIND=1 \
+            CTEST_PARALLEL_LEVEL=$(nproc) ctest -T memcheck --test-dir build \
+            --timeout 3000 ARGS="-output-on-failure"
+        else
+            S2N_DEBUG=true \
+            CTEST_OUTPUT_ON_FAILURE=1 \
+            CTEST_MEMORYCHECK_COMMAND_OPTIONS="${CTEST_MEMORYCHECK_COMMAND_OPTIONS_REGULAR}" \
+            S2N_VALGRIND=1 \
+            CTEST_PARALLEL_LEVEL=$(nproc) ctest -T memcheck --test-dir build \
+            --timeout 3000 ARGS="-output-on-failure"
+        fi

From 13f270ba93c57661b61d9cfb8c2310a08a35710a Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Mon, 9 Sep 2024 16:22:36 +0000
Subject: [PATCH 02/26] ci: add valgrind options into `CMakeLists.txt`

* append options for both regular and pedantic valgrind check above ASAN
  check
---
 CMakeLists.txt | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5ad6aea68d6..e40808261a7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -192,6 +192,29 @@ if(TSAN)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=thread)
 endif()
 
+set(CTEST_MEMORYCHECK_COMMAND_OPTIONS_REGULAR " \
+            --leak-check=full \
+            --leak-resolution=high \
+            --trace-children=yes \
+            --run-libc-freeres=no \
+            -q --error-exitcode=123 \
+            --error-limit=no \
+            --num-callers=40 \
+            --undef-value-errors=no")
+set(CTEST_MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC " \
+            --leak-check=full \
+            --leak-resolution=high \
+            --trace-children=yes \
+            --run-libc-freeres=no \
+            -q --error-exitcode=123 \
+            --error-limit=no \
+            --num-callers=40 \
+            --undef-value-errors=no \
+            --errors-for-leak-kinds=all \
+            --suppressions=valgrind.suppressions")
+
+set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+
 if(ASAN)
     target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=address -DS2N_ADDRESS_SANITIZER=1)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=address)

From d1c197df27ad28a8b41f420d30f3da52a1363d84 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Thu, 12 Sep 2024 23:18:51 +0000
Subject: [PATCH 03/26] ci: add CTest memcheck for codebuild

* set up valgrind options
* set up valgrind ci codebuild check
---
 CMakeLists.txt                        | 54 +++++++++++++++------------
 codebuild/spec/buildspec_valgrind.yml | 46 ++---------------------
 tests/unit/valgrind.suppressions      |  6 ++-
 3 files changed, 39 insertions(+), 67 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e40808261a7..60c2342dc18 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,6 +14,37 @@ set(INSTALL_CMAKE_DIR lib/cmake CACHE PATH "Installation directory for cmake fil
 
 set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE)
 
+set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+
+set(MEMORYCHECK_COMMAND_OPTIONS_REGULAR " \
+    --leak-check=full \
+    --leak-resolution=high \
+    --trace-children=yes \
+    --run-libc-freeres=no \
+    -q --error-exitcode=123 \
+    --error-limit=no \
+    --num-callers=40 \
+    --undef-value-errors=no \
+    --suppressions=valgrind.suppressions")
+
+set(MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC " \
+    --leak-check=full \
+    --leak-resolution=high \
+    --trace-children=yes \
+    --run-libc-freeres=no \
+    -q --error-exitcode=123 \
+    --error-limit=no \
+    --num-callers=40 \
+    --undef-value-errors=no \
+    --errors-for-leak-kinds=all \
+    --suppressions=valgrind.suppressions")
+
+if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
+    set(MEMORYCHECK_COMMAND_OPTIONS ${MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC})
+else()
+    set(MEMORYCHECK_COMMAND_OPTIONS ${MEMORYCHECK_COMMAND_OPTIONS_REGULAR})
+endif()
+
 # These Version numbers are for major updates only- we won't track minor/patch updates here.
 set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
@@ -192,29 +223,6 @@ if(TSAN)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=thread)
 endif()
 
-set(CTEST_MEMORYCHECK_COMMAND_OPTIONS_REGULAR " \
-            --leak-check=full \
-            --leak-resolution=high \
-            --trace-children=yes \
-            --run-libc-freeres=no \
-            -q --error-exitcode=123 \
-            --error-limit=no \
-            --num-callers=40 \
-            --undef-value-errors=no")
-set(CTEST_MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC " \
-            --leak-check=full \
-            --leak-resolution=high \
-            --trace-children=yes \
-            --run-libc-freeres=no \
-            -q --error-exitcode=123 \
-            --error-limit=no \
-            --num-callers=40 \
-            --undef-value-errors=no \
-            --errors-for-leak-kinds=all \
-            --suppressions=valgrind.suppressions")
-
-set(CTEST_MEMORYCHECK_TYPE "Valgrind")
-
 if(ASAN)
     target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=address -DS2N_ADDRESS_SANITIZER=1)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=address)
diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index b4b81a8656e..05b82802781 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -13,30 +13,6 @@
 # limitations under the License.
 version: 0.2
 
-# This buildspec runs on an Ubuntu22 image. That configuration is a property of
-# the codebuild job itself.
-
-# Codebuild's matrix jobs have non-differentiated names so use batch-list
-# instead.
-
-# Parameter motivation
-
-# COMPILERS
-# We run asan on both gcc and clang because of different features sets for their
-# address sanitizers. Specifically there was a case where GCC was able to detect
-# a memcpy-param-overlap that Clang did not.
-
-# LIBCRYPTOS
-# awslc: happy path libcrypto for s2n-tls
-# openssl 3: s2n-tls takes different code paths for ossl3, so make sure we run 
-#            asan on it. See pr 4033 for a historical motivating example.
-# openssl 1.1.1: a widely deployed version of openssl.
-# openssl 1.0.2: the default libcrypto on AL2, and AL2 is still widely deployed.
-
-# CMAKE_BUILD_TYPE
-# RelWithDebInfo: This instructs CMake to do all optimizations (Rel -> Release)
-# along with debug info (DebInfo). Debug info is necessary to get line numbers 
-# in the stack traces that ASAN reports.
 batch:
   build-list:
     - identifier: clang_awslc
@@ -82,7 +58,6 @@ phases:
       - |
         if [ -d "third-party-src" ]; then
           cd third-party-src;
-          ln -s /usr/local $CODEBUILD_SRC_DIR/third-party-src/test-deps;
         fi
       - /usr/bin/$COMPILER --version
   build:
@@ -97,20 +72,7 @@ phases:
   post_build:
     on-failure: ABORT
     commands:
-      - |    
-        if [[ "$S2N_LIBCRYPTO" == "openssl-1.1.1" ]]; then
-            echo "running task pedantic_valgrind"
-            S2N_DEBUG=true\ 
-            CTEST_OUTPUT_ON_FAILURE=1 \
-            CTEST_MEMORYCHECK_COMMAND_OPTIONS="${CTEST_MEMORYCHECK_COMMAND_OPTIONS_PEDANTIC}" \
-            S2N_VALGRIND=1 \
-            CTEST_PARALLEL_LEVEL=$(nproc) ctest -T memcheck --test-dir build \
-            --timeout 3000 ARGS="-output-on-failure"
-        else
-            S2N_DEBUG=true \
-            CTEST_OUTPUT_ON_FAILURE=1 \
-            CTEST_MEMORYCHECK_COMMAND_OPTIONS="${CTEST_MEMORYCHECK_COMMAND_OPTIONS_REGULAR}" \
-            S2N_VALGRIND=1 \
-            CTEST_PARALLEL_LEVEL=$(nproc) ctest -T memcheck --test-dir build \
-            --timeout 3000 ARGS="-output-on-failure"
-        fi
+      - |
+        S2N_DEBUG=true S2N_VALGRIND=1 \
+          CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck --test-dir build \
+          --timeout 3000 ARGS="-output-on-failure"
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 3662410a9bf..e5f0af5ef6f 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -1,13 +1,15 @@
 # It looks like valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
 {
-   pthred_false_positive
+   pthread_false_positive
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
+   ...
    fun:allocate_dtv
    fun:_dl_allocate_tls
    fun:allocate_stack
-   fun:pthread_create@@GLIBC_2.2.5
+   fun:pthread_create@@GLIBC_2.34
+   ...
    fun:main
 }
 

From ff78b5d45c0e26d5a1d5ac0959965882b058954c Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Mon, 16 Sep 2024 21:59:48 +0000
Subject: [PATCH 04/26] ci: adjusting valgrind suppressions and valgrind setup

---
 codebuild/spec/buildspec_valgrind.yml | 4 ++--
 tests/unit/valgrind.suppressions      | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 05b82802781..1b95fe9c3d0 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -74,5 +74,5 @@ phases:
     commands:
       - |
         S2N_DEBUG=true S2N_VALGRIND=1 \
-          CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck --test-dir build \
-          --timeout 3000 ARGS="-output-on-failure"
+          CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
+          --test-dir build ARGS="-output-on-failure"
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index e5f0af5ef6f..f6f0fc03d9a 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -4,7 +4,7 @@
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
-   ...
+   fun:calloc
    fun:allocate_dtv
    fun:_dl_allocate_tls
    fun:allocate_stack

From c02588aaf0184064457da7bfcd8c5e51ac588243 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Mon, 16 Sep 2024 23:24:01 +0000
Subject: [PATCH 05/26] ci: modify valgrind suppressions

---
 tests/unit/valgrind.suppressions | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index f6f0fc03d9a..95581c2e589 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -12,7 +12,31 @@
    ...
    fun:main
 }
-
+# Backtrace might generate false positives with its _dl_open function call: https://stackoverflow.com/questions/78525668/egl-memory-leak
+{
+   stacktrace_suppression
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   fun:malloc
+   fun:_dlfo_mappings_segment_allocate
+   fun:_dl_find_object_update_1
+   fun:_dl_find_object_update
+   fun:dl_open_worker_begin
+   fun:_dl_catch_exception
+   fun:dl_open_worker
+   fun:_dl_catch_exception
+   fun:_dl_open
+   fun:do_dlopen
+   fun:_dl_catch_exception
+   fun:_dl_catch_error
+   fun:dlerror_run
+   fun:__libc_dlopen_mode
+   fun:__libc_unwind_link_get
+   fun:backtrace
+   ...
+   fun:main
+}
 # TODO: fix the pedantic leak errors from s2n_fork_generation_number_test
 {
    ignore_s2n_fork_generation_number_test

From d4d0d526f46df529ea1b3811aee8b379327d744e Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 00:28:24 +0000
Subject: [PATCH 06/26] ci: fixing valgrind suppressions and reformat valgrind
 yml

---
 codebuild/spec/buildspec_valgrind.yml | 27 +--------------------------
 tests/unit/valgrind.suppressions      | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 1b95fe9c3d0..e89363086aa 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -15,30 +15,6 @@ version: 0.2
 
 batch:
   build-list:
-    - identifier: clang_awslc
-      env:
-        compute-type: BUILD_GENERAL1_LARGE
-        variables:
-          S2N_LIBCRYPTO: awslc
-          COMPILER: clang
-    - identifier: clang_openssl_3_0
-      env:
-        compute-type: BUILD_GENERAL1_LARGE
-        variables:
-          S2N_LIBCRYPTO: openssl-3.0
-          COMPILER: clang
-    - identifier: clang_openssl_1_1_1
-      env:
-        compute-type: BUILD_GENERAL1_LARGE
-        variables:
-          S2N_LIBCRYPTO: openssl-1.1.1
-          COMPILER: clang
-    - identifier: clang_openssl_1_0_2
-      env:
-        compute-type: BUILD_GENERAL1_LARGE
-        variables:
-          S2N_LIBCRYPTO: openssl-1.0.2
-          COMPILER: clang
     - identifier: gcc_awslc
       env:
         compute-type: BUILD_GENERAL1_LARGE
@@ -73,6 +49,5 @@ phases:
     on-failure: ABORT
     commands:
       - |
-        S2N_DEBUG=true S2N_VALGRIND=1 \
-          CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
+        S2N_VALGRIND=1 CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
           --test-dir build ARGS="-output-on-failure"
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 95581c2e589..896484d96a2 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -1,6 +1,18 @@
 # It looks like valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
 {
-   pthread_false_positive
+   pthred_false_positive
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:allocate_stack
+   fun:pthread_create@@GLIBC_2.2.5
+   fun:main
+}
+# Append valgrind suppression for ubuntu22
+{
+   pthread_false_positive_ubuntu22
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
@@ -33,6 +45,7 @@
    fun:dlerror_run
    fun:__libc_dlopen_mode
    fun:__libc_unwind_link_get
+   fun:__libc_unwind_link_get
    fun:backtrace
    ...
    fun:main

From 76aa3b43fde679ae8bc6089da998899babb803b6 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 00:29:50 +0000
Subject: [PATCH 07/26] ci: reformat buildspec_valgrind.yml

---
 codebuild/spec/buildspec_valgrind.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index e89363086aa..6ba75219d6d 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -49,5 +49,6 @@ phases:
     on-failure: ABORT
     commands:
       - |
-        S2N_VALGRIND=1 CTEST_PARALLEL_LEVEL=$(nproc) CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
+        S2N_VALGRIND=1 CTEST_PARALLEL_LEVEL=$(nproc) \
+          CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
           --test-dir build ARGS="-output-on-failure"

From 8e3b17c4803bcabcba6813311b6e98fbab72d2d4 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 17:30:45 +0000
Subject: [PATCH 08/26] ci: address PR comments

* add comments to `CMakeLists.txt` to explain pedantic valgrind check
  logic
* change variable names to be concise
* use `VALGRIND_DEFAULT` to define `VALGRIND_PEDANTIC`
* reformat the CTest memecheck command to make it more readable
* add explanations and necessary comments to `valgrind.suppressions`
---
 CMakeLists.txt                        | 25 +++++++++----------------
 codebuild/spec/buildspec_valgrind.yml |  8 +++++---
 tests/unit/valgrind.suppressions      | 12 +++++++++---
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 60c2342dc18..caf954872c3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,9 +14,7 @@ set(INSTALL_CMAKE_DIR lib/cmake CACHE PATH "Installation directory for cmake fil
 
 set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE)
 
-set(CTEST_MEMORYCHECK_TYPE "Valgrind")
-
-set(MEMORYCHECK_COMMAND_OPTIONS_REGULAR " \
+set(VALGRIND_DEFAULT " \
     --leak-check=full \
     --leak-resolution=high \
     --trace-children=yes \
@@ -27,22 +25,16 @@ set(MEMORYCHECK_COMMAND_OPTIONS_REGULAR " \
     --undef-value-errors=no \
     --suppressions=valgrind.suppressions")
 
-set(MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC " \
-    --leak-check=full \
-    --leak-resolution=high \
-    --trace-children=yes \
-    --run-libc-freeres=no \
-    -q --error-exitcode=123 \
-    --error-limit=no \
-    --num-callers=40 \
-    --undef-value-errors=no \
-    --errors-for-leak-kinds=all \
-    --suppressions=valgrind.suppressions")
+set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
 
+# Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
+# Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
+# valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
+# Tracking issue: #3758
 if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
-    set(MEMORYCHECK_COMMAND_OPTIONS ${MEMORYCHECK_COMMAND_OPTIONS_PENDANTIC})
+    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
 else()
-    set(MEMORYCHECK_COMMAND_OPTIONS ${MEMORYCHECK_COMMAND_OPTIONS_REGULAR})
+    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
 endif()
 
 # These Version numbers are for major updates only- we won't track minor/patch updates here.
@@ -497,6 +489,7 @@ if (BUILD_TESTING)
     add_library(allocator_overrides SHARED ${TEST_LD_PRELOAD})
 
     set(UNIT_TEST_ENVS S2N_DONT_MLOCK=1)
+    set(CTEST_MEMORYCHECK_TYPE "Valgrind")
     if (TSAN OR ASAN)
         set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} S2N_ADDRESS_SANITIZER=1)
     endif()
diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 6ba75219d6d..0c81f5bf5e2 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -49,6 +49,8 @@ phases:
     on-failure: ABORT
     commands:
       - |
-        S2N_VALGRIND=1 CTEST_PARALLEL_LEVEL=$(nproc) \
-          CTEST_OUTPUT_ON_FAILURE=1 ctest -T memcheck \
-          --test-dir build ARGS="-output-on-failure"
+        S2N_VALGRIND=1 \
+        CTEST_PARALLEL_LEVEL=$(nproc) \
+        CTEST_OUTPUT_ON_FAILURE=1 \
+        ctest -T memcheck \
+              --test-dir build
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 896484d96a2..e244964d8f8 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -1,6 +1,6 @@
 # It looks like valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
 {
-   pthred_false_positive
+   pthread_false_positive
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
@@ -10,7 +10,10 @@
    fun:pthread_create@@GLIBC_2.2.5
    fun:main
 }
-# Append valgrind suppression for ubuntu22
+# Append valgrind suppression for ubuntu22 and ubuntu24
+# If this suppression is not include, Test number 82, 95,113, 121, and 158 will fail
+# This block is the same suppression as the pthread suppressions above, but for a diffeent libC.
+# Adding tracking issue #4777. Remove the previous pthread suppressions block after new Ctest memcheck is fully integrated.
 {
    pthread_false_positive_ubuntu22
    Memcheck:Leak
@@ -24,7 +27,10 @@
    ...
    fun:main
 }
-# Backtrace might generate false positives with its _dl_open function call: https://stackoverflow.com/questions/78525668/egl-memory-leak
+# This suppression is to suppress backtrace() memory defects for Ubuntu22 and 24.
+# backtrace are loaded dynamically when they were first used, and dynamic loading will call malloc.
+# Those allocated memory are not subsequently freed in Ubuntu22 and 24's libC.
+# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
 {
    stacktrace_suppression
    Memcheck:Leak

From 654a9e06fb1a7838e306725d805700fd2458d7d5 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 18:36:52 +0000
Subject: [PATCH 09/26] ci: address PR comments

* reformat `valgrind.suppressions` comments to make them more concise
* move set Valgrind variable up. The next PR will change the location of
  set MEMORYCHECK_COMMAND_OPTIONS
---
 CMakeLists.txt                   |  3 ++-
 tests/unit/valgrind.suppressions | 20 +++++++++++---------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7f3204c799a..4d4e40201c9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -37,6 +37,8 @@ else()
     set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
 endif()
 
+set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+
 # These Version numbers are for major updates only- we won't track minor/patch updates here.
 set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
@@ -496,7 +498,6 @@ if (BUILD_TESTING)
     add_library(allocator_overrides SHARED ${TEST_LD_PRELOAD})
 
     set(UNIT_TEST_ENVS S2N_DONT_MLOCK=1)
-    set(CTEST_MEMORYCHECK_TYPE "Valgrind")
     if (TSAN OR ASAN)
         set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} S2N_ADDRESS_SANITIZER=1)
     endif()
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index e244964d8f8..60a04fa9253 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -1,4 +1,7 @@
-# It looks like valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
+# Valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
+# Without these suppressions, the following tests will fail:
+# s2n_examples_test, s2n_fork_generation_number_test, s2n_init_test, s2n_key_update_threads_test, and s2n_random_test.
+# These are the suppressions for pthread_create in GLIBC_2.2.5.
 {
    pthread_false_positive
    Memcheck:Leak
@@ -10,10 +13,10 @@
    fun:pthread_create@@GLIBC_2.2.5
    fun:main
 }
-# Append valgrind suppression for ubuntu22 and ubuntu24
-# If this suppression is not include, Test number 82, 95,113, 121, and 158 will fail
-# This block is the same suppression as the pthread suppressions above, but for a diffeent libC.
-# Adding tracking issue #4777. Remove the previous pthread suppressions block after new Ctest memcheck is fully integrated.
+# This block is a similar suppression to the pthread suppressions above, but for a different libc version.
+# These are the suppressions for pthread_create in GLIBC_2.34.
+# Remove the previous pthread suppression block after the new CTest memcheck is fully integrated.
+# Tracking issue: https://github.com/aws/s2n-tls/issues/4777
 {
    pthread_false_positive_ubuntu22
    Memcheck:Leak
@@ -27,10 +30,9 @@
    ...
    fun:main
 }
-# This suppression is to suppress backtrace() memory defects for Ubuntu22 and 24.
-# backtrace are loaded dynamically when they were first used, and dynamic loading will call malloc.
-# Those allocated memory are not subsequently freed in Ubuntu22 and 24's libC.
-# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
+# This suppression is to address false positives from backtrace().
+# backtrace is loaded dynamically when first used, and dynamic loading invokes malloc.
+# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html#:~:text=%E2%80%A2%20%20backtrace()%20and,is%20loaded%20beforehand.
 {
    stacktrace_suppression
    Memcheck:Leak

From 4f3b54343e0e13fbc978b724d1a3268f261a93cc Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 19:53:11 +0000
Subject: [PATCH 10/26] ci: address PR comments

* paste tracking issue's link to the comment
---
 CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4d4e40201c9..c6659338989 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -30,7 +30,7 @@ set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
 # Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
 # Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
 # valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
-# Tracking issue: #3758
+# Tracking issue: https://github.com/aws/s2n-tls/issues/3758
 if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
     set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
 else()

From b6c7fb68084ccbd473a6ca5b942fa6b3b341dc47 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 21:57:45 +0000
Subject: [PATCH 11/26] ci: address PR comments

* relocate `include(CTest)` location to above unit testing sections
---
 CMakeLists.txt | 56 +++++++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c6659338989..ec303cf346b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,31 +14,6 @@ set(INSTALL_CMAKE_DIR lib/cmake CACHE PATH "Installation directory for cmake fil
 
 set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE)
 
-set(VALGRIND_DEFAULT " \
-    --leak-check=full \
-    --leak-resolution=high \
-    --trace-children=yes \
-    --run-libc-freeres=no \
-    -q --error-exitcode=123 \
-    --error-limit=no \
-    --num-callers=40 \
-    --undef-value-errors=no \
-    --suppressions=valgrind.suppressions")
-
-set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
-
-# Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
-# Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
-# valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
-# Tracking issue: https://github.com/aws/s2n-tls/issues/3758
-if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
-    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
-else()
-    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
-endif()
-
-set(CTEST_MEMORYCHECK_TYPE "Valgrind")
-
 # These Version numbers are for major updates only- we won't track minor/patch updates here.
 set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
@@ -66,9 +41,6 @@ option(TSAN "Enable ThreadSanitizer to test thread safety" OFF)
 option(ASAN "Enable AddressSanitizer to test memory safety" OFF)
 option(SECCOMP "Link with seccomp and run seccomp tests" OFF)
 
-# Turn BUILD_TESTING=ON by default
-include(CTest)
-
 file(GLOB API_HEADERS "api/*.h")
 file(GLOB API_UNSTABLE_HEADERS "api/unstable/*.h")
 
@@ -467,6 +439,34 @@ target_link_libraries(${PROJECT_NAME} PUBLIC ${OS_LIBS} m)
 target_include_directories(${PROJECT_NAME} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)
 target_include_directories(${PROJECT_NAME} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/api> $<INSTALL_INTERFACE:include>)
 
+set(VALGRIND_DEFAULT " \
+    --leak-check=full \
+    --leak-resolution=high \
+    --trace-children=yes \
+    --run-libc-freeres=no \
+    -q --error-exitcode=123 \
+    --error-limit=no \
+    --num-callers=40 \
+    --undef-value-errors=no \
+    --suppressions=valgrind.suppressions")
+
+set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
+
+# Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
+# Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
+# valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
+# Tracking issue: https://github.com/aws/s2n-tls/issues/3758
+if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
+    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
+else()
+    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
+endif()
+
+set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+
+# Turn BUILD_TESTING=ON by default
+include(CTest)
+
 if (BUILD_TESTING)
     enable_testing()
 

From eea54eb81b0c4323d6e3afc110446529ec53ae4e Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 17 Sep 2024 23:21:40 +0000
Subject: [PATCH 12/26] ci: address PR comments

* explicitly enable `BUILD_TESTING` options in `CMakeLists.txt`
* relocate `MEMORYCHECK_COMMAND_OPTIONS` to the testing sections above
  ASAN
---
 CMakeLists.txt | 57 +++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ec303cf346b..a77b2351db6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,6 +19,7 @@ set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
 set(VERSION_PATCH 0)
 
+option(BUILD_TESTING "Enable BUILD_TESTING by default" ON)
 option(SEARCH_LIBCRYPTO "Set this if you want to let S2N search libcrypto for you,
 otherwise a crypto target needs to be defined." ON)
 option(UNSAFE_TREAT_WARNINGS_AS_ERRORS "Compiler warnings are treated as errors. Warnings may
@@ -439,34 +440,6 @@ target_link_libraries(${PROJECT_NAME} PUBLIC ${OS_LIBS} m)
 target_include_directories(${PROJECT_NAME} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)
 target_include_directories(${PROJECT_NAME} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/api> $<INSTALL_INTERFACE:include>)
 
-set(VALGRIND_DEFAULT " \
-    --leak-check=full \
-    --leak-resolution=high \
-    --trace-children=yes \
-    --run-libc-freeres=no \
-    -q --error-exitcode=123 \
-    --error-limit=no \
-    --num-callers=40 \
-    --undef-value-errors=no \
-    --suppressions=valgrind.suppressions")
-
-set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
-
-# Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
-# Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
-# valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
-# Tracking issue: https://github.com/aws/s2n-tls/issues/3758
-if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
-    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
-else()
-    set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
-endif()
-
-set(CTEST_MEMORYCHECK_TYPE "Valgrind")
-
-# Turn BUILD_TESTING=ON by default
-include(CTest)
-
 if (BUILD_TESTING)
     enable_testing()
 
@@ -497,6 +470,31 @@ if (BUILD_TESTING)
     file (GLOB TEST_LD_PRELOAD "tests/LD_PRELOAD/*.c")
     add_library(allocator_overrides SHARED ${TEST_LD_PRELOAD})
 
+    set(VALGRIND_DEFAULT " \
+    --leak-check=full \
+    --leak-resolution=high \
+    --trace-children=yes \
+    --run-libc-freeres=no \
+    -q --error-exitcode=123 \
+    --error-limit=no \
+    --num-callers=40 \
+    --undef-value-errors=no \
+    --suppressions=valgrind.suppressions")
+
+    set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
+
+    # Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
+    # Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
+    # valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
+    # Tracking issue: https://github.com/aws/s2n-tls/issues/3758
+    if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
+        set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
+    else()
+        set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
+    endif()
+
+    set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+
     set(UNIT_TEST_ENVS S2N_DONT_MLOCK=1)
     if (TSAN OR ASAN)
         set(UNIT_TEST_ENVS ${UNIT_TEST_ENVS} S2N_ADDRESS_SANITIZER=1)
@@ -702,6 +700,9 @@ if (BUILD_TESTING)
     endif()
 endif()
 
+# Turn BUILD_TESTING=ON by default
+include(CTest)
+
 #install the s2n files
 install(FILES ${API_HEADERS} DESTINATION "include/" COMPONENT Development)
 install(FILES ${API_UNSTABLE_HEADERS} DESTINATION "include/s2n/unstable" COMPONENT Development)

From 93b4f861157fb2c05e3dfcf94d8f4a46f26a9cdc Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Wed, 18 Sep 2024 15:41:50 +0000
Subject: [PATCH 13/26] ci: change `CMakeLists.txt` comments

* remove the comments above `include(CTest)`
* update the message to set `BUILD_TESTING=ON`
---
 CMakeLists.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a77b2351db6..e8b6e3c93f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,7 +19,7 @@ set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
 set(VERSION_PATCH 0)
 
-option(BUILD_TESTING "Enable BUILD_TESTING by default" ON)
+option(BUILD_TESTING "Turn BUILD_TESTING=on by default" ON)
 option(SEARCH_LIBCRYPTO "Set this if you want to let S2N search libcrypto for you,
 otherwise a crypto target needs to be defined." ON)
 option(UNSAFE_TREAT_WARNINGS_AS_ERRORS "Compiler warnings are treated as errors. Warnings may
@@ -700,7 +700,6 @@ if (BUILD_TESTING)
     endif()
 endif()
 
-# Turn BUILD_TESTING=ON by default
 include(CTest)
 
 #install the s2n files

From fb8ef6459364b414f727a2d278d0475160729529 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Wed, 18 Sep 2024 20:33:26 +0000
Subject: [PATCH 14/26] ci: address PR comments

* refactor some comments in `CMakeLists.txt`
* change suppressions names for valgrind and delete some comments
---
 CMakeLists.txt                   | 14 +++++++-------
 tests/unit/valgrind.suppressions | 14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e8b6e3c93f3..e068f43ddab 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,7 +19,6 @@ set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
 set(VERSION_PATCH 0)
 
-option(BUILD_TESTING "Turn BUILD_TESTING=on by default" ON)
 option(SEARCH_LIBCRYPTO "Set this if you want to let S2N search libcrypto for you,
 otherwise a crypto target needs to be defined." ON)
 option(UNSAFE_TREAT_WARNINGS_AS_ERRORS "Compiler warnings are treated as errors. Warnings may
@@ -34,6 +33,7 @@ option(S2N_LTO, "Enables link time optimizations when building s2n-tls." OFF)
 option(S2N_STACKTRACE "Enables stacktrace functionality in s2n-tls. Note that this functionality is
 only available on platforms that support execinfo." ON)
 option(COVERAGE "Enable profiling collection for code coverage calculation" OFF)
+option(BUILD_TESTING "Build tests for s2n-tls. By default only unit tests are built." ON)
 option(S2N_INTEG_TESTS "Enable the integrationv2 tests" OFF)
 option(S2N_FAST_INTEG_TESTS "Enable the integrationv2 with more parallelism, only has effect if S2N_INTEG_TESTS=ON" ON)
 option(S2N_INSTALL_S2NC_S2ND "Install the binaries s2nc and s2nd" OFF)
@@ -470,6 +470,7 @@ if (BUILD_TESTING)
     file (GLOB TEST_LD_PRELOAD "tests/LD_PRELOAD/*.c")
     add_library(allocator_overrides SHARED ${TEST_LD_PRELOAD})
 
+    # These configuration variables need to be set before include(CTest) is called
     set(VALGRIND_DEFAULT " \
     --leak-check=full \
     --leak-resolution=high \
@@ -483,10 +484,9 @@ if (BUILD_TESTING)
 
     set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
 
-    # Add pendentic Valgrind check for Libcrypto is openssl-1.1.1.
-    # Default Valgrind tests ignores "Still reachable" leak and we want to enable pedantic
-    # valgrind check incrementally, so we will do pedantic check for Libcrypto=openssl-1.1.1 for now.
-    # Tracking issue: https://github.com/aws/s2n-tls/issues/3758
+    # "pedantic valgrind" will error on memory that is "Still Reachable". 
+    # We only run this on OpenSSL 1.1.1 because there are hundreds of false positives in other libcryptos.
+    # Tracking issue: https://github.com/aws/s2n-tls/issues/4777
     if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
         set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
     else()
@@ -564,6 +564,8 @@ if (BUILD_TESTING)
     target_link_libraries(policy ${PROJECT_NAME})
     target_compile_options(policy PRIVATE -std=gnu99)
 
+    include(CTest)
+
     if(S2N_LTO)
         target_compile_options(s2nc PRIVATE -flto)
         target_compile_options(s2nd PRIVATE -flto)
@@ -700,8 +702,6 @@ if (BUILD_TESTING)
     endif()
 endif()
 
-include(CTest)
-
 #install the s2n files
 install(FILES ${API_HEADERS} DESTINATION "include/" COMPONENT Development)
 install(FILES ${API_UNSTABLE_HEADERS} DESTINATION "include/s2n/unstable" COMPONENT Development)
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 60a04fa9253..e2434b6180f 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -1,9 +1,8 @@
 # Valgrind may generate false positives on pthreads: https://stackoverflow.com/a/13132968
 # Without these suppressions, the following tests will fail:
 # s2n_examples_test, s2n_fork_generation_number_test, s2n_init_test, s2n_key_update_threads_test, and s2n_random_test.
-# These are the suppressions for pthread_create in GLIBC_2.2.5.
 {
-   pthread_false_positive
+   pthread_false_positive_glibc_2_2_5
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
@@ -14,11 +13,10 @@
    fun:main
 }
 # This block is a similar suppression to the pthread suppressions above, but for a different libc version.
-# These are the suppressions for pthread_create in GLIBC_2.34.
 # Remove the previous pthread suppression block after the new CTest memcheck is fully integrated.
 # Tracking issue: https://github.com/aws/s2n-tls/issues/4777
 {
-   pthread_false_positive_ubuntu22
+   pthread_false_positive_glibc_2_34
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
@@ -30,9 +28,11 @@
    ...
    fun:main
 }
-# This suppression is to address false positives from backtrace().
-# backtrace is loaded dynamically when first used, and dynamic loading invokes malloc.
-# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html#:~:text=%E2%80%A2%20%20backtrace()%20and,is%20loaded%20beforehand.
+# > backtrace() and backtrace_symbols_fd() don't call malloc()
+# > explicitly, but they are part of libgcc, which gets loaded
+# > dynamically when first used.  Dynamic loading usually triggers
+# > a call to [malloc(3)](https://man7.org/linux/man-pages/man3/malloc.3.html).
+# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html#NOTES
 {
    stacktrace_suppression
    Memcheck:Leak

From f8997479a833bce2e7e4cf4e250918ad2182d498 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Wed, 18 Sep 2024 20:41:55 +0000
Subject: [PATCH 15/26] ci: address PR comments

* relocate `include(CTest)` to the end of unit tests set up
---
 CMakeLists.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e068f43ddab..0018ce225c8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -564,13 +564,13 @@ if (BUILD_TESTING)
     target_link_libraries(policy ${PROJECT_NAME})
     target_compile_options(policy PRIVATE -std=gnu99)
 
-    include(CTest)
-
     if(S2N_LTO)
         target_compile_options(s2nc PRIVATE -flto)
         target_compile_options(s2nd PRIVATE -flto)
     endif()
 
+    include(CTest)
+
     if (S2N_INTEG_TESTS)
         find_package (Python3 COMPONENTS Interpreter Development)
         file(GLOB integv2_test_files "${PROJECT_SOURCE_DIR}/tests/integrationv2/test_*.py")

From f0f08c6db1b19d7d0985a569ab3a5a2f8943c98c Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Wed, 18 Sep 2024 22:12:17 +0000
Subject: [PATCH 16/26] ci: address PR comments

* refactor formats
---
 CMakeLists.txt                   | 6 +++---
 tests/unit/valgrind.suppressions | 5 ++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0018ce225c8..049092be06a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -470,7 +470,7 @@ if (BUILD_TESTING)
     file (GLOB TEST_LD_PRELOAD "tests/LD_PRELOAD/*.c")
     add_library(allocator_overrides SHARED ${TEST_LD_PRELOAD})
 
-    # These configuration variables need to be set before include(CTest) is called
+    # CTest configuration variables need to be set before include(CTest) is called
     set(VALGRIND_DEFAULT " \
     --leak-check=full \
     --leak-resolution=high \
@@ -523,6 +523,8 @@ if (BUILD_TESTING)
     endif()
     message(STATUS "Running tests with environment: ${UNIT_TEST_ENVS}")
 
+    include(CTest)
+
     file(GLOB UNITTESTS_SRC "tests/unit/*.c")
     foreach(test_case ${UNITTESTS_SRC})
         string(REGEX REPLACE ".+\\/(.+)\\.c" "\\1" test_case_name ${test_case})
@@ -569,8 +571,6 @@ if (BUILD_TESTING)
         target_compile_options(s2nd PRIVATE -flto)
     endif()
 
-    include(CTest)
-
     if (S2N_INTEG_TESTS)
         find_package (Python3 COMPONENTS Interpreter Development)
         file(GLOB integv2_test_files "${PROJECT_SOURCE_DIR}/tests/integrationv2/test_*.py")
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index e2434b6180f..8473b878a2d 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -12,9 +12,6 @@
    fun:pthread_create@@GLIBC_2.2.5
    fun:main
 }
-# This block is a similar suppression to the pthread suppressions above, but for a different libc version.
-# Remove the previous pthread suppression block after the new CTest memcheck is fully integrated.
-# Tracking issue: https://github.com/aws/s2n-tls/issues/4777
 {
    pthread_false_positive_glibc_2_34
    Memcheck:Leak
@@ -28,6 +25,7 @@
    ...
    fun:main
 }
+
 # > backtrace() and backtrace_symbols_fd() don't call malloc()
 # > explicitly, but they are part of libgcc, which gets loaded
 # > dynamically when first used.  Dynamic loading usually triggers
@@ -58,6 +56,7 @@
    ...
    fun:main
 }
+
 # TODO: fix the pedantic leak errors from s2n_fork_generation_number_test
 {
    ignore_s2n_fork_generation_number_test

From 3bab3bb679dc9c2bf07843997f1481a07bdf13f1 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Thu, 19 Sep 2024 17:00:19 +0000
Subject: [PATCH 17/26] ci: adjust indentations for valgrind default options

---
 CMakeLists.txt | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 049092be06a..d907256ac7c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -472,15 +472,15 @@ if (BUILD_TESTING)
 
     # CTest configuration variables need to be set before include(CTest) is called
     set(VALGRIND_DEFAULT " \
-    --leak-check=full \
-    --leak-resolution=high \
-    --trace-children=yes \
-    --run-libc-freeres=no \
-    -q --error-exitcode=123 \
-    --error-limit=no \
-    --num-callers=40 \
-    --undef-value-errors=no \
-    --suppressions=valgrind.suppressions")
+        --leak-check=full \
+        --leak-resolution=high \
+        --trace-children=yes \
+        --run-libc-freeres=no \
+        -q --error-exitcode=123 \
+        --error-limit=no \
+        --num-callers=40 \
+        --undef-value-errors=no \
+        --suppressions=valgrind.suppressions")
 
     set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
 

From b0f74a1cf85611ed3fa82ef5e687f666522b1bf2 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Thu, 19 Sep 2024 17:27:09 +0000
Subject: [PATCH 18/26] ci: adjust `valgrind.suppressions`

* add one wildcard to suppress backtrace memory defects for ubuntu22 and
  24 platforms
---
 tests/unit/valgrind.suppressions | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 8473b878a2d..ece38888df1 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -51,7 +51,7 @@
    fun:dlerror_run
    fun:__libc_dlopen_mode
    fun:__libc_unwind_link_get
-   fun:__libc_unwind_link_get
+   ...
    fun:backtrace
    ...
    fun:main

From 47147f1d56e82d72eb6780dbbbbe1cf5cf8c650d Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Thu, 19 Sep 2024 20:24:24 +0000
Subject: [PATCH 19/26] ci: modify links in comments

---
 tests/unit/valgrind.suppressions | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index ece38888df1..92ec748063b 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -30,7 +30,7 @@
 # > explicitly, but they are part of libgcc, which gets loaded
 # > dynamically when first used.  Dynamic loading usually triggers
 # > a call to [malloc(3)](https://man7.org/linux/man-pages/man3/malloc.3.html).
-# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html#NOTES
+# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
 {
    stacktrace_suppression
    Memcheck:Leak

From 991b2a9437ae71369a14647a5ea75d717bb2bb6f Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Thu, 19 Sep 2024 22:22:02 +0000
Subject: [PATCH 20/26] ci: add docker image

---
 codebuild/spec/buildspec_valgrind.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 0c81f5bf5e2..7cb5d971fd4 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -18,12 +18,14 @@ batch:
     - identifier: gcc_awslc
       env:
         compute-type: BUILD_GENERAL1_LARGE
+        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
         variables:
           S2N_LIBCRYPTO: awslc
           COMPILER: gcc
     - identifier: gcc_openssl_3_0
       env:
         compute-type: BUILD_GENERAL1_LARGE
+        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
         variables:
           S2N_LIBCRYPTO: openssl-3.0
           COMPILER: gcc

From fdeac7083360ec3efd9f0add171aa0476ca561dd Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Fri, 20 Sep 2024 18:58:20 +0000
Subject: [PATCH 21/26] ci: adding display error script to CI

---
 codebuild/bin/display_memory_errors.sh | 25 +++++++++++++++++++++++++
 codebuild/spec/buildspec_valgrind.yml  |  7 ++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100755 codebuild/bin/display_memory_errors.sh

diff --git a/codebuild/bin/display_memory_errors.sh b/codebuild/bin/display_memory_errors.sh
new file mode 100755
index 00000000000..0ac6a42c56c
--- /dev/null
+++ b/codebuild/bin/display_memory_errors.sh
@@ -0,0 +1,25 @@
+#!/bin/bash
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License").
+# You may not use this file except in compliance with the License.
+# A copy of the License is located at
+#
+#  http://aws.amazon.com/apache2.0
+#
+# or in the "license" file accompanying this file. This file is distributed
+# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+# express or implied. See the License for the specific language governing
+# permissions and limitations under the License.
+
+directory="../../build/Testing/Temporary"
+
+for file in "$directory"/*
+do
+    if [[ $(basename "$file") =~ ^MemoryChecker\..*\.log$ ]]; then
+        if [[ -f "$file" && -s "$file" ]]; then
+            echo $(realpath "$file")
+            cat "$file"
+        fi
+    fi
+done
diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 7cb5d971fd4..37e8041fb4f 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -55,4 +55,9 @@ phases:
         CTEST_PARALLEL_LEVEL=$(nproc) \
         CTEST_OUTPUT_ON_FAILURE=1 \
         ctest -T memcheck \
-              --test-dir build
+              --test-dir build;
+              EXITCODE=$?
+      - cd codebuild/bin/
+      - ./display_memory_errors.sh
+      - cd ../../
+      - exit $EXITCODE

From e74c1d5b0a2579082b34337536034e1838f27c3d Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 24 Sep 2024 18:02:43 +0000
Subject: [PATCH 22/26] ci: address PR comments

* adjust valgrind suppressions
* adjust comments format
---
 CMakeLists.txt                        |  4 +---
 codebuild/spec/buildspec_valgrind.yml | 14 ++++++++++++++
 tests/unit/valgrind.suppressions      | 25 +++++++------------------
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d907256ac7c..9ca18d47ac6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -482,13 +482,11 @@ if (BUILD_TESTING)
         --undef-value-errors=no \
         --suppressions=valgrind.suppressions")
 
-    set(VALGRIND_PENDANTIC "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
-
     # "pedantic valgrind" will error on memory that is "Still Reachable". 
     # We only run this on OpenSSL 1.1.1 because there are hundreds of false positives in other libcryptos.
     # Tracking issue: https://github.com/aws/s2n-tls/issues/4777
     if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
-        set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_PENDANTIC})
+        set(MEMORYCHECK_COMMAND_OPTIONS "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
     else()
         set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
     endif()
diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 37e8041fb4f..834c84c9da7 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -29,6 +29,20 @@ batch:
         variables:
           S2N_LIBCRYPTO: openssl-3.0
           COMPILER: gcc
+    - identifier: gcc_openssl_1_1_1
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
+        variables:
+          S2N_LIBCRYPTO: openssl-1.1.1
+          COMPILER: gcc
+    - identifier: gcc_openssl_1_0_2
+      env:
+        compute-type: BUILD_GENERAL1_LARGE
+        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
+        variables:
+          S2N_LIBCRYPTO: openssl-1.0.2
+          COMPILER: gcc
 
 phases:
   pre_build:
diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 92ec748063b..289cb673aef 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -2,35 +2,24 @@
 # Without these suppressions, the following tests will fail:
 # s2n_examples_test, s2n_fork_generation_number_test, s2n_init_test, s2n_key_update_threads_test, and s2n_random_test.
 {
-   pthread_false_positive_glibc_2_2_5
+   pthread_false_positive
    Memcheck:Leak
    match-leak-kinds: possible
    fun:calloc
+   ...
    fun:allocate_dtv
    fun:_dl_allocate_tls
    fun:allocate_stack
-   fun:pthread_create@@GLIBC_2.2.5
-   fun:main
-}
-{
-   pthread_false_positive_glibc_2_34
-   Memcheck:Leak
-   match-leak-kinds: possible
-   fun:calloc
-   fun:calloc
-   fun:allocate_dtv
-   fun:_dl_allocate_tls
-   fun:allocate_stack
-   fun:pthread_create@@GLIBC_2.34
+   fun:pthread_create@@*
    ...
    fun:main
 }
 
-# > backtrace() and backtrace_symbols_fd() don't call malloc()
-# > explicitly, but they are part of libgcc, which gets loaded
-# > dynamically when first used.  Dynamic loading usually triggers
-# > a call to [malloc(3)](https://man7.org/linux/man-pages/man3/malloc.3.html).
 # https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
+#  backtrace() and backtrace_symbols_fd() don't call malloc()
+#  explicitly, but they are part of libgcc, which gets loaded
+#  dynamically when first used.  Dynamic loading usually triggers
+#  a call to [malloc(3)](https://man7.org/linux/man-pages/man3/malloc.3.html).
 {
    stacktrace_suppression
    Memcheck:Leak

From e7ab1ff58bd13829773023951579bba154674d98 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Fri, 27 Sep 2024 23:12:59 +0000
Subject: [PATCH 23/26] ci: correct Valgrind and buildspec setting

---
 CMakeLists.txt                         |  8 ++++----
 codebuild/bin/display_memory_errors.sh | 25 -------------------------
 codebuild/spec/buildspec_valgrind.yml  | 11 +++--------
 3 files changed, 7 insertions(+), 37 deletions(-)
 delete mode 100755 codebuild/bin/display_memory_errors.sh

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 17cabebee62..886c6366043 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -500,23 +500,23 @@ if (BUILD_TESTING)
         --leak-check=full \
         --leak-resolution=high \
         --trace-children=yes \
-        --run-libc-freeres=no \
         -q --error-exitcode=123 \
         --error-limit=no \
         --num-callers=40 \
         --undef-value-errors=no \
+        --log-fd=2 \
         --suppressions=valgrind.suppressions")
 
     # "pedantic valgrind" will error on memory that is "Still Reachable". 
     # We only run this on OpenSSL 1.1.1 because there are hundreds of false positives in other libcryptos.
     # Tracking issue: https://github.com/aws/s2n-tls/issues/4777
     if ($ENV{S2N_LIBCRYPTO} MATCHES "openssl-1.1.1")
-        set(MEMORYCHECK_COMMAND_OPTIONS "${VALGRIND_DEFAULT} --errors-for-leak-kinds=all")
+        set(MEMORYCHECK_COMMAND_OPTIONS "${VALGRIND_DEFAULT} --run-libc-freeres=yes --errors-for-leak-kinds=all --show-leak-kinds=all")
     else()
-        set(MEMORYCHECK_COMMAND_OPTIONS ${VALGRIND_DEFAULT})
+        set(MEMORYCHECK_COMMAND_OPTIONS "${VALGRIND_DEFAULT} --run-libc-freeres=no")
     endif()
 
-    set(CTEST_MEMORYCHECK_TYPE "Valgrind")
+    set(MEMORYCHECK_TYPE "Valgrind")
 
     set(UNIT_TEST_ENVS S2N_DONT_MLOCK=1)
     if (TSAN OR ASAN)
diff --git a/codebuild/bin/display_memory_errors.sh b/codebuild/bin/display_memory_errors.sh
deleted file mode 100755
index 0ac6a42c56c..00000000000
--- a/codebuild/bin/display_memory_errors.sh
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/bin/bash
-# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
-#
-# Licensed under the Apache License, Version 2.0 (the "License").
-# You may not use this file except in compliance with the License.
-# A copy of the License is located at
-#
-#  http://aws.amazon.com/apache2.0
-#
-# or in the "license" file accompanying this file. This file is distributed
-# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
-# express or implied. See the License for the specific language governing
-# permissions and limitations under the License.
-
-directory="../../build/Testing/Temporary"
-
-for file in "$directory"/*
-do
-    if [[ $(basename "$file") =~ ^MemoryChecker\..*\.log$ ]]; then
-        if [[ -f "$file" && -s "$file" ]]; then
-            echo $(realpath "$file")
-            cat "$file"
-        fi
-    fi
-done
diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml
index 834c84c9da7..01c9b21bb9b 100644
--- a/codebuild/spec/buildspec_valgrind.yml
+++ b/codebuild/spec/buildspec_valgrind.yml
@@ -32,7 +32,7 @@ batch:
     - identifier: gcc_openssl_1_1_1
       env:
         compute-type: BUILD_GENERAL1_LARGE
-        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild
+        image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild
         variables:
           S2N_LIBCRYPTO: openssl-1.1.1
           COMPILER: gcc
@@ -68,10 +68,5 @@ phases:
         S2N_VALGRIND=1 \
         CTEST_PARALLEL_LEVEL=$(nproc) \
         CTEST_OUTPUT_ON_FAILURE=1 \
-        ctest -T memcheck \
-              --test-dir build;
-              EXITCODE=$?
-      - cd codebuild/bin/
-      - ./display_memory_errors.sh
-      - cd ../../
-      - exit $EXITCODE
+        cmake --build build/ --target test \
+          -- ARGS="--test-action memcheck"

From efe6477d977f23e956a20ab8642cd0e3717f2c6c Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 1 Oct 2024 22:17:35 +0000
Subject: [PATCH 24/26] ci: address PR comments

* modify valgrind suppression comments for backtrace.
---
 tests/unit/valgrind.suppressions | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index 289cb673aef..b738589f308 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -15,11 +15,9 @@
    fun:main
 }
 
-# https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
-#  backtrace() and backtrace_symbols_fd() don't call malloc()
-#  explicitly, but they are part of libgcc, which gets loaded
-#  dynamically when first used.  Dynamic loading usually triggers
-#  a call to [malloc(3)](https://man7.org/linux/man-pages/man3/malloc.3.html).
+# backtrace() leaks a static amount of memory when it is first called: https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
+# This leak is unscalable, so it can be safely suppressed.
+# Without this suppression, s2n_stacktrace_test will fail.
 {
    stacktrace_suppression
    Memcheck:Leak

From 6db76f47298579b495581226fbcebf7c6c8242d4 Mon Sep 17 00:00:00 2001
From: Boquan Fang <bqfang@amazon.com>
Date: Tue, 1 Oct 2024 22:37:22 +0000
Subject: [PATCH 25/26] ci: address PR comments

* make suppression comments more precise
---
 tests/unit/valgrind.suppressions | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index b738589f308..b48f4d7a79e 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -15,8 +15,8 @@
    fun:main
 }
 
-# backtrace() leaks a static amount of memory when it is first called: https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
-# This leak is unscalable, so it can be safely suppressed.
+# backtrace() allocates memory when it is first called: https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
+# The possible memory loss is unscalable, so it can be safely suppressed.
 # Without this suppression, s2n_stacktrace_test will fail.
 {
    stacktrace_suppression

From 0e4774b06104008d0b3b09805da0c7a6d51eea85 Mon Sep 17 00:00:00 2001
From: Boquan Fang <boquanfang3@gmail.com>
Date: Tue, 1 Oct 2024 16:20:56 -0700
Subject: [PATCH 26/26] Update tests/unit/valgrind.suppressions

Co-authored-by: Lindsay Stewart <stewart.r.lindsay@gmail.com>
---
 tests/unit/valgrind.suppressions | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/unit/valgrind.suppressions b/tests/unit/valgrind.suppressions
index b48f4d7a79e..e0f90fd05cb 100644
--- a/tests/unit/valgrind.suppressions
+++ b/tests/unit/valgrind.suppressions
@@ -15,9 +15,11 @@
    fun:main
 }
 
-# backtrace() allocates memory when it is first called: https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
-# The possible memory loss is unscalable, so it can be safely suppressed.
-# Without this suppression, s2n_stacktrace_test will fail.
+# This memory leak is believed to be caused by backtrace() loading libgcc dynamically.
+# See https://man7.org/linux/man-pages/man3/backtrace_symbols_fd.3.html
+# We were unable to find any relevant bug reports. However, testing showed that the memory
+# leak didn't scale with the number of calls to backtrace(), both supporting this theory and
+# limiting the potential impact of the leak.
 {
    stacktrace_suppression
    Memcheck:Leak