Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm/llvm-project/pull/70306/cl...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/70306 >From b7af057b1e8cd56ce1151d6232244c137d349ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Wed, 25 Oct 2023 08:33:30 +0200 Subject: [PATCH 1/5] [clang][Interp] Implement dynamic memory allocation handling --- clang/lib/AST/CMakeLists.txt | 1 + clang/lib/AST/Interp/ByteCodeExprGen.cpp | 76 +++++++ clang/lib/AST/Interp/ByteCodeExprGen.h | 2 + clang/lib/AST/Interp/Context.cpp | 2 +- clang/lib/AST/Interp/DynamicAllocator.cpp | 91 ++++++++ clang/lib/AST/Interp/DynamicAllocator.h | 90 ++++++++ clang/lib/AST/Interp/EvalEmitter.cpp | 4 +- clang/lib/AST/Interp/Interp.cpp | 37 ++++ clang/lib/AST/Interp/Interp.h | 94 ++++++++ clang/lib/AST/Interp/InterpState.cpp | 15 ++ clang/lib/AST/Interp/InterpState.h | 10 + clang/lib/AST/Interp/Opcodes.td | 24 +++ clang/test/AST/Interp/new-delete.cpp | 247 ++++++++++++++++++++++ 13 files changed, 690 insertions(+), 3 deletions(-) create mode 100644 clang/lib/AST/Interp/DynamicAllocator.cpp create mode 100644 clang/lib/AST/Interp/DynamicAllocator.h create mode 100644 clang/test/AST/Interp/new-delete.cpp diff --git a/clang/lib/AST/CMakeLists.txt b/clang/lib/AST/CMakeLists.txt index fe3f8c485ec1c56..1423623fb038cab 100644 --- a/clang/lib/AST/CMakeLists.txt +++ b/clang/lib/AST/CMakeLists.txt @@ -75,6 +75,7 @@ add_clang_library(clangAST Interp/Function.cpp Interp/InterpBuiltin.cpp Interp/Floating.cpp + Interp/DynamicAllocator.cpp Interp/Interp.cpp Interp/InterpBlock.cpp Interp/InterpFrame.cpp diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp index 195af664c13dafc..380ec8fc2dda661 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1629,6 +1629,82 @@ bool ByteCodeExprGen<Emitter>::VisitCXXScalarValueInitExpr( return this->visitZeroInitializer(classifyPrim(Ty), Ty, E); } +template <class Emitter> +bool ByteCodeExprGen<Emitter>::VisitCXXNewExpr(const CXXNewExpr *E) { + assert(classifyPrim(E->getType()) == PT_Ptr); + const Expr *Init = E->getInitializer(); + QualType ElementType = E->getAllocatedType(); + std::optional<PrimType> ElemT = classify(ElementType); + + const Descriptor *Desc; + if (ElemT) { + if (E->isArray()) + Desc = nullptr; // We're not going to use it in this case. + else + Desc = P.createDescriptor(E, *ElemT, Descriptor::InlineDescMD, + /*IsConst=*/false, /*IsTemporary=*/false, + /*IsMutable=*/false); + } else { + Desc = P.createDescriptor( + E, ElementType.getTypePtr(), + E->isArray() ? std::nullopt : Descriptor::InlineDescMD, + /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false, Init); + } + + if (E->isArray()) { + assert(E->getArraySize()); + PrimType SizeT = classifyPrim((*E->getArraySize())->getType()); + + if (!this->visit(*E->getArraySize())) + return false; + + if (ElemT) { + // N primitive elements. + if (!this->emitAllocN(SizeT, *ElemT, E, E)) + return false; + } else { + // N Composite elements. + if (!this->emitAllocCN(SizeT, Desc, E)) + return false; + } + + } else { + // Allocate just one element. + if (!this->emitAlloc(Desc, E)) + return false; + + if (Init) { + if (ElemT) { + if (!this->visit(Init)) + return false; + + if (!this->emitInit(*ElemT, E)) + return false; + } else { + // Composite. + if (!this->visitInitializer(Init)) + return false; + } + } + } + + if (DiscardResult) + return this->emitPopPtr(E); + + return true; +} + +template <class Emitter> +bool ByteCodeExprGen<Emitter>::VisitCXXDeleteExpr(const CXXDeleteExpr *E) { + const Expr *Arg = E->getArgument(); + + // Arg must be an lvalue. + if (!this->visit(Arg)) + return false; + + return this->emitFree(E->isArrayForm(), E); +} + template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) { if (E->containsErrors()) return false; diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 83986d3dd579ed6..f65dc9494db36ef 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -107,6 +107,8 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>, bool VisitSourceLocExpr(const SourceLocExpr *E); bool VisitOffsetOfExpr(const OffsetOfExpr *E); bool VisitCXXScalarValueInitExpr(const CXXScalarValueInitExpr *E); + bool VisitCXXNewExpr(const CXXNewExpr *E); + bool VisitCXXDeleteExpr(const CXXDeleteExpr *E); protected: bool visitExpr(const Expr *E) override; diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp index cb96e56fb5e1ad8..ede14de30419d0b 100644 --- a/clang/lib/AST/Interp/Context.cpp +++ b/clang/lib/AST/Interp/Context.cpp @@ -164,7 +164,7 @@ bool Context::Run(State &Parent, const Function *Func, APValue &Result) { State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, {}); if (Interpret(State, Result)) { assert(Stk.empty()); - return true; + return !State.maybeDiagnoseDanglingAllocations(); } // State gets destroyed here, so the Stk.clear() below doesn't accidentally diff --git a/clang/lib/AST/Interp/DynamicAllocator.cpp b/clang/lib/AST/Interp/DynamicAllocator.cpp new file mode 100644 index 000000000000000..a353d09d1f960ea --- /dev/null +++ b/clang/lib/AST/Interp/DynamicAllocator.cpp @@ -0,0 +1,91 @@ +//==---- DynamicAllocator.cpp - Types for the constexpr VM -------*- C++ -*-==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DynamicAllocator.h" +#include "InterpBlock.h" +#include "InterpState.h" + +using namespace clang; +using namespace clang::interp; + +Block *DynamicAllocator::allocate(const Expr *Source, PrimType T, + size_t NumElements) { + assert(NumElements > 0); + // Create a new descriptor for an array of the specified size and + // element type. + const Descriptor *D = allocateDescriptor( + Source, T, Descriptor::InlineDescMD, NumElements, /*IsConst=*/false, + /*IsTemporary=*/false, /*IsMutable=*/false); + return allocate(D); +} + +Block *DynamicAllocator::allocate(const Descriptor *ElementDesc, + size_t NumElements) { + assert(NumElements > 0); + // Create a new descriptor for an array of the specified size and + // element type. + const Descriptor *D = allocateDescriptor( + ElementDesc->asExpr(), ElementDesc, Descriptor::InlineDescMD, NumElements, + /*IsConst=*/false, /*IsTemporary=*/false, /*IsMutable=*/false); + return allocate(D); +} + +Block *DynamicAllocator::allocate(const Descriptor *D) { + assert(D->asExpr()); + + auto Memory = + std::make_unique<std::byte[]>(sizeof(Block) + D->getAllocSize()); + auto *B = new (Memory.get()) Block(D, /*isStatic=*/false); + B->invokeCtor(); + + InlineDescriptor *ID = reinterpret_cast<InlineDescriptor *>(B->rawData()); + ID->Desc = const_cast<Descriptor *>(D); + ID->IsActive = true; + ID->Offset = sizeof(InlineDescriptor); + ID->IsBase = false; + ID->IsFieldMutable = false; + ID->IsConst = false; + ID->IsInitialized = false; + assert(ID->Desc); + + if (auto It = AllocationSites.find(D->asExpr()); It != AllocationSites.end()) + It->second.Allocations.emplace_back(std::move(Memory)); + else + AllocationSites.insert( + {D->asExpr(), AllocationSite(std::move(Memory), D->isArray())}); + return B; +} + +void DynamicAllocator::deallocate(const Expr *Source, + const Block *BlockToDelete, InterpState &S) { + assert(AllocationSites.contains(Source)); + + auto It = AllocationSites.find(Source); + assert(It != AllocationSites.end()); + + auto &Site = It->second; + assert(Site.size() > 0); + + // Find the Block to delete. + size_t I = 0; + [[maybe_unused]] bool Found = false; + for (auto &A : Site.Allocations) { + Block *B = reinterpret_cast<Block *>(A.Memory.get()); + if (B == BlockToDelete) { + S.deallocate(B); + B->invokeDtor(); + Site.Allocations.erase(Site.Allocations.begin() + I); + Found = true; + break; + } + } + assert(Found); + + if (Site.size() == 0) + AllocationSites.erase(It); +} diff --git a/clang/lib/AST/Interp/DynamicAllocator.h b/clang/lib/AST/Interp/DynamicAllocator.h new file mode 100644 index 000000000000000..d2a1d96232b9730 --- /dev/null +++ b/clang/lib/AST/Interp/DynamicAllocator.h @@ -0,0 +1,90 @@ +//==- DynamicAllocator.h - Bytecode allocator for the constexpr VM -*- C++ +//-*-=// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_AST_INTERP_DYNAMIC_ALLOCATOR_H +#define LLVM_CLANG_AST_INTERP_DYNAMIC_ALLOCATOR_H + +#include "Descriptor.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/iterator_range.h" +#include "llvm/Support/Allocator.h" + +namespace clang { +class Expr; +namespace interp { +class Block; +class InterpState; + +/// Manages dynamic memory allocations done during bytecode interpretation. +/// +/// We manage allocations as a map from their new-expression to a list +/// of allocations. This is called an AllocationSite. For each site, we +/// record whether it was allocated using new or new[], the +/// IsArrayAllocation flag. +/// +/// For all array allocations, we need to allocat new Descriptor instances, +/// so the DynamicAllocator has a llvm::BumpPtrAllocator similar to Program. +class DynamicAllocator final { + struct Allocation { + std::unique_ptr<std::byte[]> Memory; + Allocation(std::unique_ptr<std::byte[]> Memory) + : Memory(std::move(Memory)) {} + }; + + struct AllocationSite { + llvm::SmallVector<Allocation> Allocations; + bool IsArrayAllocation = false; + + AllocationSite(std::unique_ptr<std::byte[]> Memory, bool Array) + : IsArrayAllocation(Array) { + Allocations.push_back({std::move(Memory)}); + } + + size_t size() const { return Allocations.size(); } + }; + +public: + DynamicAllocator() = default; + + unsigned getNumAllocations() const { return AllocationSites.size(); } + + /// Allocate ONE element of the given descriptor. + Block *allocate(const Descriptor *D); + /// Allocate \p NumElements primitive elements of the given type. + Block *allocate(const Expr *Source, PrimType T, size_t NumElements); + /// Allocate \p NumElements elements of the given descriptor. + Block *allocate(const Descriptor *D, size_t NumElements); + + /// Deallocate the given source+block combination. + void deallocate(const Expr *Source, const Block *BlockToDelete, + InterpState &S); + + /// Checks whether the allocation done at the given source is an array + /// allocation. + bool isArrayAllocation(const Expr *Source) const { + assert(AllocationSites.contains(Source)); + return AllocationSites.at(Source).IsArrayAllocation; + } + + // FIXME: Public because I'm not sure how to expose an iterator to it. + llvm::DenseMap<const Expr *, AllocationSite> AllocationSites; + +private: + using PoolAllocTy = llvm::BumpPtrAllocatorImpl<llvm::MallocAllocator>; + PoolAllocTy DescAllocator; + + /// Allocates a new descriptor. + template <typename... Ts> Descriptor *allocateDescriptor(Ts &&...Args) { + return new (DescAllocator) Descriptor(std::forward<Ts>(Args)...); + } +}; + +} // namespace interp +} // namespace clang +#endif diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp index 9bc42057c5f5782..989ec8c99854d9e 100644 --- a/clang/lib/AST/Interp/EvalEmitter.cpp +++ b/clang/lib/AST/Interp/EvalEmitter.cpp @@ -35,7 +35,7 @@ EvalEmitter::~EvalEmitter() { llvm::Expected<bool> EvalEmitter::interpretExpr(const Expr *E) { if (this->visitExpr(E)) - return true; + return S.maybeDiagnoseDanglingAllocations(); if (BailLocation) return llvm::make_error<ByteCodeGenError>(*BailLocation); return false; @@ -43,7 +43,7 @@ llvm::Expected<bool> EvalEmitter::interpretExpr(const Expr *E) { llvm::Expected<bool> EvalEmitter::interpretDecl(const VarDecl *VD) { if (this->visitDecl(VD)) - return true; + return S.maybeDiagnoseDanglingAllocations(); if (BailLocation) return llvm::make_error<ByteCodeGenError>(*BailLocation); return false; diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index 144b674451e353c..3d40d37498cb20f 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -586,6 +586,43 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result, return true; } +bool CheckDynamicMemoryAllocation(InterpState &S, CodePtr OpPC) { + if (S.getLangOpts().CPlusPlus20) + return true; + + const SourceInfo &E = S.Current->getSource(OpPC); + S.FFDiag(E, diag::note_constexpr_new); + return false; +} + +bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC, bool NewWasArray, + bool DeleteIsArray, const Block *B, + const Expr *NewExpr) { + if (NewWasArray == DeleteIsArray) + return true; + + const Descriptor *D = B->getDescriptor(); + + QualType TypeToDiagnose; + // We need to shuffle things around a bit here to get a better diagnostic, + // because the expression we allocated the block for was of type int*, + // but we want to get the array size right. + if (D->isArray()) { + QualType ElemQT = D->getType()->getPointeeType(); + TypeToDiagnose = S.getCtx().getConstantArrayType( + ElemQT, APInt(64, D->getNumElems(), false), nullptr, ArrayType::Normal, + 0); + } else + TypeToDiagnose = D->getType()->getPointeeType(); + + const SourceInfo &E = S.Current->getSource(OpPC); + S.FFDiag(E, diag::note_constexpr_new_delete_mismatch) + << DeleteIsArray << 0 << TypeToDiagnose; + S.Note(NewExpr->getExprLoc(), diag::note_constexpr_dynamic_alloc_here) + << NewExpr->getSourceRange(); + return false; +} + /// We aleady know the given DeclRefExpr is invalid for some reason, /// now figure out why and print appropriate diagnostics. bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) { diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 7dd415d6e460536..4dbe0b19e28ace7 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -14,6 +14,7 @@ #define LLVM_CLANG_AST_INTERP_INTERP_H #include "Boolean.h" +#include "DynamicAllocator.h" #include "Floating.h" #include "Function.h" #include "FunctionPointer.h" @@ -116,6 +117,15 @@ bool CheckCtorCall(InterpState &S, CodePtr OpPC, const Pointer &This); bool CheckPotentialReinterpretCast(InterpState &S, CodePtr OpPC, const Pointer &Ptr); +/// Checks if dynamic memory allocation is available in the current +/// language mode. +bool CheckDynamicMemoryAllocation(InterpState &S, CodePtr OpPC); + +/// Diagnose mismatched new[]/delete or new/delete[] pairs. +bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC, bool NewWasArray, + bool DeleteIsArray, const Block *B, + const Expr *NewExpr); + /// Sets the given integral value to the pointer, which is of /// a std::{weak,partial,strong}_ordering type. bool SetThreeWayComparisonField(InterpState &S, CodePtr OpPC, @@ -1365,6 +1375,17 @@ bool InitPop(InterpState &S, CodePtr OpPC) { return true; } +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool Init(InterpState &S, CodePtr OpPC) { + const T &Value = S.Stk.pop<T>(); + const Pointer &Ptr = S.Stk.peek<Pointer>(); + if (!CheckInit(S, OpPC, Ptr)) + return false; + Ptr.initialize(); + new (&Ptr.deref<T>()) T(Value); + return true; +} + /// 1) Pops the value from the stack /// 2) Peeks a pointer and gets its index \Idx /// 3) Sets the value on the pointer, leaving the pointer on the stack. @@ -2010,6 +2031,79 @@ inline bool OffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E) { return true; } +inline bool Alloc(InterpState &S, CodePtr OpPC, const Descriptor *Desc) { + assert(Desc); + + if (!CheckDynamicMemoryAllocation(S, OpPC)) + return false; + + DynamicAllocator &Allocator = S.getAllocator(); + Block *B = Allocator.allocate(Desc); + + S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); + + return true; +} + +template <PrimType Name, class SizeT = typename PrimConv<Name>::T> +inline bool AllocN(InterpState &S, CodePtr OpPC, PrimType T, + const Expr *Source) { + if (!CheckDynamicMemoryAllocation(S, OpPC)) + return false; + + SizeT NumElements = S.Stk.pop<SizeT>(); + + DynamicAllocator &Allocator = S.getAllocator(); + Block *B = Allocator.allocate(Source, T, static_cast<size_t>(NumElements)); + + S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); + + return true; +} + +template <PrimType Name, class SizeT = typename PrimConv<Name>::T> +inline bool AllocCN(InterpState &S, CodePtr OpPC, + const Descriptor *ElementDesc) { + if (!CheckDynamicMemoryAllocation(S, OpPC)) + return false; + + SizeT NumElements = S.Stk.pop<SizeT>(); + + DynamicAllocator &Allocator = S.getAllocator(); + Block *B = Allocator.allocate(ElementDesc, static_cast<size_t>(NumElements)); + + S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); + + return true; +} + +static inline bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm) { + + if (!CheckDynamicMemoryAllocation(S, OpPC)) + return false; + + const Expr *Source = nullptr; + const Block *BlockToDelete = nullptr; + { + // Extra scope for this so the block doesn't have this pointer + // pointing to it when we destroy it. + const Pointer &Ptr = S.Stk.pop<Pointer>(); + Source = Ptr.getDeclDesc()->asExpr(); + BlockToDelete = Ptr.block(); + } + + assert(Source); + + DynamicAllocator &Allocator = S.getAllocator(); + bool Result = + CheckNewDeleteForms(S, OpPC, Allocator.isArrayAllocation(Source), + DeleteIsArrayForm, BlockToDelete, Source); + + Allocator.deallocate(Source, BlockToDelete, S); + + return Result; +} + //===----------------------------------------------------------------------===// // Read opcode arguments //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/Interp/InterpState.cpp b/clang/lib/AST/Interp/InterpState.cpp index 2cb87ef07fe5882..b231dd3e1aeae8a 100644 --- a/clang/lib/AST/Interp/InterpState.cpp +++ b/clang/lib/AST/Interp/InterpState.cpp @@ -71,3 +71,18 @@ void InterpState::deallocate(Block *B) { B->invokeDtor(); } } + +bool InterpState::maybeDiagnoseDanglingAllocations() { + bool NoAllocationsLeft = (Alloc.getNumAllocations() == 0); + + if (!checkingPotentialConstantExpression()) { + for (const auto &It : Alloc.AllocationSites) { + assert(It.second.size() > 0); + + const Expr *Source = It.first; + CCEDiag(Source->getExprLoc(), diag::note_constexpr_memory_leak) + << (It.second.size() - 1) << Source->getSourceRange(); + } + } + return NoAllocationsLeft; +} diff --git a/clang/lib/AST/Interp/InterpState.h b/clang/lib/AST/Interp/InterpState.h index 8f84bf6ed2eaffa..a2aa1a985e57c69 100644 --- a/clang/lib/AST/Interp/InterpState.h +++ b/clang/lib/AST/Interp/InterpState.h @@ -14,6 +14,7 @@ #define LLVM_CLANG_AST_INTERP_INTERPSTATE_H #include "Context.h" +#include "DynamicAllocator.h" #include "Function.h" #include "InterpFrame.h" #include "InterpStack.h" @@ -94,6 +95,13 @@ class InterpState final : public State, public SourceMapper { Context &getContext() const { return Ctx; } + DynamicAllocator &getAllocator() { return Alloc; } + + /// Diagnose any dynamic allocations that haven't been freed yet. + /// Will return \c false if there were any allocations to diagnose, + /// \c true otherwise. + bool maybeDiagnoseDanglingAllocations(); + private: /// AST Walker state. State &Parent; @@ -101,6 +109,8 @@ class InterpState final : public State, public SourceMapper { DeadBlock *DeadBlocks = nullptr; /// Reference to the offset-source mapping. SourceMapper *M; + /// Allocator used for dynamic allocations performed via the program. + DynamicAllocator Alloc; public: /// Reference to the module containing all bytecode. diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td index 69068e87d5720ab..0e23ce2489c80e9 100644 --- a/clang/lib/AST/Interp/Opcodes.td +++ b/clang/lib/AST/Interp/Opcodes.td @@ -55,9 +55,12 @@ def ArgRoundingMode : ArgType { let Name = "llvm::RoundingMode"; } def ArgLETD: ArgType { let Name = "const LifetimeExtendedTemporaryDecl *"; } def ArgCastKind : ArgType { let Name = "CastKind"; } def ArgCallExpr : ArgType { let Name = "const CallExpr *"; } +def ArgExpr : ArgType { let Name = "const Expr *"; } def ArgOffsetOfExpr : ArgType { let Name = "const OffsetOfExpr *"; } def ArgDeclRef : ArgType { let Name = "const DeclRefExpr *"; } def ArgCCI : ArgType { let Name = "const ComparisonCategoryInfo *"; } +def ArgDesc : ArgType { let Name = "const Descriptor *"; } +def ArgPrimType : ArgType { let Name = "PrimType"; } //===----------------------------------------------------------------------===// // Classes of types instructions operate on. @@ -460,6 +463,7 @@ def StoreBitFieldPop : StoreBitFieldOpcode {} // [Pointer, Value] -> [] def InitPop : StoreOpcode {} +def Init : StoreOpcode {} // [Pointer, Value] -> [Pointer] def InitElem : Opcode { let Types = [AllTypeClass]; @@ -689,3 +693,23 @@ def InvalidDeclRef : Opcode { } def ArrayDecay : Opcode; + +def Alloc : Opcode { + let Args = [ArgDesc]; +} + +def AllocN : Opcode { + let Types = [IntegerTypeClass]; + let Args = [ArgPrimType, ArgExpr]; + let HasGroup = 1; +} + +def AllocCN : Opcode { + let Types = [IntegerTypeClass]; + let Args = [ArgDesc]; + let HasGroup = 1; +} + +def Free : Opcode { + let Args = [ArgBool]; +} diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp new file mode 100644 index 000000000000000..0e50a17427c8020 --- /dev/null +++ b/clang/test/AST/Interp/new-delete.cpp @@ -0,0 +1,247 @@ +// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify %s +// RUN: %clang_cc1 -verify=ref %s +// RUN: %clang_cc1 -std=c++20 -verify=ref %s + +#if __cplusplus >= 202002L +constexpr int a() { + new int(12); // expected-note {{allocation performed here was not deallocated}} \ + // ref-note {{allocation performed here was not deallocated}} + return 1; +} +static_assert(a() == 1, ""); // expected-error {{not an integral constant expression}} \ + // ref-error {{not an integral constant expression}} + +constexpr int b() { + int *i = new int(12); + int m = *i; + delete(i); + return m; +} +static_assert(b() == 12, ""); + + +struct S { + int a; + int b; + + static constexpr S *create(int a, int b) { + return new S(a, b); + } +}; + +constexpr int c() { + S *s = new S(12, 13); + + int i = s->a; + delete s; + + return i; +} +static_assert(c() == 12, ""); + +/// Dynamic allocation in function ::create(), freed in function d(). +constexpr int d() { + S* s = S::create(12, 14); + + int sum = s->a + s->b; + delete s; + return sum; +} +static_assert(d() == 26); + + +/// Test we emit the right diagnostic for several allocations done on +/// the same site. +constexpr int loop() { + for (int i = 0; i < 10; ++i) { + int *a = new int[10]; // ref-note {{not deallocated (along with 9 other memory leaks)}} \ + // expected-note {{not deallocated (along with 9 other memory leaks)}} + } + + return 1; +} +static_assert(loop() == 1, ""); // ref-error {{not an integral constant expression}} \ + // expected-error {{not an integral constant expression}} + + +/// No initializer. +constexpr int noInit() { + int *i = new int; + delete i; + return 0; +} +static_assert(noInit() == 0, ""); + + +namespace Arrays { + constexpr int d() { + int *Arr = new int[12]; + + Arr[0] = 1; + Arr[1] = 5; + + int sum = Arr[0] + Arr[1]; + delete[] Arr; + return sum; + } + static_assert(d() == 6); + + + constexpr int mismatch1() { // ref-error {{never produces a constant expression}} \ + // expected-error {{never produces a constant expression}} + int *i = new int(12); // ref-note {{allocated with 'new' here}} \ + // ref-note 2{{heap allocation performed here}} \ + // expected-note {{allocated with 'new' here}} \ + // expected-note 2{{heap allocation performed here}} + delete[] i; // ref-warning {{'delete[]' applied to a pointer that was allocated with 'new'}} \ + // ref-note 2{{array delete used to delete pointer to non-array object of type 'int'}} \ + // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'}} \ + // expected-note 2{{array delete used to delete pointer to non-array object of type 'int'}} + return 6; + } + static_assert(mismatch1() == 6); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'mismatch1()'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'mismatch1()'}} + + + constexpr int mismatch2() { // ref-error {{never produces a constant expression}} \ + // expected-error {{never produces a constant expression}} + int *i = new int[12]; // ref-note {{allocated with 'new[]' here}} \ + // ref-note 2{{heap allocation performed here}} \ + // expected-note {{allocated with 'new[]' here}} \ + // expected-note 2{{heap allocation performed here}} + delete i; // ref-warning {{'delete' applied to a pointer that was allocated with 'new[]'}} \ + // ref-note 2{{non-array delete used to delete pointer to array object of type 'int[12]'}} \ + // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'}} \ + // expected-note 2{{non-array delete used to delete pointer to array object of type 'int[12]'}} + return 6; + } + static_assert(mismatch2() == 6); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'mismatch2()'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'mismatch2()'}} + /// Array of composite elements. + constexpr int foo() { + S *ss = new S[12]; + + ss[0].a = 12; + + int m = ss[0].a; + + delete[] ss; + return m; + } + static_assert(foo() == 12); +} + +/// From test/SemaCXX/cxx2a-consteval.cpp + +namespace std { +template <typename T> struct remove_reference { using type = T; }; +template <typename T> struct remove_reference<T &> { using type = T; }; +template <typename T> struct remove_reference<T &&> { using type = T; }; +template <typename T> +constexpr typename std::remove_reference<T>::type&& move(T &&t) noexcept { + return static_cast<typename std::remove_reference<T>::type &&>(t); +} +} + +namespace cxx2a { +struct A { + int* p = new int(42); // expected-note 7{{heap allocation performed here}} \ + // ref-note 7{{heap allocation performed here}} + consteval int ret_i() const { return p ? *p : 0; } + consteval A ret_a() const { return A{}; } + constexpr ~A() { delete p; } +}; + +consteval int by_value_a(A a) { return a.ret_i(); } + +consteval int const_a_ref(const A &a) { + return a.ret_i(); +} + +consteval int rvalue_ref(const A &&a) { + return a.ret_i(); +} + +consteval const A &to_lvalue_ref(const A &&a) { + return a; +} + +void test() { + constexpr A a{ nullptr }; + { int k = A().ret_i(); } + + { A k = A().ret_a(); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{heap-allocated object is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{heap-allocated object is not a constant expression}} + { A k = to_lvalue_ref(A()); } // expected-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \ + // expected-note {{reference to temporary is not a constant expression}} \ + // expected-note {{temporary created here}} \ + // ref-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \ + // ref-note {{reference to temporary is not a constant expression}} \ + // ref-note {{temporary created here}} + { A k = to_lvalue_ref(A().ret_a()); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{heap-allocated object is not a constant expression}} \ + // expected-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \ + // expected-note {{reference to temporary is not a constant expression}} \ + // expected-note {{temporary created here}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{heap-allocated object is not a constant expression}} \ + // ref-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \ + // ref-note {{reference to temporary is not a constant expression}} \ + // ref-note {{temporary created here}} + { int k = A().ret_a().ret_i(); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{heap-allocated object is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{heap-allocated object is not a constant expression}} + { int k = by_value_a(A()); } + { int k = const_a_ref(A()); } + { int k = const_a_ref(a); } + { int k = rvalue_ref(A()); } + { int k = rvalue_ref(std::move(a)); } + { int k = const_a_ref(A().ret_a()); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{is not a constant expression}} + + + { int k = const_a_ref(to_lvalue_ref(A().ret_a())); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{is not a constant expression}} + { int k = const_a_ref(to_lvalue_ref(std::move(a))); } + { int k = by_value_a(A().ret_a()); } + { int k = by_value_a(to_lvalue_ref(static_cast<const A&&>(a))); } + { int k = (A().ret_a(), A().ret_i()); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{is not a constant expression}} \ + // expected-warning {{left operand of comma operator has no effect}} \ + // ref-warning {{left operand of comma operator has no effect}} + { int k = (const_a_ref(A().ret_a()), A().ret_i()); } // expected-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // expected-note {{is not a constant expression}} \ + // ref-error {{'cxx2a::A::ret_a' is not a constant expression}} \ + // ref-note {{is not a constant expression}} \ + // expected-warning {{left operand of comma operator has no effect}} \ + // ref-warning {{left operand of comma operator has no effect}} +} +} + +#else +/// Make sure we reject this prior to C++20 +constexpr int a() { // ref-error {{never produces a constant expression}} \ + // expected-error {{never produces a constant expression}} + delete new int(12); // ref-note 2{{dynamic memory allocation is not permitted in constant expressions until C++20}} \ + // expected-note 2{{dynamic memory allocation is not permitted in constant expressions until C++20}} + return 1; +} +static_assert(a() == 1, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'a()'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'a()'}} +#endif >From 99dc2116168f70acdef95ee68de6112848385c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 26 Oct 2023 12:56:47 +0200 Subject: [PATCH 2/5] Address review and add more tests --- clang/lib/AST/Interp/DynamicAllocator.cpp | 33 ++++++++++----------- clang/lib/AST/Interp/DynamicAllocator.h | 8 +++-- clang/lib/AST/Interp/Interp.cpp | 18 ++++++++++-- clang/lib/AST/Interp/Interp.h | 36 ++++++++++++++++++----- clang/test/AST/Interp/new-delete.cpp | 35 ++++++++++++++++++++++ 5 files changed, 99 insertions(+), 31 deletions(-) diff --git a/clang/lib/AST/Interp/DynamicAllocator.cpp b/clang/lib/AST/Interp/DynamicAllocator.cpp index a353d09d1f960ea..0b2f3141afc0ba5 100644 --- a/clang/lib/AST/Interp/DynamicAllocator.cpp +++ b/clang/lib/AST/Interp/DynamicAllocator.cpp @@ -61,31 +61,30 @@ Block *DynamicAllocator::allocate(const Descriptor *D) { return B; } -void DynamicAllocator::deallocate(const Expr *Source, +bool DynamicAllocator::deallocate(const Expr *Source, const Block *BlockToDelete, InterpState &S) { - assert(AllocationSites.contains(Source)); - auto It = AllocationSites.find(Source); - assert(It != AllocationSites.end()); + if (It == AllocationSites.end()) + return false; auto &Site = It->second; assert(Site.size() > 0); // Find the Block to delete. - size_t I = 0; - [[maybe_unused]] bool Found = false; - for (auto &A : Site.Allocations) { - Block *B = reinterpret_cast<Block *>(A.Memory.get()); - if (B == BlockToDelete) { - S.deallocate(B); - B->invokeDtor(); - Site.Allocations.erase(Site.Allocations.begin() + I); - Found = true; - break; - } - } - assert(Found); + auto AllocIt = llvm::find_if(Site.Allocations, [&](const Allocation &A) { + const Block *B = reinterpret_cast<const Block *>(A.Memory.get()); + return BlockToDelete == B; + }); + + assert(AllocIt != Site.Allocations.end()); + + Block *B = reinterpret_cast<Block *>(AllocIt->Memory.get()); + B->invokeDtor(); + S.deallocate(B); + Site.Allocations.erase(AllocIt); if (Site.size() == 0) AllocationSites.erase(It); + + return true; } diff --git a/clang/lib/AST/Interp/DynamicAllocator.h b/clang/lib/AST/Interp/DynamicAllocator.h index d2a1d96232b9730..8b8701490aa4c5d 100644 --- a/clang/lib/AST/Interp/DynamicAllocator.h +++ b/clang/lib/AST/Interp/DynamicAllocator.h @@ -62,14 +62,16 @@ class DynamicAllocator final { Block *allocate(const Descriptor *D, size_t NumElements); /// Deallocate the given source+block combination. - void deallocate(const Expr *Source, const Block *BlockToDelete, + /// Returns \c true if anything has been deallocatd, \c false otherwise. + bool deallocate(const Expr *Source, const Block *BlockToDelete, InterpState &S); /// Checks whether the allocation done at the given source is an array /// allocation. bool isArrayAllocation(const Expr *Source) const { - assert(AllocationSites.contains(Source)); - return AllocationSites.at(Source).IsArrayAllocation; + if (auto It = AllocationSites.find(Source); It != AllocationSites.end()) + return It->second.IsArrayAllocation; + return false; } // FIXME: Public because I'm not sure how to expose an iterator to it. diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index 3d40d37498cb20f..90a6e1d151650b0 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -596,13 +596,11 @@ bool CheckDynamicMemoryAllocation(InterpState &S, CodePtr OpPC) { } bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC, bool NewWasArray, - bool DeleteIsArray, const Block *B, + bool DeleteIsArray, const Descriptor *D, const Expr *NewExpr) { if (NewWasArray == DeleteIsArray) return true; - const Descriptor *D = B->getDescriptor(); - QualType TypeToDiagnose; // We need to shuffle things around a bit here to get a better diagnostic, // because the expression we allocated the block for was of type int*, @@ -623,6 +621,20 @@ bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC, bool NewWasArray, return false; } +bool CheckDeleteSource(InterpState &S, CodePtr OpPC, const Expr *Source, + const Pointer &Ptr) { + if (Source && isa<CXXNewExpr>(Source)) + return true; + + // Whatever this is, we didn't heap allocate it. + const SourceInfo &Loc = S.Current->getSource(OpPC); + // FIXME: There is a problem with pretty-printing the APValue we create + // for pointers (&local_primitive in this case). + S.FFDiag(Loc, diag::note_constexpr_delete_not_heap_alloc) << ""; + S.Note(Ptr.getDeclLoc(), diag::note_declared_at); + return false; +} + /// We aleady know the given DeclRefExpr is invalid for some reason, /// now figure out why and print appropriate diagnostics. bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) { diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 4dbe0b19e28ace7..f109962c92daf0c 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -123,9 +123,14 @@ bool CheckDynamicMemoryAllocation(InterpState &S, CodePtr OpPC); /// Diagnose mismatched new[]/delete or new/delete[] pairs. bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC, bool NewWasArray, - bool DeleteIsArray, const Block *B, + bool DeleteIsArray, const Descriptor *D, const Expr *NewExpr); +/// Check the source of the pointer passed to delete/delete[] has actually +/// been heap allocated by us. +bool CheckDeleteSource(InterpState &S, CodePtr OpPC, const Expr *Source, + const Pointer &Ptr); + /// Sets the given integral value to the pointer, which is of /// a std::{weak,partial,strong}_ordering type. bool SetThreeWayComparisonField(InterpState &S, CodePtr OpPC, @@ -2039,6 +2044,7 @@ inline bool Alloc(InterpState &S, CodePtr OpPC, const Descriptor *Desc) { DynamicAllocator &Allocator = S.getAllocator(); Block *B = Allocator.allocate(Desc); + assert(B); S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); @@ -2055,6 +2061,7 @@ inline bool AllocN(InterpState &S, CodePtr OpPC, PrimType T, DynamicAllocator &Allocator = S.getAllocator(); Block *B = Allocator.allocate(Source, T, static_cast<size_t>(NumElements)); + assert(B); S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); @@ -2071,6 +2078,7 @@ inline bool AllocCN(InterpState &S, CodePtr OpPC, DynamicAllocator &Allocator = S.getAllocator(); Block *B = Allocator.allocate(ElementDesc, static_cast<size_t>(NumElements)); + assert(B); S.Stk.push<Pointer>(B, sizeof(InlineDescriptor)); @@ -2088,20 +2096,32 @@ static inline bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm) { // Extra scope for this so the block doesn't have this pointer // pointing to it when we destroy it. const Pointer &Ptr = S.Stk.pop<Pointer>(); + + // Deleteing nullptr is always fine. + if (Ptr.isZero()) + return true; + Source = Ptr.getDeclDesc()->asExpr(); BlockToDelete = Ptr.block(); - } + if (!CheckDeleteSource(S, OpPC, Source, Ptr)) + return false; + } assert(Source); + assert(BlockToDelete); DynamicAllocator &Allocator = S.getAllocator(); - bool Result = - CheckNewDeleteForms(S, OpPC, Allocator.isArrayAllocation(Source), - DeleteIsArrayForm, BlockToDelete, Source); - - Allocator.deallocate(Source, BlockToDelete, S); + bool WasArrayAlloc = Allocator.isArrayAllocation(Source); + const Descriptor *BlockDesc = BlockToDelete->getDescriptor(); - return Result; + if (!Allocator.deallocate(Source, BlockToDelete, S)) { + // Nothing has been deallocated, this must be a double-delete. + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.FFDiag(Loc, diag::note_constexpr_double_delete); + return false; + } + return CheckNewDeleteForms(S, OpPC, WasArrayAlloc, DeleteIsArrayForm, + BlockDesc, Source); } //===----------------------------------------------------------------------===// diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp index 0e50a17427c8020..552f506fc43adbc 100644 --- a/clang/test/AST/Interp/new-delete.cpp +++ b/clang/test/AST/Interp/new-delete.cpp @@ -73,6 +73,41 @@ constexpr int noInit() { } static_assert(noInit() == 0, ""); +/// Try to delete a pointer that hasn't been heap allocated. +/// FIXME: pretty-printing the pointer is broken here. +constexpr int notHeapAllocated() { // ref-error {{never produces a constant expression}} \ + // expected-error {{never produces a constant expression}} + int A = 0; // ref-note 2{{declared here}} \ + // expected-note 2{{declared here}} + delete &A; // ref-note 2{{delete of pointer '&A' that does not point to a heap-allocated object}} \ + // expected-note 2{{delete of pointer '' that does not point to a heap-allocated object}} + + return 1; +} +static_assert(notHeapAllocated() == 1, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'notHeapAllocated()'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'notHeapAllocated()'}} + +consteval int deleteNull() { + int *A = nullptr; + delete A; + return 1; +} +static_assert(deleteNull() == 1, ""); + +consteval int doubleDelete() { // ref-error {{never produces a constant expression}} \ + // expected-error {{never produces a constant expression}} + int *A = new int; + delete A; + delete A; // ref-note 2{{delete of pointer that has already been deleted}} \ + // expected-note 2{{delete of pointer that has already been deleted}} + return 1; +} +static_assert(doubleDelete() == 1); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'doubleDelete()'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'doubleDelete()'}} namespace Arrays { constexpr int d() { >From 3c2402f4542dce38cdd4ab921d886ac92dbb9148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 26 Oct 2023 13:49:04 +0200 Subject: [PATCH 3/5] Check array sizes --- clang/lib/AST/Interp/Interp.h | 27 +++++++++++++++++++++++++++ clang/lib/AST/Interp/Opcodes.td | 8 ++++++-- clang/test/AST/Interp/new-delete.cpp | 28 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index f109962c92daf0c..8486a38d8710574 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -194,6 +194,29 @@ bool CheckDivRem(InterpState &S, CodePtr OpPC, const T &LHS, const T &RHS) { return true; } +template <typename SizeT> +bool CheckArraySize(InterpState &S, CodePtr OpPC, const SizeT NumElements) { + static_assert(!SizeT::isSigned()); + + // FIXME: Both the SizeT::from() as well as the + // NumElements.toAPSInt() in this function are rather expensive. + + // FIXME: GH63562 + // APValue stores array extents as unsigned, + // so anything that is greater that unsigned would overflow when + // constructing the array, we catch this here. + SizeT MaxElements = SizeT::from(std::numeric_limits<unsigned>::max()); + if (NumElements.toAPSInt().getActiveBits() > + ConstantArrayType::getMaxSizeBits(S.getCtx()) || + NumElements > MaxElements) { + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.FFDiag(Loc, diag::note_constexpr_new_too_large) + << NumElements.toDiagnosticString(S.getCtx()); + return false; + } + return true; +} + /// Checks if the result of a floating-point operation is valid /// in the current context. bool CheckFloatResult(InterpState &S, CodePtr OpPC, const Floating &Result, @@ -2058,6 +2081,8 @@ inline bool AllocN(InterpState &S, CodePtr OpPC, PrimType T, return false; SizeT NumElements = S.Stk.pop<SizeT>(); + if (!CheckArraySize(S, OpPC, NumElements)) + return false; DynamicAllocator &Allocator = S.getAllocator(); Block *B = Allocator.allocate(Source, T, static_cast<size_t>(NumElements)); @@ -2075,6 +2100,8 @@ inline bool AllocCN(InterpState &S, CodePtr OpPC, return false; SizeT NumElements = S.Stk.pop<SizeT>(); + if (!CheckArraySize(S, OpPC, NumElements)) + return false; DynamicAllocator &Allocator = S.getAllocator(); Block *B = Allocator.allocate(ElementDesc, static_cast<size_t>(NumElements)); diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td index 0e23ce2489c80e9..09c76433a1ff15b 100644 --- a/clang/lib/AST/Interp/Opcodes.td +++ b/clang/lib/AST/Interp/Opcodes.td @@ -75,6 +75,10 @@ def IntegerTypeClass : TypeClass { Uint32, Sint64, Uint64, IntAP, IntAPS]; } +def UnsignedIntegerTypeClass : TypeClass { + let Types = [Uint8, Uint16, Uint32, Uint64, IntAP]; +} + def FixedSizeIntegralTypeClass : TypeClass { let Types = [Sint8, Uint8, Sint16, Uint16, Sint32, Uint32, Sint64, Uint64, Bool]; @@ -699,13 +703,13 @@ def Alloc : Opcode { } def AllocN : Opcode { - let Types = [IntegerTypeClass]; + let Types = [UnsignedIntegerTypeClass]; let Args = [ArgPrimType, ArgExpr]; let HasGroup = 1; } def AllocCN : Opcode { - let Types = [IntegerTypeClass]; + let Types = [UnsignedIntegerTypeClass]; let Args = [ArgDesc]; let HasGroup = 1; } diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp index 552f506fc43adbc..33d0fc3eb7e9e24 100644 --- a/clang/test/AST/Interp/new-delete.cpp +++ b/clang/test/AST/Interp/new-delete.cpp @@ -109,6 +109,34 @@ static_assert(doubleDelete() == 1); // ref-error {{not an integral constant expr // expected-error {{not an integral constant expression}} \ // expected-note {{in call to 'doubleDelete()'}} +consteval int largeArray1(bool b) { + if (b) { + int *a = new int[1ul<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ + // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} + delete[] a; + } + return 1; +} +static_assert(largeArray1(false) == 1, ""); +static_assert(largeArray1(true) == 1, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'largeArray1(true)'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'largeArray1(true)'}} + +consteval int largeArray2(bool b) { + if (b) { + S *a = new S[1ul<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ + // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} + delete[] a; + } + return 1; +} +static_assert(largeArray2(false) == 1, ""); +static_assert(largeArray2(true) == 1, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'largeArray2(true)'}} \ + // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'largeArray2(true)'}} + namespace Arrays { constexpr int d() { int *Arr = new int[12]; >From 0b3451eba603b422b80eaf13437b7eb6c77e307b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 26 Oct 2023 14:46:31 +0200 Subject: [PATCH 4/5] Disable a test in SemaCXX Unfortunate. --- clang/test/AST/Interp/new-delete.cpp | 8 ++++---- clang/test/SemaCXX/paren-list-agg-init.cpp | 8 ++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp index 33d0fc3eb7e9e24..21471ff009ff5fb 100644 --- a/clang/test/AST/Interp/new-delete.cpp +++ b/clang/test/AST/Interp/new-delete.cpp @@ -111,8 +111,8 @@ static_assert(doubleDelete() == 1); // ref-error {{not an integral constant expr consteval int largeArray1(bool b) { if (b) { - int *a = new int[1ul<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ - // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} + int *a = new int[1ull<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ + // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} delete[] a; } return 1; @@ -125,8 +125,8 @@ static_assert(largeArray1(true) == 1, ""); // ref-error {{not an integral consta consteval int largeArray2(bool b) { if (b) { - S *a = new S[1ul<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ - // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} + S *a = new S[1ull<<32]; // ref-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} \ + // expected-note {{cannot allocate array; evaluated array bound 4294967296 is too large}} delete[] a; } return 1; diff --git a/clang/test/SemaCXX/paren-list-agg-init.cpp b/clang/test/SemaCXX/paren-list-agg-init.cpp index f60b20e0d465685..b0e44e375fd3e7e 100644 --- a/clang/test/SemaCXX/paren-list-agg-init.cpp +++ b/clang/test/SemaCXX/paren-list-agg-init.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only -// RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter -// RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter -DNEW_INTERP +// RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only -fexperimental-new-constant-interpreter -DNEW_INTERP // RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only struct A { // expected-note 4{{candidate constructor}} @@ -271,9 +271,13 @@ O o3(0); } namespace gh63008 { +/// FIXME: This is currently disabled in the new interpreter because it can't handle +/// failure when initializing global variables. +#ifndef NEW_INTERP auto a = new A('a', {1.1}); // expected-warning@-1 {{braces around scalar init}} // beforecxx20-warning@-2 {{aggregate initialization of type 'A' from a parenthesized list of values is a C++20 extension}} +#endif } >From 23aff34f15b42af73f6841a5dbb1f0cc81e5e22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 26 Oct 2023 14:59:33 +0200 Subject: [PATCH 5/5] Add a failing test --- clang/test/AST/Interp/new-delete.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clang/test/AST/Interp/new-delete.cpp b/clang/test/AST/Interp/new-delete.cpp index 21471ff009ff5fb..20fe34b07183805 100644 --- a/clang/test/AST/Interp/new-delete.cpp +++ b/clang/test/AST/Interp/new-delete.cpp @@ -4,6 +4,16 @@ // RUN: %clang_cc1 -std=c++20 -verify=ref %s #if __cplusplus >= 202002L + +/// FIXME: This should work, but currently doesn't. +/// The global variable is visited twice, and we don't currently +/// handle failure when initializing a global variable. So the +/// second time we try to use already freed memory. +#if 0 +constexpr int *a = new int(12); +#endif + + constexpr int a() { new int(12); // expected-note {{allocation performed here was not deallocated}} \ // ref-note {{allocation performed here was not deallocated}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
