Skip to content

docs: record the environment an ASan build needs #12736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jvepsalainen-nv wants to merge 1 commit into
base: master
Choose a base branch
from
Open

docs: record the environment an ASan build needs #12736

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 43 additions & 0 deletions docs/building.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,49 @@ Downstream packaging may or may not choose to distribute them, at their
discretion. **We strongly encourage downstream users of Slang to move to the
new library names as soon as they are able.**

## Building with AddressSanitizer

Configuring with `-DSLANG_ENABLE_ASAN=ON` needs two environment settings that are easy to miss,
because without them the build fails partway through for reasons that do not mention the sanitizer.

```bash
# 1. The build compiles and then *runs* its own generators (slang-embed, slang-fiddle). Under
# SLANG_ENABLE_ASAN they are instrumented and link the ASan runtime dynamically, so the runtime
# must be locatable or they fail to start:
# slang-embed: error while loading shared libraries: libclang_rt.asan-x86_64.so:
# cannot open shared object file: No such file or directory
export LD_LIBRARY_PATH="$(clang -print-runtime-dir):$LD_LIBRARY_PATH"
Comment on lines +613 to +622

@coderabbitai coderabbitai Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  'SLANG_ENABLE_(ASAN|TSAN)|shared-libsan|CMAKE_(C|CXX)_COMPILER_ID' \
  cmake/CompilerFlags.cmake .github/workflows/ci-slang-sanitizer.yml docs/building.md

Repository: shader-slang/slang

Length of output: 26757

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- docs/building.md ---'
sed -n '611,652p' docs/building.md

printf '%s\n' '--- sanitizer option definitions and compiler branches ---'
rg -n -C 5 \
  'SLANG_ENABLE_(ASAN|TSAN)|shared-libsan|fsanitize=(address|thread)|/fsanitize=address|CMAKE_CXX_COMPILER_ID' \
  CMakeLists.txt cmake docs/building.md .github/workflows/ci-slang-sanitizer.yml

Repository: shader-slang/slang

Length of output: 24874

Limit the sanitizer instructions to Linux Clang builds.

The CMake implementation uses -shared-libsan only for non-Apple Clang builds. GNU and MSVC use different sanitizer runtime handling, while Apple Clang already uses a dynamic runtime. The TSan example also names the x86_64-specific libclang_rt.tsan-x86_64.so. Scope these instructions to Linux Clang builds, or document the platform- and architecture-specific equivalents.

@coderabbitai coderabbitai Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

unset LD_LIBRARY_PATH
unsafe="runtime:${LD_LIBRARY_PATH-}"
safe="runtime${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"

test "$unsafe" = "runtime:"
test "$safe" = "runtime"

Repository: shader-slang/slang

Length of output: 156

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- docs/building.md ---'
sed -n '600,655p' docs/building.md

printf '%s\n' '--- relevant loader-path references ---'
rg -n -C 3 'LD_LIBRARY_PATH|clang -print-runtime-dir' docs cmake

Repository: shader-slang/slang

Length of output: 6096

🌐 Web query:

Linux ld.so LD_LIBRARY_PATH empty path element current working directory man page

💡 Result:

In the Linux dynamic linker (ld.so), an empty path element within the LD_LIBRARY_PATH environment variable is interpreted as the current working directory [1][2][3]. According to the official ld.so(8) manual page, LD_LIBRARY_PATH is defined as a list of directories separated by colons or semicolons, and a zero-length directory name (e.g., resulting from a leading colon, trailing colon, or consecutive colons like::) explicitly indicates that the dynamic linker should search the current working directory [1][4][3]. This behavior is a legacy feature consistent with the treatment of the PATH environment variable in POSIX systems [5][6]. Because this behavior can cause the linker to load arbitrary libraries from the current directory—potentially leading to security vulnerabilities if an attacker introduces malicious shared objects into that directory—it is generally recommended to avoid empty elements in LD_LIBRARY_PATH [7]. This variable is also ignored entirely when the program is executed in secure-execution mode (such as for set-user-ID or set-group-ID binaries) to mitigate such risks [1][3].

Citations:

Avoid empty entries in LD_LIBRARY_PATH.

When LD_LIBRARY_PATH is unset, both commands create a trailing :. Linux treats the empty entry as the current directory, which can load unintended libraries. Use a conditional separator.


# 2. Those same generators leak, and LeakSanitizer's non-zero exit status fails the build:
# ERROR: LeakSanitizer: detected memory leaks
# SUMMARY: AddressSanitizer: 828842 byte(s) leaked in 3661 allocation(s)
# They are short-lived build tools, so leak detection is not meaningful for them.
export ASAN_OPTIONS=detect_leaks=0
Comment on lines +624 to +628

@coderabbitai coderabbitai Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Scope ASAN_OPTIONS to the build commands.

The exported variable remains active after cmake --build completes. If the user runs slang-test in the same shell, ASan tests inherit detect_leaks=0, so leak reports are suppressed. Apply the variable only to the configure and build commands, or instruct users to unset it before running tests.

Proposed documentation change 
-export ASAN_OPTIONS=detect_leaks=0
-
-cmake --preset default -DSLANG_ENABLE_ASAN=ON
-cmake --build --preset release
+ASAN_OPTIONS=detect_leaks=0 cmake --preset default -DSLANG_ENABLE_ASAN=ON
+ASAN_OPTIONS=detect_leaks=0 cmake --build --preset release


cmake --preset default -DSLANG_ENABLE_ASAN=ON
cmake --build --preset release
```

When _running_ the resulting binaries, put the build's own `lib/` directory on `LD_LIBRARY_PATH` as
well. `slang-test` loads its tools (`librender-test-tool.so` and friends) with `dlopen`, and if that
fails the affected tests are reported as **ignored** rather than failed — so a missing path silently
removes coverage instead of producing an error.

This is needed only for sanitizer builds. Normally that `dlopen` resolves through the executable's
`$ORIGIN/../lib` runpath without any help; under a sanitizer the call is intercepted by the
sanitizer runtime and issued from _its_ object, which has no runpath, so `$ORIGIN/../lib` is never
searched. Setting `LD_LIBRARY_PATH` for the runtime alone is therefore not enough — it must also
name the build's `lib/`:

```bash
export LD_LIBRARY_PATH="$PWD/build/Release/lib:$(clang -print-runtime-dir):$LD_LIBRARY_PATH"
```

@github-actions github-actions Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Gap: -DSLANG_ENABLE_TSAN=ON documents a build option that does not exist

This paragraph tells the reader that the same setup applies to a ThreadSanitizer build enabled via -DSLANG_ENABLE_TSAN=ON, but no such option exists in the build system. The only sanitizer option is SLANG_ENABLE_ASAN (CMakeLists.txt:162, "Enable AddressSanitizer (ASan), and UndefinedBehaviorSanitizer (UBSan) if available as well"), and cmake/CompilerFlags.cmake only implements -fsanitize=address/-fsanitize=undefined under that option — there is no -fsanitize=thread path. A grep of CMakeLists.txt, cmake/, and CMakePresets.json for TSAN/ThreadSanitizer returns zero build-configuration matches, and ci-slang-sanitizer.yml configures ASan only.

Example: A reader following this line runs cmake --preset default -DSLANG_ENABLE_TSAN=ON. CMake emits a "Manually-specified variables were not used by the project" warning and produces an ordinary, non-instrumented build — not a ThreadSanitizer build — so the described libclang_rt.tsan-x86_64.so setup never applies. Since the PR's purpose is to "record the environment an ASan build needs," documenting a toggle the build does not honor is actively misleading.

Suggested fix: Drop the TSAN paragraph (lines 647–648), or gate it as aspirational (e.g. "if a SLANG_ENABLE_TSAN option is added…"), or add the actual CMake option before documenting it.


The same applies to `-DSLANG_ENABLE_TSAN=ON`, with `libclang_rt.tsan-x86_64.so` in place of the ASan
runtime; `ASAN_OPTIONS=detect_leaks=0` is not needed there, as ThreadSanitizer has no leak checker.

The equivalent settings for CI are in `.github/workflows/ci-slang-sanitizer.yml`.

## Notes

[^1] below 3.25, CMake lacks the ability to mark directories as being
Expand Down
Loading