From 8727aaf6cd90409d142d043cdc0e4634486415a9 Mon Sep 17 00:00:00 2001 From: Vaibhav Thakkar Date: Mon, 26 Feb 2024 12:41:46 +0100 Subject: [PATCH] Fix builtin macros and zero init for memory operations --- .clang-tidy | 1 + lib/Differentiator/CladUtils.cpp | 30 ++++++++++++++++++++--- lib/Differentiator/ReverseModeVisitor.cpp | 17 ++++++++++--- test/Gradient/Pointers.C | 4 +-- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 9c90276eb..894a98c74 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -28,6 +28,7 @@ Checks: > -readability-function-cognitive-complexity, -readability-implicit-bool-conversion, -cppcoreguidelines-avoid-magic-numbers, + -clang-analyzer-cplusplus.NewDeleteLeaks, CheckOptions: - key: readability-identifier-naming.ClassCase diff --git a/lib/Differentiator/CladUtils.cpp b/lib/Differentiator/CladUtils.cpp index 3d1a872c9..e7f8445c3 100644 --- a/lib/Differentiator/CladUtils.cpp +++ b/lib/Differentiator/CladUtils.cpp @@ -6,6 +6,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Basic/Builtins.h" #include "clang/Basic/SourceLocation.h" #include "clang/Sema/Lookup.h" #include "llvm/ADT/SmallVector.h" @@ -400,6 +401,13 @@ namespace clad { auto& C = semaRef.getASTContext(); if (!TSI) TSI = C.getTrivialTypeSourceInfo(qType); + bool implicitInit = false; + if (isa_and_nonnull(initializer)) + // If the initializer is an implicit value init expression, then + // we don't need to pass it explicitly to the CXXNewExpr. As, clang + // internally adds it when initializer is nullptr and DirectInitRange + // is valid. + implicitInit = true; auto newExpr = semaRef .BuildCXXNew( @@ -407,7 +415,7 @@ namespace clad { SourceRange(), qType, TSI, (arraySize ? arraySize : clad_compat::ArraySize_None()), initializer ? GetValidSRange(semaRef) : SourceRange(), - initializer) + implicitInit ? nullptr : initializer) .getAs(); return newExpr; } @@ -643,17 +651,31 @@ namespace clad { } bool IsMemoryAllocationFunction(const clang::FunctionDecl* FD) { + +#if CLANG_VERSION_MAJOR > 12 if (FD->getBuiltinID() == Builtin::BImalloc) return true; - if (FD->getBuiltinID() == Builtin::BIcalloc) + if (FD->getBuiltinID() == Builtin::ID::BIcalloc) + return true; + if (FD->getBuiltinID() == Builtin::ID::BIrealloc) + return true; +#else + if (FD->getNameAsString() == "malloc") return true; - if (FD->getBuiltinID() == Builtin::BIrealloc) + if (FD->getNameAsString() == "calloc") return true; + if (FD->getNameAsString() == "realloc") + return true; +#endif return false; } bool IsMemoryDeallocationFunction(const clang::FunctionDecl* FD) { - return FD->getBuiltinID() == Builtin::BIfree; +#if CLANG_VERSION_MAJOR > 12 + return FD->getBuiltinID() == Builtin::ID::BIfree; +#else + return FD->getNameAsString() == "free"; +#endif } } // namespace utils } // namespace clad diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp index c0acb6f0f..360e5b24b 100644 --- a/lib/Differentiator/ReverseModeVisitor.cpp +++ b/lib/Differentiator/ReverseModeVisitor.cpp @@ -2649,8 +2649,7 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, bool isPointerType = VD->getType()->isPointerType(); bool isInitializedByNewExpr = false; // Check if the variable is pointer type and initialized by new expression - if (isPointerType && (VD->getInit() != nullptr) && - isa(VD->getInit())) + if (isPointerType && VD->getInit() && isa(VD->getInit())) isInitializedByNewExpr = true; // VDDerivedInit now serves two purposes -- as the initial derivative value @@ -3824,9 +3823,19 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, Expr* clonedNewE = utils::BuildCXXNewExpr( m_Sema, CNE->getAllocatedType(), clonedArraySizeE, initializerDiff.getExpr(), CNE->getAllocatedTypeSourceInfo()); + Expr* diffInit = initializerDiff.getExpr_dx(); + if (!diffInit) { + // we should initialize it implicitly using ImplicitValueInitExpr + QualType type = CNE->getAllocatedType(); + if (CNE->isArray()) { + type = m_Context.getVariableArrayType( + type, derivedArraySizeE, ArrayType::Normal, 0, SourceRange()); + } + diffInit = new (m_Context) ImplicitValueInitExpr(type); + } Expr* derivedNewE = utils::BuildCXXNewExpr( - m_Sema, CNE->getAllocatedType(), derivedArraySizeE, - initializerDiff.getExpr_dx(), CNE->getAllocatedTypeSourceInfo()); + m_Sema, CNE->getAllocatedType(), derivedArraySizeE, diffInit, + CNE->getAllocatedTypeSourceInfo()); return {clonedNewE, derivedNewE}; } diff --git a/test/Gradient/Pointers.C b/test/Gradient/Pointers.C index c216aba6e..b52d3bf8d 100644 --- a/test/Gradient/Pointers.C +++ b/test/Gradient/Pointers.C @@ -363,7 +363,7 @@ double newAndDeletePointer(double i, double j) { // CHECK-NEXT: double *p = new double(i); // CHECK-NEXT: _d_q = new double(* _d_j); // CHECK-NEXT: double *q = new double(j); -// CHECK-NEXT: _d_r = new double [2]; +// CHECK-NEXT: _d_r = new double [2](/*implicit*/(double{{[ ]?}}[2])0); // CHECK-NEXT: double *r = new double [2]; // CHECK-NEXT: _t0 = r[0]; // CHECK-NEXT: r[0] = i + j; @@ -418,7 +418,7 @@ double structPointer (double x) { // CHECK: void structPointer_grad(double x, clad::array_ref _d_x) { // CHECK-NEXT: T *_d_t = 0; // CHECK-NEXT: double _d_res = 0; -// CHECK-NEXT: _d_t = new T; +// CHECK-NEXT: _d_t = new T(); // CHECK-NEXT: T *t = new T({x, /*implicit*/(int)0}); // CHECK-NEXT: double res = t->x; // CHECK-NEXT: goto _label0;