Skip to content

Parser: skip primary function templates in Sema instantiation calls#33

Open
Fedr wants to merge 8 commits into
masterfrom
fix/skip-primary-templates-in-sema-calls
Open

Parser: skip primary function templates in Sema instantiation calls#33
Fedr wants to merge 8 commits into
masterfrom
fix/skip-primary-templates-in-sema-calls

Conversation

@Fedr
Copy link
Copy Markdown

@Fedr Fedr commented May 12, 2026

Summary

The visitor unconditionally called Sema::InstantiateDefaultArgument and Sema::InstantiateFunctionDefinition on every FunctionDecl it traversed, including primary function templates that pass through because of allow_uninstantiated_templates. On a primary template both APIs null-deref inside libclang and crash with STATUS_ACCESS_VIOLATION. Skipping the call when decl.isTemplated() is true fixes both crashes — the specific instantiations are visited separately and processed correctly.

Stack traces (clang 22.1.4, MSYS2 mingw-w64-clang-x86_64)

InstantiateDefaultArgument at main.cpp:2838:

#0  clang::FunctionDecl::getNumParams() const                      (this = invalid)
#1  clang::Sema::addInstantiatedParametersToScope
#2  clang::Sema::SubstDefaultArgument                              (ForCallExpr = true)
#3  clang::Sema::InstantiateDefaultArgument
#4  mrbind::ClangAstVisitor_InstTypesAndCollectNewTypes::VisitFunctionDecl  (main.cpp:2838)

InstantiateFunctionDefinition at main.cpp:1072:

#0  clang::FunctionDecl::isDefined(FunctionDecl const*&, bool)     (this = invalid)
#1  clang::Sema::InstantiateFunctionDefinition
#2  mrbind::InstantiateReturnTypeIfNeeded                          (main.cpp:1072)
#3  mrbind::ClangAstVisitor_InstTypesAndCollectNewTypes::VisitFunctionDecl  (main.cpp:2859)

Both inputs the same: PatternFD returned by FunctionDecl::getTemplateInstantiationPattern(/*ForDefinition*/ false) on a primary template is null, and Sema dereferences it without a guard.

Tested clang versions

clang repro without fix with fix
18.1.8 crashes clean
19.1.7 crashes clean
21.1.8 crashes clean
22.1.4 crashes clean

Source-diff of Sema::SubstDefaultArgument, Sema::addInstantiatedParametersToScope, Sema::InstantiateDefaultArgument, FunctionDecl::getTemplateInstantiationPattern across llvmorg-18.1.8 ... llvmorg-22.1.4 is logically identical (only type/RAII renames), so the bug is at least as old as v18.

Standalone clang-tooling reproducer (no mrbind dependency):

// repro_input.cpp
template <typename T> struct Wrap {
    template <typename U = T> void method(int x = sizeof(U)) {}
};
inline void use() { Wrap<int> w; w.method(); }

A RecursiveASTVisitor whose VisitFunctionDecl calls Sema::InstantiateDefaultArgument on every ParmVarDecl with hasUninstantiatedDefaultArg() (no isTemplated() guard) crashes identically inside libclang. Will be filed upstream against LLVM as API hardening (Sema::InstantiateDefaultArgument's documented precondition is only assert(Param->hasUninstantiatedDefaultArg()) — it should also assert FD->isTemplateInstantiation() or fail gracefully on a null pattern).

Verification on real input

Local mrbind build with this fix, parsing #include <boost/multiprecision/cpp_int.hpp> on clang 22.1.4:

  • Without fix: silent crash after 0 lines of output (the original CI signature).
  • With fix: parser progresses past Sema instantiation, reaches a separate cppdecl error on _Complex _Float16 — that's a different issue in a different library, not addressed here.

Test plan

  • CI on this branch is green (mrbind has its own tests in test/).
  • Picked up as a submodule bump in MeshLib PR #6021 (clang 22 toolchain) and observed to clear the STATUS_ACCESS_VIOLATION segfault during Generate C bindings.

… / InstantiateFunctionDefinition calls

When the visitor walks a primary `FunctionDecl` for which `decl.isTemplated()`
is true, calling `Sema::InstantiateDefaultArgument` and
`Sema::InstantiateFunctionDefinition` on it is meaningless — there is no
specialization to substitute into — and on every clang version I tested
(18.1.8, 19.1.7, 21.1.8, 22.1.4) both APIs crash with `STATUS_ACCESS_VIOLATION`:

* `Sema::InstantiateDefaultArgument` -> `SubstDefaultArgument` ->
  `addInstantiatedParametersToScope`: derefs a `nullptr` returned by
  `FunctionDecl::getTemplateInstantiationPattern(/*ForDefinition*/ false)`.

* `Sema::InstantiateFunctionDefinition` -> `FunctionDecl::isDefined()`:
  derefs an invalid pattern decl pointer.

The visitor is allowed to see primary templates via `allow_uninstantiated_templates`,
and the specific instantiations are visited separately and handled correctly,
so just skipping the call on primary templates is the right fix.

Reduces to a standalone clang-tooling reproducer:

  // repro_input.cpp
  template <typename T> struct Wrap {
      template <typename U = T> void method(int x = sizeof(U)) {}
  };
  inline void use() { Wrap<int> w; w.method(); }

  // tool: a RecursiveASTVisitor that calls
  // Sema::InstantiateDefaultArgument on every ParmVarDecl with
  // `hasUninstantiatedDefaultArg()` (no `isTemplated()` guard) crashes
  // identically inside libclang. Bug exists at least back to clang 18.1.8.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fedr added a commit to MeshInspector/MeshLib that referenced this pull request May 12, 2026
Picks up the fix that skips primary function templates in
`Sema::InstantiateDefaultArgument` / `Sema::InstantiateFunctionDefinition`
calls. Without the fix, mrbind parsing `<boost/multiprecision/cpp_int.hpp>`
on clang 22 segfaults inside libclang with `STATUS_ACCESS_VIOLATION` on
a null `PatternDecl`. See MeshInspector/mrbind#33 for the full repro and
stack traces.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Install `llvm::sys::PrintStackTraceOnErrorSignal` at the top of `main()`
so a `SIGSEGV` / Windows SEH `STATUS_ACCESS_VIOLATION` from inside libclang
prints a backtrace to stderr instead of silently dying. Diagnostic-only —
no behavior change on the happy path.

With `RelWithDebInfo` builds (already the default per
`install_mrbind_windows_msys2.bat`), mrbind frames carry DWARF source
locations. libclang-cpp / libLLVM frames show function names only,
because msys2 strips DLL debug info.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adalisk-emikhaylov and others added 2 commits May 13, 2026 05:25
…rgs path

The block's own comment already says this path is only needed for free
function templates and that "something else already instantiates them for
the class member functions". When the path is exercised on a CXXMethodDecl
(in particular a CXXConstructorDecl), `Sema::SubstDecl` on the member
template's pattern null-derefs inside
`TemplateDeclInstantiator::InitMethodInstantiation`.

Captured stack from CI (clang 22.1.4) parsing
`<boost/multiprecision/cpp_int.hpp>` with
`--buggy-substitute-default-template-args`:

  #0 clang::TemplateDeclInstantiator::InitMethodInstantiation
  #1 clang::TemplateDeclInstantiator::VisitCXXMethodDecl
  #2 lambda inside Sema::SubstDecl (runWithSufficientStackSpace)
  #3 clang::StackExhaustionHandler::runWithSufficientStackSpace
  #4 clang::Sema::runWithSufficientStackSpace
  #5 clang::Sema::SubstDecl
  #6 mrbind::ClangAstVisitor_InstTypesAndCollectNewTypes::VisitFunctionDecl
       main.cpp:2815  <- this call site
  #7 clang::RecursiveASTVisitor::TraverseCXXConstructorDecl

Adding `!llvm::isa<clang::CXXMethodDecl>(decl)` to the existing
`decl->isTemplated()` check matches the comment and avoids the crash.
This is consistent with the previous commit, which skipped primary
function templates from `InstantiateDefaultArgument` /
`InstantiateFunctionDefinition` for the same class of latent libclang
bug (public Sema API null-derefs on a primary template's pattern).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adalisk-emikhaylov adalisk-emikhaylov force-pushed the fix/skip-primary-templates-in-sema-calls branch from 8f5d884 to c75830f Compare May 13, 2026 14:53
adalisk-emikhaylov added a commit to MeshInspector/MeshLib that referenced this pull request May 20, 2026
* ci(windows): use preinstalled MSYS2 + pacman-installed clang

Stop fetching the ~725 MB msys64_meshlib_mrbind.zip from S3 (which
has been seen returning 404 in CI) and stop relying on the
install-msys2-mrbind composite action. The windows-2025 runner
image already ships:

  * MSYS2 base at C:\msys64 (pacman 6.1.0)
  * Standalone LLVM 20.1.8 at C:\Program Files\LLVM (unused by
    MRBind, kept here for reference)

For MRBind we still need the MSYS2 -clang64 environment (mrbind
links against MSYS2's libclang/libc++, not the Windows-native
LLVM build). So this change pacman-installs the minimal toolchain
into the preinstalled C:\msys64:

  pacman -Sy --noconfirm --needed make \
    mingw-w64-clang-x86_64-{clang,clang-tools-extra,cmake,ninja,libc++}

and points all subsequent MRBind/binding-generation steps at
C:\msys64 by setting MSYS2_DIR (read by install_mrbind_windows_msys2.bat
and generate_win.bat). GETTEXT_ROOT moves from
C:\msys64_meshlib_mrbind\clang64 to C:\msys64\clang64.

The install-msys2-mrbind composite action is no longer used and is
deleted. install_deps_windows_msys2.bat (local-developer install
path) is unchanged: it still creates a separate
C:\msys64_meshlib_mrbind tree, untouched by this CI change.

Note: this swaps an S3-mirror dependency for an MSYS2-mirror
dependency. It also moves clang from a pinned 18.1.8 to whatever
mingw-w64-clang-x86_64-clang is currently in the clang64 repo
(22.x at the time of writing). MRBind's Windows path uses bare
`clang++` (no version suffix) so this works without further
changes; if a specific clang version becomes required later, pin
via pacman or use the MSYS2 archive.

* ci(windows): also install mingw-w64-clang-x86_64-llvm package

mrbind's CMakeLists does find_package(Clang REQUIRED), which
transitively requires LLVMConfig.cmake from the llvm dev package.
That package is listed only as an *optional* dep of clang, so
pacman did not pull it in automatically — Build MRBind failed
with:

  Could not find a package configuration file provided by "LLVM"
  (requested version 22.1.4)

Add mingw-w64-clang-x86_64-llvm to the install list explicitly.

* Update mrbind.

* Generate fresh MSYS2 lockfiles.

* We no longer upload zipped MSYS2 to S3.

* ci(windows): make generate_win.bat fail loudly on missing MSYS2

Two bugs combined to make a real failure look green in
https://github.com/MeshInspector/MeshLib/actions/runs/25392220595/job/74469566838:

1. The 'Generate and build Python bindings' step in
   build-test-windows.yml didn't carry the MSYS2_DIR=C:\msys64 env
   override that the other generate_win.bat / install_mrbind_windows_msys2.bat
   callers on this branch already have. So the script saw the
   default MSYS2_DIR=C:\msys64_meshlib_mrbind, which doesn't exist
   on the runner anymore.

2. generate_win.bat printed 'MSYS2 was NOT found' and then fell
   through to a normal exit, returning code 0. The step's `call`
   inherited that 0, GitHub Actions marked the step green, the
   .pyd never got built, and the failure only surfaced four steps
   later when 'Unit Tests' couldn't load mrmeshpy.pyd
   (LoadLibrary error 126).

Fix both:

* Add `MSYS2_DIR: C:\msys64` to the 'Generate and build Python
  bindings' step's env block, matching Build MRBind / Generate C
  bindings / Generate C# bindings.

* Replace generate_win.bat's silent fall-through with `exit /b 1`
  after the missing-MSYS2 message. Mirrors what
  install_mrbind_windows_msys2.bat already does on the same
  condition. Future workflow callers that forget MSYS2_DIR will
  fail at the right step instead of being papered over until
  something later trips on the missing artifacts.

* Try enabling the debug env.

* Hopefully fix ambiguous Python names.

* ci(windows): diagnostic step for embedded-python ImportError

Add a post-Unit-Tests step (always(), continue-on-error) that
captures state when MRTest.exe's embedded python smoke test
trips `ImportError: initialization failed` from `<string>(2):
<module>`. CPython's wrapper hides the real cause; this dumps:

* contents of source\x64\<config>\ (DLLs, .pyd, EXE) and
  source\x64\<config>\meshlib\
* dumpbin /exports of MR{Test,EmbeddedPython,Python}.{exe,dll}
  and each *.pyd, filtered to PyInit_* — confirms the module
  init function is exported
* dumpbin /dependents of the same, filtered to python*/MR*/CRT —
  shows which python.dll the bindings and the embedded
  interpreter resolve (catches ABI mismatches between vcpkg's
  Python 3.12 and any other python.dll on PATH)
* `py -0p` inventory of available pythons
* a standalone `py -3.12 -c "import meshlib.mrmeshpy"` with
  full traceback — gives the real Python exception instead of
  CPython's wrapped "initialization failed"
* python*.dll listings under C:\vcpkg\installed\...\bin and
  `where.exe` for python3.dll / python312.dll

Step is purely informational; it never fails the job.

* ci(windows): drop post-Unit-Tests Python diagnostic step

The pre-Unit-Tests "Verify meshlib.mrmeshpy import" step (added in
#6069) now surfaces the import failure with a real traceback before
MRTest's embedded-python smoke test runs, so the exhaustive
post-Unit-Tests dumpbin/PyInit walk is no longer needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): install latest clang from MSYS2 instead of pinning clang 18

Switch the install-msys2-mrbind composite from `pacman -U` against a
sha256-pinned lockfile (master's approach via #6060, ensuring clang
18.1.8) to a plain `pacman -S --needed` against the live MSYS2
mirror — picks up whatever clang/libc++/cmake/etc. is current.

  * Minimal explicit package set: clang, lld, libc++, cmake, ninja,
    gettext-tools, plus msys2 `make`. Pacman resolves transitive
    deps; nothing is downgraded so `--needed` skips anything the
    runner image already ships.
  * No `actions/cache@v5` step at all. The clang-18 cache populated
    by master's composite (key `msys2-mrbind-clang18-<hash>`) is on
    a different path + key and cannot be hit.
  * scripts/mrbind/msys2_package_hashes_clang18.txt deleted — the
    lockfile is no longer consulted on the CI install path.
    Developer-bootstrap path (install_deps_windows_msys2.bat) still
    uses the unsuffixed msys2_package_hashes.txt and is unaffected.

Trade-off vs master: faster (one `pacman -S`, ~30-60 s, no archive
extraction step), but exposed to mrbind regressions on newer clang
that motivated the original clang-18 pin. Whether mrbind tolerates
current clang now is what this PR validates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): add mingw-w64-clang-x86_64-llvm to the install list

mrbind's CMake build needs LLVMConfig.cmake (via Clang's ClangConfig
which does find_package(LLVM 22.1.4)). The earlier pacman -S call
pulled in llvm-libs as a clang dep (runtime libs) but NOT llvm
(which carries lib/cmake/llvm/LLVMConfig.cmake), so cmake fails:

  CMake Error at C:/msys64/clang64/lib/cmake/clang/ClangConfig.cmake:11:
    Could not find a package configuration file provided by "LLVM"
    (requested version 22.1.4) with any of the following names:
      LLVMConfig.cmake / llvm-config.cmake / LLVM.cps / llvm.cps

MSYS2 split the monolithic llvm package into llvm + llvm-libs + llvm-tools
since this CI's last run on this branch — used to be one package.
Add the explicit `mingw-w64-clang-x86_64-llvm` to pacman -S so the
cmake configs are present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): re-add cache step (key distinct from master's clang-18 cache)

Caching `C:\msys64\var\cache\pacman\pkg\` lets `pacman -S --needed`
reuse archives across runs — exact-match runs become near-instant,
partial matches (after action.yml package-list edits or mirror
updates) save the unchanged-package downloads.

Key:        `msys2-mrbind-latest-<os>-<hashFiles(action.yml)>`
Restore-key: `msys2-mrbind-latest-<os>-`

Different prefix from master's `msys2-mrbind-clang18-...` so the
two caches cannot collide, and on a different path
(`C:\msys64\var\cache\pacman\pkg` here vs
`scripts/mrbind/msys2_packages` on master).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): add clang-tools-extra to the install list

ClangTargets.cmake (shipped by mingw-w64-clang-x86_64-clang) imports
the `clangApplyReplacements` target backed by
`lib/libclangApplyReplacements.a`, which lives in the separate
`mingw-w64-clang-x86_64-clang-tools-extra` package
(clang-apply-replacements, clang-format, clang-tidy, etc.).
Without it cmake fails configuring mrbind:

  CMake Error at .../ClangTargets.cmake:808:
    The imported target "clangApplyReplacements" references the file
       ".../lib/libclangApplyReplacements.a"
    but this file does not exist.

Master's clang-18 lockfile pinned clang-tools-extra explicitly;
add it here too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): pin clang 22 via lockfile, mirroring master's clang-18 mechanism

Replace the previous `pacman -S --needed` against live mirrors with
the same lockfile-driven `pacman -U` approach master uses for clang
18, but pinning to the currently-shipped clang 22 versions.

  * New `scripts/mrbind/msys2_package_hashes_clang22.txt` — 55
    packages, all clang-22.1.4-3 era + transitive deps.
  * Composite action restored to two steps: cache + install. Cache
    keyed on `msys2-mrbind-clang22-<lockfile-hash>`, path
    `scripts/mrbind/msys2_packages` (same shape as master, just a
    different lockfile / cache key).
  * Reuses the existing SUFFIX-aware `msys2_download_packages.sh` /
    `msys2_install_packages.sh` scripts on this branch (they accept
    `_clang22` as the lockfile suffix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Try to fix mrbind errors.

* restore comment

* ci(windows): bump mrbind to MeshInspector/mrbind#33

Picks up the fix that skips primary function templates in
`Sema::InstantiateDefaultArgument` / `Sema::InstantiateFunctionDefinition`
calls. Without the fix, mrbind parsing `<boost/multiprecision/cpp_int.hpp>`
on clang 22 segfaults inside libclang with `STATUS_ACCESS_VIOLATION` on
a null `PatternDecl`. See MeshInspector/mrbind#33 for the full repro and
stack traces.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): pass -fuse-ld=lld on compile invocations too

Clang 22's driver pre-validates linker compatibility whenever LTO flags
are present, even when only compiling. The PCH compile rule at
generate.mk:799 passes `-flto=thin` (via EXTRA_CFLAGS), and on clang 22 it
hard-errors with `LTO requires -fuse-ld=lld` before producing any output.

Adds the flag to COMPILER so PCH compilation passes the new driver check,
mirroring what LINKER already does. Clang treats `-fuse-ld` as a no-op on
non-link invocations, so this is safe on older clangs too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): pass -c to the PCH compile rule

Clang 22's driver pre-validates linker compatibility whenever LTO flags
are present in a non-`-c` invocation. The PCH compile rule at
generate.mk:799 invokes `$(COMPILER) -o foo.gch -xc++-header foo.hpp ...
-flto=thin ...` with no `-c`, so v22 hard-errors with
`LTO requires -fuse-ld=lld` before producing any output.

Adding `-c` tells the driver this is a compile-only step (which PCH always
is), bypassing the precheck. More surgical than putting `-fuse-ld=lld`
on every compile invocation via COMPILER.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(windows): bump mrbind to capture stack trace on segfault

Picks up MeshInspector/mrbind@87d1aee, which installs LLVM's
PrintStackTraceOnErrorSignal in mrbind's main(). The previous CI
segfault during Python bindings generation printed only
`make: *** ... Segmentation fault`; with this bump the stack trace will
be in the job log, letting us pinpoint the next unguarded Sema call.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Update mrbind.

* ci(windows): bump mrbind to skip class members in buggy-substitute path

Picks up MeshInspector/mrbind@a0153e2, which adds
`!llvm::isa<clang::CXXMethodDecl>(decl)` to the
`--buggy-substitute-default-template-args` entry condition. Without it,
the path calls `Sema::SubstDecl` on a CXXConstructorDecl template's
pattern, which null-derefs inside
`TemplateDeclInstantiator::InitMethodInstantiation` on clang 22 — that's
the SIGSEGV we saw in the previous Generate C bindings failure.

Submodule also carries the in-tree LLVM stack-trace signal handler in
mrbind's `main()`, so any future libclang crash will print a backtrace
to the CI log automatically.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Udpate mrbind.

* Temporarily enable verbose mrbind logging.

* Revert Claude fix.

* Update mrbind to add more logs.

* Update mrbind.

* Bump

* Update mrbind.

---------

Co-authored-by: Egor Mikhaylov <egor.mikhaylov@meshinspector.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants