Skip to content

Commit 409913c

Browse files
committed
Undo a shrinking in-place realloc in the reverse sweep.
Since #1925 an in-place `p = realloc(p, n)` keeps the reallocated pointer for both p and its adjoint instead of saving and restoring it, because realloc frees the old block and a saved pointer would dangle. That is correct only when the buffer grows or keeps its size: a shrinking realloc leaves the reverse sweep addressing a buffer smaller than the indices the forward pass wrote, reading past its end. Track the pointer's allocation size in a size_t shadow set at malloc/calloc/realloc. At an in-place realloc, capture the pre- and post-realloc sizes in the forward pass and, in the reverse sweep, call clad::reverse_realloc to resize both the primal and adjoint buffers back to the pre-realloc size so the earlier accesses stay in bounds; the adjoint's re-grown tail is zeroed so fresh derivatives start at 0. Each realloc captures its own sizes, so chained reallocs unwind correctly. Only pointers actually reallocated in place need the shadow, so DiffCollector records them on the DiffRequest during planning -- reusing the traversal it already runs -- and reverse mode shadows exactly those, leaving every other allocation untouched. The shadow rides on the pointer's existing adjoint record rather than a separate map. Without a tracked size the previous keep-the-pointer behaviour still applies. Add Gradient/ReallocShrink.C, covering a shrinking and a chained shrink-then-grow realloc, Memcheck-clean under the valgrind row only with this change; update Gradient/Pointers.C for the shadows.
1 parent 24a56e9 commit 409913c

8 files changed

Lines changed: 307 additions & 75 deletions

File tree

include/clad/Differentiator/BuiltinDerivatives.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
#include <algorithm>
1414
#include <cmath>
15+
#include <cstdlib>
16+
#include <cstring>
1517
#include <functional>
1618

1719
#define elidable_reverse_forw __attribute__((annotate("elidable_reverse_forw")))
@@ -1600,6 +1602,30 @@ void constructor_pullback(ValueAndPushforward<T, U> rhs,
16001602
}
16011603
} // namespace class_functions
16021604
} // namespace custom_derivatives
1605+
1606+
// Reverse-mode helper for an in-place realloc: resize `ptr` back to the byte
1607+
// size it had before the forward realloc. When that regrows a buffer the
1608+
// forward pass shrank, zero the re-grown tail for adjoint buffers so fresh
1609+
// derivatives start at 0; primal buffers pass zeroGrownTail=false, their tail
1610+
// being overwritten by value restores. Returns the possibly-moved pointer.
1611+
// NOLINTBEGIN(cppcoreguidelines-no-malloc)
1612+
// NOLINTBEGIN(cppcoreguidelines-owning-memory)
1613+
// NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
1614+
inline void* reverse_realloc(void* ptr, size_t oldBytes, size_t newBytes,
1615+
bool zeroGrownTail) {
1616+
void* resized = ::realloc(ptr, oldBytes);
1617+
// realloc failed: keep the original buffer instead of leaking it.
1618+
if (!resized)
1619+
return ptr;
1620+
ptr = resized;
1621+
if (zeroGrownTail && oldBytes > newBytes)
1622+
::memset(static_cast<char*>(ptr) + newBytes, 0, oldBytes - newBytes);
1623+
return ptr;
1624+
}
1625+
// NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
1626+
// NOLINTEND(cppcoreguidelines-owning-memory)
1627+
// NOLINTEND(cppcoreguidelines-no-malloc)
1628+
16031629
} // namespace clad
16041630

16051631
// FIXME: These math functions depend on promote_2 just like pow:

include/clad/Differentiator/DiffPlanner.h

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "clang/AST/Decl.h"
1111
#include "clang/AST/DeclBase.h"
12+
#include "clang/AST/Expr.h"
1213
#include "clang/AST/ExprCXX.h"
1314
#include "clang/AST/RecursiveASTVisitor.h"
1415
#include "clang/AST/Stmt.h"
@@ -17,10 +18,13 @@
1718

1819
#include "llvm/ADT/DenseSet.h"
1920
#include "llvm/ADT/SmallVector.h"
21+
#include "llvm/ADT/StringSwitch.h"
22+
#include "llvm/Support/Casting.h"
2023
#include "llvm/Support/Compiler.h"
2124
#include "llvm/Support/SaveAndRestore.h"
2225
#include "llvm/Support/raw_ostream.h"
2326

27+
#include <cstdint>
2428
#include <functional>
2529
#include <iterator>
2630
#include <map>
@@ -49,6 +53,84 @@ using ParamInfo = std::map<const clang::FunctionDecl*, ParamSet>;
4953
/// rediscovering them inside a visitor, keeps them available to every visitor
5054
/// and correct after a request is copied and re-pointed at another Function.
5155
struct DiffRequest {
56+
/// Recognises a C heap-memory builtin call and centralises the invariants
57+
/// reverse mode must preserve for it, so all memory-op reasoning goes through
58+
/// one place instead of ad-hoc name checks scattered across the code base.
59+
/// The planner uses it to record which pointers are reallocated in place; the
60+
/// reverse-mode visitor uses it to emit and undo the resize.
61+
class AllocCallInfo {
62+
public:
63+
enum class Kind : std::uint8_t { None, Malloc, Calloc, Realloc, Free };
64+
65+
AllocCallInfo() = default;
66+
67+
// Recognise E as a memory builtin; Kind::None if it is not one. Strips the
68+
// C-style cast that wraps the call (e.g. `(double*)realloc(...)`), so
69+
// IgnoreParenCasts, not IgnoreParenImpCasts, is required here.
70+
[[nodiscard]] static AllocCallInfo recognize(clang::Expr* E) {
71+
auto* CE = llvm::dyn_cast_or_null<clang::CallExpr>(
72+
E ? E->IgnoreParenCasts() : nullptr);
73+
if (!CE)
74+
return {};
75+
const clang::FunctionDecl* FD = CE->getDirectCallee();
76+
// getName() asserts on non-identifier names (operators, constructors),
77+
// which vector/STL code produces; the builtins are plain identifiers.
78+
if (!FD || !FD->getDeclName().isIdentifier())
79+
return {};
80+
Kind k = llvm::StringSwitch<Kind>(FD->getName())
81+
.Case("malloc", Kind::Malloc)
82+
.Case("calloc", Kind::Calloc)
83+
.Case("realloc", Kind::Realloc)
84+
.Case("free", Kind::Free)
85+
.Default(Kind::None);
86+
return AllocCallInfo(k, CE);
87+
}
88+
89+
[[nodiscard]] Kind getKind() const { return m_Kind; }
90+
[[nodiscard]] clang::CallExpr* getCall() const { return m_Call; }
91+
92+
// The number-of-bytes operand that a following memset must zero:
93+
// malloc(n) -> n, realloc(p, n) -> n. calloc self-zeroes and needs no
94+
// memset, so it (and free/none) report null here.
95+
[[nodiscard]] clang::Expr* memsetByteSize() const {
96+
switch (m_Kind) {
97+
case Kind::Malloc:
98+
return m_Call->getArg(0);
99+
case Kind::Realloc:
100+
return m_Call->getArg(1);
101+
default:
102+
return nullptr;
103+
}
104+
}
105+
106+
// True for an in-place `p = realloc(p, n)`: the LHS is realloc's own
107+
// pointer argument. Only then may the reallocated pointer be kept across
108+
// the call (realloc frees the old block, so a saved pointer would dangle).
109+
[[nodiscard]] bool isInPlaceRealloc(const clang::Expr* LHS) const {
110+
if (m_Kind != Kind::Realloc || m_Call->getNumArgs() == 0)
111+
return false;
112+
const auto* LDRE =
113+
llvm::dyn_cast<clang::DeclRefExpr>(LHS->IgnoreParenCasts());
114+
const auto* ArgDRE = llvm::dyn_cast<clang::DeclRefExpr>(
115+
m_Call->getArg(0)->IgnoreParenCasts());
116+
return LDRE && ArgDRE && LDRE->getDecl() == ArgDRE->getDecl();
117+
}
118+
119+
// The pointer variable of an in-place `p = realloc(p, n)`, or null.
120+
[[nodiscard]] const clang::VarDecl*
121+
getInPlaceReallocPtr(const clang::Expr* LHS) const {
122+
if (!isInPlaceRealloc(LHS))
123+
return nullptr;
124+
return llvm::dyn_cast<clang::VarDecl>(
125+
llvm::cast<clang::DeclRefExpr>(LHS->IgnoreParenCasts())->getDecl());
126+
}
127+
128+
private:
129+
AllocCallInfo(Kind k, clang::CallExpr* c) : m_Kind(k), m_Call(c) {}
130+
Kind m_Kind = Kind::None;
131+
clang::CallExpr* m_Call = nullptr;
132+
};
133+
52134
private:
53135
/// Based on To-Be-Recorded analysis performed before differentiation, tells
54136
/// UsefulToStoreGlobal whether a variable with a given SourceLocation has to
@@ -109,6 +191,16 @@ struct DiffRequest {
109191
const clang::Expr* Args = nullptr;
110192
/// Indexes of global GPU args of function as a subset of Args.
111193
std::vector<size_t> CUDAGlobalArgsIndexes;
194+
/// Pointer variables that are the target of an in-place `p = realloc(p, n)`
195+
/// in this function, collected by DiffCollector during planning. Reverse
196+
/// mode gives exactly these an allocation-size shadow so the realloc can be
197+
/// undone; other allocated pointers get none. Empty unless the body
198+
/// reallocates in place.
199+
std::set<const clang::VarDecl*> InPlaceReallocPtrs;
200+
/// Whether VD is reallocated in place somewhere in this function.
201+
bool isInPlaceReallocated(const clang::VarDecl* VD) const {
202+
return InPlaceReallocPtrs.count(VD) != 0;
203+
}
112204
/// Requested differentiation mode, forward or reverse.
113205
DiffMode Mode = DiffMode::unknown;
114206
/// If function appears in the call to clad::gradient/differentiate,
@@ -323,6 +415,10 @@ struct RequestOptions {
323415
void Walk(clang::DeclGroupRef DGR);
324416
bool VisitCallExpr(clang::CallExpr* E);
325417
bool VisitDeclRefExpr(clang::DeclRefExpr* DRE);
418+
/// Record an in-place `p = realloc(p, n)` on the request whose body is
419+
/// being traversed, so reverse mode knows p needs an allocation-size
420+
/// shadow.
421+
bool VisitBinaryOperator(clang::BinaryOperator* BO);
326422
bool VisitCXXConstructExpr(clang::CXXConstructExpr* e);
327423
bool shouldVisitImplicitCode() const { return true; }
328424
/// Here we use TraverseLambdaExpr and not VisitLambdaExpr to ensure the

include/clad/Differentiator/ReverseModeVisitor.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,12 @@ namespace clad {
591591
/// @returns The call to memset if the condition is met, otherwise nullptr.
592592
clang::Expr* CheckAndBuildCallToMemset(clang::Expr* LHS, clang::Expr* RHS);
593593

594+
/// The byte-size operand of a malloc/calloc/realloc call, in the primal's
595+
/// terms: malloc(n) -> n, calloc(k, s) -> k*s, realloc(p, n) -> n. Returns
596+
/// null if `allocExpr` is not one of those. Used to track allocation sizes
597+
/// so a realloc can be undone in the reverse sweep.
598+
clang::Expr* buildAllocByteSize(clang::Expr* allocExpr);
599+
594600
static DeclDiff<clang::StaticAssertDecl>
595601
DifferentiateStaticAssertDecl(const clang::StaticAssertDecl* SAD);
596602

include/clad/Differentiator/VisitorBase.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,11 @@ namespace clad {
242242
struct AdjointInfo {
243243
clang::VarDecl* Decl = nullptr;
244244
enum WrapKind : std::uint8_t { Plain, Deref, ParenDeref } Wrap = Plain;
245+
/// For a pointer reallocated in place, the `size_t` variable tracking its
246+
/// current allocation size in bytes: set where the pointer is allocated,
247+
/// read where it is realloc'd to undo the resize in reverse. Null for
248+
/// every other variable.
249+
clang::VarDecl* AllocSize = nullptr;
245250
};
246251
/// Map used to keep track of variable declarations and match them
247252
/// with their derivatives.

lib/Differentiator/DiffPlanner.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,6 +1569,18 @@ static QualType GetDerivedFunctionType(const CallExpr* CE) {
15691569
return true;
15701570
}
15711571

1572+
bool DiffCollector::VisitBinaryOperator(BinaryOperator* BO) {
1573+
// Runs while TraverseFunctionDeclOnce walks a request's body, with
1574+
// m_ParentReq pointing at that request (still local, added to the graph
1575+
// only after the walk) -- so this eagerly records into the very request
1576+
// reverse mode will consume. Outside a request body m_ParentReq is null.
1577+
if (m_ParentReq && BO->getOpcode() == BO_Assign)
1578+
if (const VarDecl* p = DiffRequest::AllocCallInfo::recognize(BO->getRHS())
1579+
.getInPlaceReallocPtr(BO->getLHS()))
1580+
m_ParentReq->InPlaceReallocPtrs.insert(p);
1581+
return true;
1582+
}
1583+
15721584
bool DiffCollector::VisitDeclRefExpr(DeclRefExpr* DRE) {
15731585
// m_TopMostReq is dereferenced below; it is null when PlanNestedRequest
15741586
// walks a lazy request's body just to record its early-return flag, and no

0 commit comments

Comments
 (0)