From 1f2f05d320cbdb8a8c6c74b05fe5c1d6dccb25e9 Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Wed, 3 Dec 2025 17:40:42 +0000
Subject: [PATCH] code review changes

---
 .github/scripts/strategy-matrix/generate.py      | 15 +++++++++++----
 .github/workflows/reusable-build-test-config.yml |  5 +++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py
index 2fb68fcaea..a460b385ab 100755
--- a/.github/scripts/strategy-matrix/generate.py
+++ b/.github/scripts/strategy-matrix/generate.py
@@ -240,7 +240,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
         # Add the configuration to the list, with the most unique fields first,
         # so that they are easier to identify in the GitHub Actions UI, as long
         # names get truncated.
-        # Add Address and Thread (both coupled with UB) sanitizers when the distro is bookworm.
+        # Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros.
         if os[
             "distro_version"
         ] == "bookworm" and f"{os['compiler_name']}-{os['compiler_version']}" in {
@@ -300,6 +300,8 @@ def addSanitizerConfigs(
     linker_relocation_flags = ""
     linker_flags = ""
 
+    cxx_flags += " -fno-omit-frame-pointer"
+
     # Use large code model to avoid relocation errors with large binaries
     # Only for x86-64 (amd64) - ARM64 doesn't support -mcmodel=large
     if architecture["platform"] == "linux/amd64" and os["compiler_name"] == "gcc":
@@ -312,6 +314,11 @@ def addSanitizerConfigs(
     # Create default sanitizer flags
     sanitizers_flags = "undefined,float-divide-by-zero"
 
+    # There are some differences between GCC and Clang support for sanitizers.
+    # Hence we must use diff. falg combinations for each compiler.
+    # These combination of flags were tested to work with GCC 15 and Clang 20.
+    # If the versions are changed, the flags might need to be updated.
+
     if os["compiler_name"] == "gcc":
         # Suppress false positive warnings in GCC with stringop-overflow
         extra_warning_flags += " -Wno-stringop-overflow"
@@ -326,7 +333,7 @@ def addSanitizerConfigs(
         # before CMake processes it. This ensures the compiler receives an absolute path.
         # CMAKE_SOURCE_DIR won't work here because it's inside CMAKE_CXX_FLAGS string.
         # GCC doesn't support ignorelist.
-        cxx_flags += " -fsanitize-ignorelist=$GITHUB_WORKSPACE/sanitizers/suppressions/sanitizer-ignorelist.txt"
+        cxx_flags += " -fsanitize-ignorelist=${GITHUB_WORKSPACE}/sanitizers/suppressions/sanitizer-ignorelist.txt"
         sanitizers_flags = f"{sanitizers_flags},unsigned-integer-overflow"
         linker_flags += (
             f" -DCMAKE_EXE_LINKER_FLAGS='-fsanitize=address,{sanitizers_flags}'"
@@ -339,7 +346,7 @@ def addSanitizerConfigs(
     cxx_flags += " -O1"
 
     # First create config for asan
-    cmake_args_flags = f'{cmake_args} -DCMAKE_CXX_FLAGS="-fsanitize=address,{sanitizers_flags} -fno-omit-frame-pointer {cxx_flags} {extra_warning_flags}" {linker_flags}'
+    cmake_args_flags = f'{cmake_args} -DCMAKE_CXX_FLAGS="-fsanitize=address,{sanitizers_flags} {cxx_flags} {extra_warning_flags}" {linker_flags}'
 
     # Add config with asan+ubsan
     configs = {}
@@ -371,7 +378,7 @@ def addSanitizerConfigs(
                 f" -DCMAKE_SHARED_LINKER_FLAGS='-fsanitize=thread,{sanitizers_flags}'"
             )
 
-        cmake_args_flags = f"{cmake_args} -DCMAKE_CXX_FLAGS='-fsanitize=thread,{sanitizers_flags} -fno-omit-frame-pointer {cxx_flags} {extra_warning_flags}' {linker_flags}"
+        cmake_args_flags = f"{cmake_args} -DCMAKE_CXX_FLAGS='-fsanitize=thread,{sanitizers_flags} {cxx_flags} {extra_warning_flags}' {linker_flags}"
 
         # Add config with tsan+ubsan
         configs["tsan_ubsan"] = cmake_args_flags
diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml
index 64d332f4b6..e938a7f608 100644
--- a/.github/workflows/reusable-build-test-config.yml
+++ b/.github/workflows/reusable-build-test-config.yml
@@ -116,11 +116,12 @@ jobs:
         env:
           BUILD_TYPE: ${{ inputs.build_type }}
         run: |
-          cmake .. \
+          cmake \
             -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \
             -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \
             -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \
-            ${{ inputs.cmake_args }}
+            ${{ inputs.cmake_args }} \
+            ..
 
       - name: Build the binary
         working-directory: ${{ inputs.build_dir }}