Skip to content

Commit c7cbbcd

Browse files
committed
Fix new/free mismatch when differentiating delegating constructors.
For a delegating constructor initializer, DifferentiateCtorInit replaced `_this`'s initializer -- set by BuildThisExpr to malloc and paired with free(_this) -- with an allocating `new ClassTy(args)`, so the new-ed pointer was then handed to free(). Valgrind reports the mismatched deallocation in Gradient/Constructors.C. Emit a placement new into the malloc'd `_this` instead, as the base-initializer path already does, keeping allocation and deallocation malloc/free.
1 parent 67937c2 commit c7cbbcd

2 files changed

Lines changed: 10 additions & 9 deletions

File tree

lib/Differentiator/ReverseModeVisitor.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -734,11 +734,12 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
734734
initDiff.getExpr(), baseTSI,
735735
{placementArg});
736736
} else if (CI->isDelegatingInitializer()) {
737-
auto* thisDRE = cast<DeclRefExpr>(thisExpr);
738-
auto* thisVD = cast<VarDecl>(thisDRE->getDecl());
739-
Expr* newInit = utils::BuildCXXNewExpr(m_Sema, baseTy, nullptr,
740-
initDiff.getExpr(), baseTSI);
741-
SetDeclInit(thisVD, newInit);
737+
// Placement-new into the malloc'd `_this` (paired with free(_this)),
738+
// as the base-initializer path above does. An allocating
739+
// `new ClassTy(args)` here would be freed with free() -- a mismatch.
740+
initCall =
741+
utils::BuildCXXNewExpr(m_Sema, baseTy, /*arraySize=*/nullptr,
742+
initDiff.getExpr(), baseTSI, {thisExpr});
742743
}
743744
}
744745
CompoundStmt* block = endBlock(direction::reverse);

test/Gradient/Constructors.C

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// RUN: ./Constructors.out | %filecheck_exec %s
33
// RUN: %cladclang -Xclang -plugin-arg-clad -Xclang -disable-tbr -Xclang -plugin-arg-clad -Xclang -enable-va %s -I%S/../../include -oConstructors.out
44
// RUN: ./Constructors.out | %filecheck_exec %s
5-
// FIXME: real new/free mismatch for delegating constructors; drop when fixed.
6-
// XFAIL: valgrind
75

86
#include "clad/Differentiator/Differentiator.h"
97
#include "clad/Differentiator/STLBuiltins.h"
@@ -368,15 +366,17 @@ double fn6(double x, double y) {
368366
} // x^2 * y
369367

370368
// CHECK: static clad::ValueAndAdjoint<argByValWrapper, argByValWrapper> constructor_reverse_forw(clad::Tag<argByValWrapper>, double v, double u, double _d_v, double _d_u) {
371-
// CHECK-NEXT: argByValWrapper *_this = new argByValWrapper(v);
369+
// CHECK-NEXT: argByValWrapper *_this = (argByValWrapper *)malloc(sizeof(argByValWrapper));
372370
// CHECK-NEXT: argByValWrapper *_d_this = (argByValWrapper *)malloc(sizeof(argByValWrapper));
373371
// CHECK-NEXT: memset(_d_this, 0, sizeof(argByValWrapper));
372+
// CHECK-NEXT: new (_this) argByValWrapper(v);
374373
// CHECK-NEXT: _this->z = _this->y * u;
375374
// CHECK-NEXT: return {*_this, *_d_this};
376375
// CHECK-NEXT: }
377376

378377
// CHECK: static void constructor_pullback(double v, double u, argByValWrapper *_d_this, double *_d_v, double *_d_u) {
379-
// CHECK-NEXT: argByValWrapper *_this = new argByValWrapper(v);
378+
// CHECK-NEXT: argByValWrapper *_this = (argByValWrapper *)malloc(sizeof(argByValWrapper));
379+
// CHECK-NEXT: new (_this) argByValWrapper(v);
380380
// CHECK-NEXT: _this->z = _this->y * u;
381381
// CHECK-NEXT: {
382382
// CHECK-NEXT: double _r_d0 = _d_this->z;

0 commit comments

Comments
 (0)