Skip to content

Commit 2cd5bef

Browse files
committed
Add LLVM 22 ASan CI coverage and fix the issues it surfaced.
Add an ubu24-clang22-runtime22-asan row that consumes the llvm-asan recipe from compiler-research/ci-workflows. Three consumer-side build gaps had to be closed to let clad link against the recipe toolchain: CMAKE_C_COMPILER/CMAKE_C_FLAGS are now swapped symmetrically with their CXX counterparts for the unittests subdir (gtest otherwise configured with a mixed toolchain); CMAKE_POSITION_INDEPENDENT_CODE is forced ON (the sanitizer recipe's _INIT plumbing weakened LLVM's default -fPIC enough that libcladDifferentiator.a ended up non-PIC and hit an R_X86_64_PC32 link error); and clad_externalproject_add strips -fsanitize=* from the flags it forwards, as CppInterOp does, so the recipe -- which ships no libclang_rt -- can be consumed at all. Two latent crashes surfaced once the sanitizer runtime started shipping. clang::NestedNameSpecifier()'s default ctor builds FlagKind::Invalid in clang 22 (Null is the std::nullopt ctor), so three compat wrappers that used it for "no qualifier" produced an NNS that converted to bool true and UNREACHABLE'd inside getKind(); they now use the null sentinel. And cloneFunction's RebuildEnclosingNamespaces leaked one clang::Scope per enclosing namespace because the outer SaveAndRestore<Scope*> restored the pointer without deleting the chain; cloneFunction now returns an RAII ClonedFunction that pops the scopes (and their DeclContext pushes) in its destructor. CreateDerivativeOverload and CreateVectorModeOverload, which rebuild the same namespaces while the caller's handle is still live, bracket their state with SaveAndRestore so the outer handle does not pop past the TU and crash. Two hygiene changes round it out: DifferentiationOptions uses plain default-initialised bool instead of `: 1` bit-fields (the packed ctor did a read-modify-write of still-uninitialised storage bytes), and clad resolves its own plugin path from a clad-owned symbol -- dladdr on POSIX, GetModuleHandleExA + GetModuleFileNameA on Windows -- rather than scraping FrontendOpts().Plugins. <windows.h> is included with NOMINMAX so its min/max macros do not mangle std::numeric_limits<>::max() in llvm/ADT/Sequence.h. MSan is intentionally left out: clad runs as an in-process plugin inside a clang whose own code is not uniformly instrumented, so every std object crossing the clang->clad.so boundary reads as poisoned. Sound MSan coverage needs clad statically linked into an instrumented clang, which is follow-up infrastructure.
1 parent 9aff8ad commit 2cd5bef

16 files changed

Lines changed: 212 additions & 93 deletions

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ jobs:
4444
- { name: ubu24-clang19-runtime20, os: ubuntu-24.04, compiler: clang-19, clang-runtime: '20' }
4545
- { name: ubu24-clang20-runtime21, os: ubuntu-24.04, compiler: clang-20, clang-runtime: '21' }
4646
- { name: ubu24-clang20-runtime22, os: ubuntu-24.04, compiler: clang-20, clang-runtime: '22' }
47+
- { name: ubu24-clang22-runtime22-asan, os: ubuntu-24.04, compiler: clang-22, clang-runtime: '22', flavor: asan, extra_cmake_options: '-DLLVM_USE_SANITIZER=Address' }
4748
# --- MACOS: Last 5 Runtimes (17-21) ---
4849
# Using macOS ARM (standard) for all; Runtime 17 on Intel for x86 sanity
4950
- { name: osx-intel-runtime17, os: macos-15-intel, compiler: clang, clang-runtime: '17' }

CMakeLists.txt

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ endif()
77
enable_language(CXX)
88
set(CMAKE_CXX_EXTENSIONS NO)
99
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
10+
# clad.so links libcladDifferentiator.a; every target must be PIC.
11+
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
1012
include(GNUInstallDirs)
1113

1214
# MUST be done before call to clad project
@@ -334,14 +336,20 @@ if (NOT CLAD_BUILD_STATIC_ONLY)
334336
# need to use a supported by clad compiler. Note that's a huge hack and it is
335337
# not guaranteed to work with cmake.
336338
set(stored_cxx_compiler ${CMAKE_CXX_COMPILER})
339+
set(stored_c_compiler ${CMAKE_C_COMPILER})
337340
set(stored_cxx_flags ${CMAKE_CXX_FLAGS})
338341
set(stored_c_flags ${CMAKE_C_FLAGS})
339342
# FIXME: Remove when vgvassilev/clad#1665 is resolved.
340343
disable_werror(CMAKE_CXX_FLAGS CMAKE_C_FLAGS)
344+
# Swap both C and C++ -- clad_externalproject_add forwards both,
345+
# so a CXX-only swap leaves gtest with a mixed toolchain.
341346
set(CMAKE_CXX_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang)
342-
# Filter some unsupported flags by clang.
343-
string(REPLACE "-fno-lifetime-dse" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
344-
string(REPLACE "-Wno-class-memaccess" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
347+
set(CMAKE_C_COMPILER ${LLVM_TOOLS_BINARY_DIR}/clang)
348+
# Drop GCC-only flags now that both C and CXX route through clang.
349+
foreach(_flags CMAKE_CXX_FLAGS CMAKE_C_FLAGS)
350+
string(REPLACE "-fno-lifetime-dse" "" ${_flags} "${${_flags}}")
351+
string(REPLACE "-Wno-class-memaccess" "" ${_flags} "${${_flags}}")
352+
endforeach()
345353
endif()
346354

347355
if (NOT CLAD_DISABLE_TESTS)
@@ -355,8 +363,9 @@ if (NOT CLAD_BUILD_STATIC_ONLY)
355363
endif(CLAD_ENABLE_BENCHMARKS)
356364

357365
if (stored_cxx_compiler)
358-
# Restore the default compiler.
366+
# Restore the default compilers.
359367
set(CMAKE_CXX_COMPILER ${stored_cxx_compiler})
368+
set(CMAKE_C_COMPILER ${stored_c_compiler})
360369
set(CMAKE_CXX_FLAGS ${stored_cxx_flags})
361370
set(CMAKE_C_FLAGS ${stored_c_flags})
362371
endif()

cmake/modules/AddClad.cmake

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,15 @@ function(clad_externalproject_add NAME)
234234
remove_coverage_flags(C_FLAGS_EXT CXX_FLAGS_EXT SHARED_CREATE_CXX_FLAGS_EXT EXE_LINKER_FLAGS_EXT SHARED_LINKER_FLAGS_EXT)
235235
disable_werror(C_FLAGS_EXT CXX_FLAGS_EXT)
236236

237+
238+
# Strip sanitizer flags: 3rd-party externals build with cmake
239+
# defaults; the parent keeps its -fsanitize=* from LLVM_USE_SANITIZER.
240+
foreach(_flags C_FLAGS_EXT CXX_FLAGS_EXT)
241+
foreach(_san "-fsanitize=address" "-fsanitize=undefined" "-fsanitize=memory")
242+
string(REPLACE "${_san}" "" ${_flags} "${${_flags}}")
243+
endforeach()
244+
endforeach()
245+
237246
# Everything EXCEPT EXTRA_CMAKE_ARGS gets forwarded unchanged
238247
set(FORWARDED_ARGS ${ARG_UNPARSED_ARGUMENTS})
239248

include/clad/Differentiator/Compatibility.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,9 @@ inline bool hasNNSPrefix(clang::NestedNameSpecifier NS) {
251251
inline clang::NestedNameSpecifier getNNSPrefix(clang::NestedNameSpecifier NS) {
252252
if (NS.getKind() == clang::NestedNameSpecifier::Kind::Namespace)
253253
return NS.getAsNamespaceAndPrefix().Prefix;
254-
return clang::NestedNameSpecifier();
254+
// Null (std::nullopt), not the default ctor's FlagKind::Invalid, which
255+
// would later UNREACHABLE in getKind() if anyone inspected it.
256+
return std::nullopt;
255257
}
256258

257259
// Clang 22 (llvm/llvm-project#147835): keyword + qualifier ride inline
@@ -290,8 +292,8 @@ inline clang::QualType getElaboratedType(clang::ASTContext& C,
290292
// Clang 22: getRecordType -> getTagType with default keyword/qualifier.
291293
inline clang::QualType getRecordType(clang::ASTContext& C,
292294
const clang::RecordDecl* RD) {
293-
return C.getTagType(clang::ElaboratedTypeKeyword::None,
294-
clang::NestedNameSpecifier(), RD, /*OwnsTag=*/false);
295+
return C.getTagType(clang::ElaboratedTypeKeyword::None, std::nullopt, RD,
296+
/*OwnsTag=*/false);
295297
}
296298

297299
// Clang 22: TagDecl::getTypeForDecl() was deleted; use
@@ -351,8 +353,8 @@ inline clang::QualType getTypeDeclType(clang::ASTContext& C,
351353
#if CLANG_VERSION_MAJOR < 22
352354
return C.getTypeDeclType(TD);
353355
#else
354-
return C.getTypeDeclType(clang::ElaboratedTypeKeyword::None,
355-
clang::NestedNameSpecifier(), TD);
356+
return C.getTypeDeclType(clang::ElaboratedTypeKeyword::None, std::nullopt,
357+
TD);
356358
#endif
357359
}
358360

include/clad/Differentiator/DerivativeBuilder.h

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,36 @@ namespace clad {
5353
namespace clad {
5454
class ErrorEstimationHandler;
5555

56-
/// A pair of FunctionDecl and potential enclosing context, e.g. a function
57-
/// in nested namespaces.
58-
// This is the type returned by cloneFunction. Using OverloadedDeclWithContext
59-
// instead would lead to unnecessarily returning a nullptr in the overloaded
60-
// FD
61-
using DeclWithContext = std::pair<clang::FunctionDecl*, clang::Decl*>;
56+
class VisitorBase;
57+
58+
/// RAII handle for a cloned derivative function. cloneFunction opens one
59+
/// clang::Scope and pushes one DeclContext per enclosing namespace of the
60+
/// original function; ClonedFunction owns those and pops them on
61+
/// destruction. The contained FunctionDecl* is independently AST-owned,
62+
/// so callers may keep using it after the handle goes away.
63+
///
64+
/// The handle is neither copyable nor movable: cloneFunction returns it by
65+
/// value, but under C++17 guaranteed copy elision the prvalue constructs the
66+
/// caller's object directly (ClonedFunction r = cloneFunction(...)), so no
67+
/// move is ever performed and none needs to exist.
68+
class ClonedFunction {
69+
VisitorBase* m_Owner = nullptr;
70+
unsigned m_NamespaceCount = 0;
71+
72+
public:
73+
clang::FunctionDecl* fd = nullptr;
74+
75+
ClonedFunction(VisitorBase& VB, unsigned NamespaceCount,
76+
clang::FunctionDecl* FD)
77+
: m_Owner(&VB), m_NamespaceCount(NamespaceCount), fd(FD) {}
78+
79+
ClonedFunction(const ClonedFunction&) = delete;
80+
ClonedFunction& operator=(const ClonedFunction&) = delete;
81+
ClonedFunction(ClonedFunction&&) = delete;
82+
ClonedFunction& operator=(ClonedFunction&&) = delete;
83+
~ClonedFunction();
84+
};
85+
6286
/// Stores derivative and the corresponding overload. If no overload exist
6387
/// then `second` data member should be `nullptr`.
6488
struct DerivativeAndOverload {
@@ -95,11 +119,11 @@ struct DerivativeAndOverload {
95119
/// A flag to keep track of whether error diagnostics are requested by user
96120
/// for numerical differentiation.
97121
bool m_PrintNumericalDiffErrorDiag = false;
98-
DeclWithContext cloneFunction(const clang::FunctionDecl* FD,
99-
clad::VisitorBase& VB, clang::DeclContext* DC,
100-
clang::SourceLocation& noLoc,
101-
clang::DeclarationNameInfo name,
102-
clang::QualType functionType);
122+
ClonedFunction cloneFunction(const clang::FunctionDecl* FD,
123+
clad::VisitorBase& VB, clang::DeclContext* DC,
124+
clang::SourceLocation& noLoc,
125+
clang::DeclarationNameInfo name,
126+
clang::QualType functionType);
103127
/// Looks for a suitable overload for a given function.
104128
///
105129
/// \param[in] Name The identification information of the function

include/clad/Differentiator/VisitorBase.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -654,8 +654,16 @@ namespace clad {
654654
/// modes.
655655
clang::FunctionDecl*
656656
CreateDerivativeOverload(clang::FunctionDecl derivative);
657-
/// Rebuild a sequence of nested namespaces ending with DC.
658-
clang::NamespaceDecl* RebuildEnclosingNamespaces(clang::DeclContext* DC);
657+
/// Rebuild a sequence of nested namespaces ending with DC and return
658+
/// how many were opened. Each opens a Scope that the caller (via
659+
/// ClonedFunction's RAII handle) must balance with the same count
660+
/// passed to popEnclosingNamespaceScopes -- otherwise the Scope
661+
/// objects leak when SaveAndRestore restores the outer Scope*.
662+
unsigned RebuildEnclosingNamespaces(clang::DeclContext* DC);
663+
664+
/// Pop N namespace Scopes (and their matching DeclContext pushes).
665+
/// Used only by ClonedFunction's destructor.
666+
void popEnclosingNamespaceScopes(unsigned N);
659667
/// Clones a statement
660668
clang::Stmt* Clone(const clang::Stmt* S);
661669
/// A shorthand to simplify cloning of expressions.

lib/Differentiator/BaseForwardModeVisitor.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,11 @@ DerivativeAndOverload BaseForwardModeVisitor::Derive() {
157157

158158
m_Sema.CurContext = DC;
159159
QualType derivedFnType = GetDerivativeType();
160-
DeclWithContext result =
160+
// `result` owns the namespace Scopes cloneFunction opens; its
161+
// destructor pops them before SaveScope restores.
162+
ClonedFunction result =
161163
m_Builder.cloneFunction(FD, *this, DC, validLoc, name, derivedFnType);
162-
FunctionDecl* derivedFD = result.first;
164+
FunctionDecl* derivedFD = result.fd;
163165
m_Derivative = derivedFD;
164166

165167
// Function declaration scope
@@ -232,7 +234,7 @@ DerivativeAndOverload BaseForwardModeVisitor::Derive() {
232234

233235
endScope(); // Function decl scope
234236

235-
return DerivativeAndOverload{result.first,
237+
return DerivativeAndOverload{result.fd,
236238
/*OverloadFunctionDecl=*/nullptr};
237239
}
238240

lib/Differentiator/DerivativeBuilder.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "clad/Differentiator/Timers.h"
2525
#include "clad/Differentiator/VectorForwardModeVisitor.h"
2626
#include "clad/Differentiator/VectorPushForwardModeVisitor.h"
27+
#include "clad/Differentiator/VisitorBase.h"
2728

2829
#include "clang/AST/ASTContext.h"
2930
#include "clang/AST/Attr.h"
@@ -49,7 +50,6 @@
4950
#include <cstddef>
5051
#include <memory>
5152
#include <string>
52-
#include <utility>
5353

5454
using namespace clang;
5555

@@ -113,12 +113,14 @@ static void registerDerivative(Decl* D, Sema& S, const DiffRequest& R) {
113113
return false;
114114
}
115115

116-
DeclWithContext DerivativeBuilder::cloneFunction(
116+
ClonedFunction DerivativeBuilder::cloneFunction(
117117
const clang::FunctionDecl* FD, clad::VisitorBase& VB,
118118
clang::DeclContext* DC, clang::SourceLocation& noLoc,
119119
clang::DeclarationNameInfo name, clang::QualType functionType) {
120120
FunctionDecl* returnedFD = nullptr;
121-
NamespaceDecl* enclosingNS = nullptr;
121+
// Count of namespace Scopes RebuildEnclosingNamespaces opens for
122+
// this clone -- the returned handle pops exactly that many.
123+
unsigned NamespaceCount = 0;
122124
TypeSourceInfo* TSI = m_Context.getTrivialTypeSourceInfo(functionType);
123125
if (isa<CXXMethodDecl>(FD)) {
124126
CXXRecordDecl* CXXRD = cast<CXXRecordDecl>(DC);
@@ -136,7 +138,7 @@ static void registerDerivative(Decl* D, Sema& S, const DiffRequest& R) {
136138
returnedFD->setAccess(AS_public);
137139
} else {
138140
assert (isa<FunctionDecl>(FD) && "Unexpected!");
139-
enclosingNS = VB.RebuildEnclosingNamespaces(DC);
141+
NamespaceCount = VB.RebuildEnclosingNamespaces(DC);
140142

141143
auto TrailingRequiresClause =
142144
CLAD_COMPAT_CLANG21_getTrailingRequiresClause(FD);
@@ -167,7 +169,14 @@ static void registerDerivative(Decl* D, Sema& S, const DiffRequest& R) {
167169
}
168170
}
169171

170-
return { returnedFD, enclosingNS };
172+
return ClonedFunction{VB, NamespaceCount, returnedFD};
173+
}
174+
175+
// The destructor is defined here so the header can hold just a forward
176+
// declaration of VisitorBase.
177+
ClonedFunction::~ClonedFunction() {
178+
if (m_Owner)
179+
m_Owner->popEnclosingNamespaceScopes(m_NamespaceCount);
171180
}
172181

173182
// This method is derived from the source code of both

lib/Differentiator/HessianModeVisitor.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,11 @@ DerivativeAndOverload HessianModeVisitor::Derive() {
239239
getEnclosingNamespaceOrTUScope());
240240
m_Sema.CurContext = DC;
241241

242-
DeclWithContext result = m_Builder.cloneFunction(
242+
// `result` owns the namespace Scopes cloneFunction opens; its
243+
// destructor pops them before SaveScope restores.
244+
ClonedFunction result = m_Builder.cloneFunction(
243245
m_DiffReq.Function, *this, DC, noLoc, name, hessianFunctionType);
244-
FunctionDecl* hessianFD = result.first;
246+
FunctionDecl* hessianFD = result.fd;
245247

246248
beginScope(Scope::FunctionPrototypeScope | Scope::FunctionDeclarationScope |
247249
Scope::DeclScope);
@@ -392,7 +394,7 @@ DerivativeAndOverload HessianModeVisitor::Derive() {
392394
m_Sema.PopDeclContext();
393395
endScope(); // Function decl scope
394396

395-
return DerivativeAndOverload{result.first,
397+
return DerivativeAndOverload{result.fd,
396398
/*OverloadFunctionDecl=*/nullptr};
397399
}
398400
} // end namespace clad

lib/Differentiator/JacobianModeVisitor.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,22 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
3636

3737
QualType vectorDiffFunctionType = GetDerivativeType();
3838

39+
// Save Sema state before cloneFunction mutates it.
40+
llvm::SaveAndRestore<DeclContext*> SaveContext(m_Sema.CurContext);
41+
llvm::SaveAndRestore<Scope*> SaveScope(getCurrentScope());
3942
// Create the function declaration for the derivative.
4043
// FIXME: We should not use const_cast to get the decl context here.
4144
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
4245
auto* DC = const_cast<DeclContext*>(m_DiffReq->getDeclContext());
4346
m_Sema.CurContext = DC;
44-
DeclWithContext result = m_Builder.cloneFunction(
47+
// `result` owns the namespace Scopes cloneFunction opens; its
48+
// destructor pops them before SaveScope restores.
49+
ClonedFunction result = m_Builder.cloneFunction(
4550
m_DiffReq.Function, *this, DC, loc, name, vectorDiffFunctionType);
46-
FunctionDecl* vectorDiffFD = result.first;
51+
FunctionDecl* vectorDiffFD = result.fd;
4752
m_Derivative = vectorDiffFD;
4853

4954
// Function declaration scope
50-
llvm::SaveAndRestore<DeclContext*> SaveContext(m_Sema.CurContext);
51-
llvm::SaveAndRestore<Scope*> SaveScope(getCurrentScope());
5255
beginScope(Scope::FunctionPrototypeScope | Scope::FunctionDeclarationScope |
5356
Scope::DeclScope);
5457
m_Sema.PushFunctionScope();

0 commit comments

Comments
 (0)