Skip to content

Commit 57ef130

Browse files
committed
Route memory-builtin recognition through AllocCallInfo. NFC
Reverse mode recognised malloc/calloc/realloc by ad-hoc name-string checks in two places: CheckAndBuildCallToMemset re-parsed the RHS for the memset size, and the in-place-realloc test re-parsed it again for the store/restore skip. The two walked the same call independently and are easy to drift apart. Introduce a small AllocCallInfo recognizer that classifies a heap-memory builtin once and owns the per-op invariants -- the memset byte-size operand (malloc arg0 / realloc arg1; calloc self-zeroes) and the in-place realloc check (LHS is realloc's own pointer argument). Both sites now go through it. No functional change: the growing realloc stays valgrind clean and the shrinking case is unchanged.
1 parent 1517207 commit 57ef130

1 file changed

Lines changed: 71 additions & 23 deletions

File tree

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 71 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -237,22 +237,80 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
237237
return atomicAddCall;
238238
}
239239

240+
namespace {
241+
// Recognises a C heap-memory builtin call and centralises the invariants
242+
// reverse mode must preserve for it, so all memory-op reasoning goes through
243+
// one place instead of ad-hoc name checks scattered across the visitor.
244+
class AllocCallInfo {
245+
public:
246+
enum class Kind { None, Malloc, Calloc, Realloc, Free };
247+
248+
AllocCallInfo() = default;
249+
250+
// Recognise E as a memory builtin; Kind::None if it is not one. Strips the
251+
// C-style cast that wraps the call (e.g. `(double*)realloc(...)`), so
252+
// IgnoreParenCasts, not IgnoreParenImpCasts, is required here.
253+
static AllocCallInfo recognize(clang::Expr* E) {
254+
auto* CE = llvm::dyn_cast_or_null<clang::CallExpr>(
255+
E ? E->IgnoreParenCasts() : nullptr);
256+
if (!CE)
257+
return {};
258+
const clang::FunctionDecl* FD = CE->getDirectCallee();
259+
if (!FD)
260+
return {};
261+
Kind k = llvm::StringSwitch<Kind>(FD->getName())
262+
.Case("malloc", Kind::Malloc)
263+
.Case("calloc", Kind::Calloc)
264+
.Case("realloc", Kind::Realloc)
265+
.Case("free", Kind::Free)
266+
.Default(Kind::None);
267+
return AllocCallInfo(k, CE);
268+
}
269+
270+
Kind kind() const { return Kind_; }
271+
clang::CallExpr* call() const { return Call_; }
272+
explicit operator bool() const { return Kind_ != Kind::None; }
273+
274+
// The number-of-bytes operand that a following memset must zero:
275+
// malloc(n) -> n, realloc(p, n) -> n. calloc self-zeroes and needs no
276+
// memset, so it (and free/none) report null here.
277+
clang::Expr* memsetByteSize() const {
278+
switch (Kind_) {
279+
case Kind::Malloc:
280+
return Call_->getArg(0);
281+
case Kind::Realloc:
282+
return Call_->getArg(1);
283+
default:
284+
return nullptr;
285+
}
286+
}
287+
288+
// True for an in-place `p = realloc(p, n)`: the LHS is realloc's own
289+
// pointer argument. Only then may the reallocated pointer be kept across
290+
// the call (realloc frees the old block, so a saved pointer would dangle).
291+
bool isInPlaceRealloc(const clang::Expr* LHS) const {
292+
if (Kind_ != Kind::Realloc || Call_->getNumArgs() == 0)
293+
return false;
294+
const auto* LDRE =
295+
llvm::dyn_cast<clang::DeclRefExpr>(LHS->IgnoreParenCasts());
296+
const auto* ArgDRE = llvm::dyn_cast<clang::DeclRefExpr>(
297+
Call_->getArg(0)->IgnoreParenCasts());
298+
return LDRE && ArgDRE && LDRE->getDecl() == ArgDRE->getDecl();
299+
}
300+
301+
private:
302+
AllocCallInfo(Kind k, clang::CallExpr* c) : Kind_(k), Call_(c) {}
303+
Kind Kind_ = Kind::None;
304+
clang::CallExpr* Call_ = nullptr;
305+
};
306+
} // namespace
307+
240308
// Both LHS (the memset destination) and the size expression are cloned here:
241309
// callers pass the derived pointer they also assign to and the malloc/realloc
242310
// size they also pass to that call, so cloning keeps the memset a distinct
243311
// subtree.
244312
Expr* ReverseModeVisitor::CheckAndBuildCallToMemset(Expr* LHS, Expr* RHS) {
245-
Expr* size = nullptr;
246-
if (auto* callExpr = dyn_cast_or_null<CallExpr>(RHS))
247-
if (auto* declRef =
248-
dyn_cast<DeclRefExpr>(callExpr->getCallee()->IgnoreImpCasts()))
249-
if (auto* FD = dyn_cast<FunctionDecl>(declRef->getDecl())) {
250-
if (FD->getNameAsString() == "malloc")
251-
size = callExpr->getArg(0);
252-
else if (FD->getNameAsString() == "realloc")
253-
size = callExpr->getArg(1);
254-
}
255-
313+
Expr* size = AllocCallInfo::recognize(RHS).memsetByteSize();
256314
if (size) {
257315
llvm::SmallVector<Expr*, 3> args = {
258316
CloneNode(LHS), getZeroInit(m_Context.IntTy), CloneNode(size)};
@@ -3174,18 +3232,8 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
31743232
// in-place realloc is not handled -- its reverse sweep may read the
31753233
// freed tail out of bounds, as it did before this change; no test
31763234
// exercises it.
3177-
bool isReallocAssignment = false;
3178-
if (opCode == BO_Assign)
3179-
if (auto* RCall = dyn_cast<CallExpr>(R->IgnoreParenCasts()))
3180-
if (const FunctionDecl* RFD = RCall->getDirectCallee())
3181-
if (RFD->getNameAsString() == "realloc" && RCall->getNumArgs()) {
3182-
// Only in-place: the LHS must be realloc's own pointer argument.
3183-
const auto* LDRE = dyn_cast<DeclRefExpr>(L->IgnoreParenCasts());
3184-
const auto* ArgDRE =
3185-
dyn_cast<DeclRefExpr>(RCall->getArg(0)->IgnoreParenCasts());
3186-
isReallocAssignment =
3187-
LDRE && ArgDRE && LDRE->getDecl() == ArgDRE->getDecl();
3188-
}
3235+
bool isReallocAssignment =
3236+
opCode == BO_Assign && AllocCallInfo::recognize(R).isInPlaceRealloc(L);
31893237

31903238
// Store the value of the LHS of the assignment in the forward pass
31913239
// and restore it in the reverse pass

0 commit comments

Comments
 (0)