Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions simpler_setup/toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,35 @@ def _parse_compiler_env(var_name: str, default: str) -> tuple[str, list[str]]:
return tokens[0], tokens[1:]


def _host_compiler_cmake_args(default_cc: str, default_cxx: str) -> list[str]:
def _host_compiler_cmake_args(default_cc: str, default_cxx: str, pin_compiler: bool = False) -> list[str]:
"""CMake ``-D`` args for a host GCC/G++ toolchain.

Reads CC/CXX from the environment and splits off any conda-injected flags
(e.g. ``-pthread -B <env>/compiler_compat``) into CMAKE_{C,CXX}_FLAGS. The
flags are re-joined with ``shlex.join`` so tokens containing spaces survive
CMake's shell-style re-parse of the flag string.

When ``pin_compiler`` is set, an env CC/CXX naming a *different* GCC is
overridden by the caller's ``default_cc``/``default_cxx`` — but any
env-injected flags are still preserved, and an env compiler that already
names the pinned version (e.g. a custom path ``/opt/tc/bin/g++-15``) is kept
so non-PATH installs still work. This is required under a sanitizer: the
host compiler is ABI-pinned to g++-15 so its sanitizer runtime
(libtsan.so.2 / libasan.so) matches the lib*san the run-step preloads.
scikit-build-core exports CXX during ``pip install``; letting it name a
different GCC (whose libtsan SONAME is .so.0) produces a runtime mismatch
that fails at dlopen with "cannot allocate memory in static TLS block".
"""
cc, cc_flags = _parse_compiler_env("CC", default_cc)
cxx, cxx_flags = _parse_compiler_env("CXX", default_cxx)
if pin_compiler:
# Keep a custom path that already names the pinned version (basename
# contains e.g. "g++-15"); only override when the env names a different
# GCC — forcing the bare name would break a non-PATH custom install.
if os.path.basename(default_cc) not in os.path.basename(cc):
cc = default_cc
if os.path.basename(default_cxx) not in os.path.basename(cxx):
cxx = default_cxx
Comment on lines +81 to +84
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin check is too permissive and can keep the wrong compiler.

Line 81 and Line 83 use substring matching, so values like clang++-15 are treated as already pinned ("g++-15" in "clang++-15"), which can bypass the intended GCC ABI pinning.

Suggested hardening
+def _matches_pinned_name(actual: str, pinned: str) -> bool:
+    base = os.path.basename(actual)
+    return base == pinned or base.endswith(f"-{pinned}")
+
 def _host_compiler_cmake_args(default_cc: str, default_cxx: str, pin_compiler: bool = False) -> list[str]:
@@
     if pin_compiler:
@@
-        if os.path.basename(default_cc) not in os.path.basename(cc):
+        if not _matches_pinned_name(cc, os.path.basename(default_cc)):
             cc = default_cc
-        if os.path.basename(default_cxx) not in os.path.basename(cxx):
+        if not _matches_pinned_name(cxx, os.path.basename(default_cxx)):
             cxx = default_cxx
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@simpler_setup/toolchain.py` around lines 81 - 84, The current pin checks in
the if statements comparing os.path.basename(default_cc) and
os.path.basename(cc) (and similarly for default_cxx/cxx) use substring
membership which is too permissive; change both checks to require exact basename
equality (use != or ==) so that only an exact match like "g++-15" is considered
pinned and other compilers like "clang++-15" won't incorrectly pass, updating
the two conditions that set cc = default_cc and cxx = default_cxx accordingly.

args = [
f"-DCMAKE_C_COMPILER={cc}",
f"-DCMAKE_CXX_COMPILER={cxx}",
Expand Down Expand Up @@ -227,7 +246,14 @@ def get_compile_flags(self, **kwargs) -> list[str]:
return flags

def get_cmake_args(self) -> list[str]:
args = _host_compiler_cmake_args("gcc-15" if self._prefer_g15 else "gcc", self.cxx_path)
# Under prefer_g15 (sanitizer build) the compiler is ABI-pinned: env
# CC/CXX must not redirect it away from g++-15, or the built .so link a
# different lib*san than the run-step preloads. See pin_compiler above.
args = _host_compiler_cmake_args(
"gcc-15" if self._prefer_g15 else "gcc",
self.cxx_path,
pin_compiler=self._prefer_g15,
)
if self.ascend_home_path:
args.append(f"-DASCEND_HOME_PATH={self.ascend_home_path}")
return args
Expand Down
45 changes: 45 additions & 0 deletions tests/ut/py/test_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,51 @@ def test_conda_env_splits_flags(self, toolchain, monkeypatch):
assert "-DCMAKE_CXX_FLAGS=-pthread -B /data/envs/lyf/compiler_compat" in args


class TestGxxToolchainPreferG15Pins:
"""Under a sanitizer, prefer_g15 ABI-pins the host compiler to g++-15 so its
sanitizer runtime (libtsan.so.2 / libasan.so) matches the lib*san the
pytest/CI run-step preloads. An env CC/CXX naming a different GCC (e.g. the
system g++ whose libtsan SONAME is .so.0) must NOT override that pin —
scikit-build-core exports CXX during `pip install`, and the mismatch fails
at dlopen with "cannot allocate memory in static TLS block"."""

@pytest.fixture
def toolchain(self, monkeypatch):
monkeypatch.setattr("simpler_setup.toolchain._is_gcc", lambda _p: True)
from simpler_setup.toolchain import GxxToolchain # noqa: PLC0415

return GxxToolchain(prefer_g15=True)

def test_env_cxx_does_not_override_pin(self, toolchain, monkeypatch):
# scikit-build-core / a plain dev shell exporting the system g++.
monkeypatch.setenv("CC", "gcc")
monkeypatch.setenv("CXX", "g++")
args = toolchain.get_cmake_args()
assert "-DCMAKE_C_COMPILER=gcc-15" in args
assert "-DCMAKE_CXX_COMPILER=g++-15" in args

def test_conda_flags_preserved_but_compiler_pinned(self, toolchain, monkeypatch):
# Conda's injected -B compiler_compat flags must survive, but the
# compiler binary is still forced to the pinned g++-15.
monkeypatch.setenv("CC", "gcc -pthread -B /data/envs/lyf/compiler_compat")
monkeypatch.setenv("CXX", "g++ -pthread -B /data/envs/lyf/compiler_compat")
args = toolchain.get_cmake_args()
assert "-DCMAKE_C_COMPILER=gcc-15" in args
assert "-DCMAKE_CXX_COMPILER=g++-15" in args
assert "-DCMAKE_C_FLAGS=-pthread -B /data/envs/lyf/compiler_compat" in args
assert "-DCMAKE_CXX_FLAGS=-pthread -B /data/envs/lyf/compiler_compat" in args
Comment thread
ChaoWao marked this conversation as resolved.

def test_custom_path_to_g15_is_preserved(self, toolchain, monkeypatch):
# An env CC/CXX that already names g++-15 at a non-PATH location is the
# right ABI already — keep it instead of forcing the bare name, which
# might not be on PATH.
monkeypatch.setenv("CC", "/opt/custom/bin/gcc-15")
monkeypatch.setenv("CXX", "/opt/custom/bin/g++-15")
args = toolchain.get_cmake_args()
assert "-DCMAKE_C_COMPILER=/opt/custom/bin/gcc-15" in args
assert "-DCMAKE_CXX_COMPILER=/opt/custom/bin/g++-15" in args


class TestGxx15ToolchainCmakeArgs:
"""Same split behavior for the simulation-kernel toolchain."""

Expand Down
Loading