Skip to content

Commit f467fa8

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 f467fa8

1 file changed

Lines changed: 70 additions & 23 deletions

File tree

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 70 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -237,22 +237,78 @@ 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+
[[nodiscard]] 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+
// getName() asserts on non-identifier names (operators, constructors),
260+
// which vector/STL code produces; the builtins are plain identifiers.
261+
if (!FD || !FD->getDeclName().isIdentifier())
262+
return {};
263+
Kind k = llvm::StringSwitch<Kind>(FD->getName())
264+
.Case("malloc", Kind::Malloc)
265+
.Case("calloc", Kind::Calloc)
266+
.Case("realloc", Kind::Realloc)
267+
.Case("free", Kind::Free)
268+
.Default(Kind::None);
269+
return AllocCallInfo(k, CE);
270+
}
271+
272+
// The number-of-bytes operand that a following memset must zero:
273+
// malloc(n) -> n, realloc(p, n) -> n. calloc self-zeroes and needs no
274+
// memset, so it (and free/none) report null here.
275+
[[nodiscard]] clang::Expr* memsetByteSize() const {
276+
switch (m_Kind) {
277+
case Kind::Malloc:
278+
return m_Call->getArg(0);
279+
case Kind::Realloc:
280+
return m_Call->getArg(1);
281+
default:
282+
return nullptr;
283+
}
284+
}
285+
286+
// True for an in-place `p = realloc(p, n)`: the LHS is realloc's own
287+
// pointer argument. Only then may the reallocated pointer be kept across
288+
// the call (realloc frees the old block, so a saved pointer would dangle).
289+
[[nodiscard]] bool isInPlaceRealloc(const clang::Expr* LHS) const {
290+
if (m_Kind != Kind::Realloc || m_Call->getNumArgs() == 0)
291+
return false;
292+
const auto* LDRE =
293+
llvm::dyn_cast<clang::DeclRefExpr>(LHS->IgnoreParenCasts());
294+
const auto* ArgDRE = llvm::dyn_cast<clang::DeclRefExpr>(
295+
m_Call->getArg(0)->IgnoreParenCasts());
296+
return LDRE && ArgDRE && LDRE->getDecl() == ArgDRE->getDecl();
297+
}
298+
299+
private:
300+
AllocCallInfo(Kind k, clang::CallExpr* c) : m_Kind(k), m_Call(c) {}
301+
Kind m_Kind = Kind::None;
302+
clang::CallExpr* m_Call = nullptr;
303+
};
304+
} // namespace
305+
240306
// Both LHS (the memset destination) and the size expression are cloned here:
241307
// callers pass the derived pointer they also assign to and the malloc/realloc
242308
// size they also pass to that call, so cloning keeps the memset a distinct
243309
// subtree.
244310
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-
311+
Expr* size = AllocCallInfo::recognize(RHS).memsetByteSize();
256312
if (size) {
257313
llvm::SmallVector<Expr*, 3> args = {
258314
CloneNode(LHS), getZeroInit(m_Context.IntTy), CloneNode(size)};
@@ -3174,18 +3230,9 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
31743230
// in-place realloc is not handled -- its reverse sweep may read the
31753231
// freed tail out of bounds, as it did before this change; no test
31763232
// 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-
}
3233+
bool isReallocAssignment =
3234+
opCode == BO_Assign &&
3235+
AllocCallInfo::recognize(R).isInPlaceRealloc(L);
31893236

31903237
// Store the value of the LHS of the assignment in the forward pass
31913238
// and restore it in the reverse pass

0 commit comments

Comments
 (0)