Skip to content

Commit fc98830

Browse files
toppercjustinfargnoli
authored andcommitted
[AST] Use APIntStorage to fix memory leak in EnumConstantDecl. (llvm#78311)
EnumConstantDecl is allocated by the ASTContext allocator so the destructor is never called. This patch takes a similar approach to IntegerLiteral by using APIntStorage to allocate large APSInts using the ASTContext allocator as well. The downside is that an additional heap allocation and copy of the data needs to be made when calling getInitValue if the APSInt is large. Fixes llvm#78160.
1 parent 0c6069b commit fc98830

File tree

8 files changed

+100
-63
lines changed

8 files changed

+100
-63
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,8 @@ Bug Fixes in This Version
675675
Fixes (`#67317 <https://github.com/llvm/llvm-project/issues/67317>`_)
676676
- Clang now properly diagnoses use of stand-alone OpenMP directives after a
677677
label (including ``case`` or ``default`` labels).
678+
- Fix compiler memory leak for enums with underlying type larger than 64 bits.
679+
Fixes (`#78311 <https://github.com/llvm/llvm-project/pull/78311>`_)
678680

679681
Before:
680682

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//===--- APNumericStorage.h - Store APInt/APFloat in ASTContext -*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_AST_APNUMERICSTORAGE_H
10+
#define LLVM_CLANG_AST_APNUMERICSTORAGE_H
11+
12+
#include "llvm/ADT/APFloat.h"
13+
#include "llvm/ADT/APInt.h"
14+
15+
namespace clang {
16+
class ASTContext;
17+
18+
/// Used by IntegerLiteral/FloatingLiteral/EnumConstantDecl to store the
19+
/// numeric without leaking memory.
20+
///
21+
/// For large floats/integers, APFloat/APInt will allocate memory from the heap
22+
/// to represent these numbers. Unfortunately, when we use a BumpPtrAllocator
23+
/// to allocate IntegerLiteral/FloatingLiteral nodes the memory associated with
24+
/// the APFloat/APInt values will never get freed. APNumericStorage uses
25+
/// ASTContext's allocator for memory allocation.
26+
class APNumericStorage {
27+
union {
28+
uint64_t VAL; ///< Used to store the <= 64 bits integer value.
29+
uint64_t *pVal; ///< Used to store the >64 bits integer value.
30+
};
31+
unsigned BitWidth;
32+
33+
bool hasAllocation() const { return llvm::APInt::getNumWords(BitWidth) > 1; }
34+
35+
APNumericStorage(const APNumericStorage &) = delete;
36+
void operator=(const APNumericStorage &) = delete;
37+
38+
protected:
39+
APNumericStorage() : VAL(0), BitWidth(0) {}
40+
41+
llvm::APInt getIntValue() const {
42+
unsigned NumWords = llvm::APInt::getNumWords(BitWidth);
43+
if (NumWords > 1)
44+
return llvm::APInt(BitWidth, NumWords, pVal);
45+
else
46+
return llvm::APInt(BitWidth, VAL);
47+
}
48+
void setIntValue(const ASTContext &C, const llvm::APInt &Val);
49+
};
50+
51+
class APIntStorage : private APNumericStorage {
52+
public:
53+
llvm::APInt getValue() const { return getIntValue(); }
54+
void setValue(const ASTContext &C, const llvm::APInt &Val) {
55+
setIntValue(C, Val);
56+
}
57+
};
58+
59+
class APFloatStorage : private APNumericStorage {
60+
public:
61+
llvm::APFloat getValue(const llvm::fltSemantics &Semantics) const {
62+
return llvm::APFloat(Semantics, getIntValue());
63+
}
64+
void setValue(const ASTContext &C, const llvm::APFloat &Val) {
65+
setIntValue(C, Val.bitcastToAPInt());
66+
}
67+
};
68+
69+
} // end namespace clang
70+
71+
#endif // LLVM_CLANG_AST_APNUMERICSTORAGE_H

clang/include/clang/AST/Decl.h

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_CLANG_AST_DECL_H
1414
#define LLVM_CLANG_AST_DECL_H
1515

16+
#include "clang/AST/APNumericStorage.h"
1617
#include "clang/AST/APValue.h"
1718
#include "clang/AST/ASTContextAllocate.h"
1819
#include "clang/AST/DeclAccessPair.h"
@@ -3251,15 +3252,16 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
32513252
/// that is defined. For example, in "enum X {a,b}", each of a/b are
32523253
/// EnumConstantDecl's, X is an instance of EnumDecl, and the type of a/b is a
32533254
/// TagType for the X EnumDecl.
3254-
class EnumConstantDecl : public ValueDecl, public Mergeable<EnumConstantDecl> {
3255+
class EnumConstantDecl : public ValueDecl,
3256+
public Mergeable<EnumConstantDecl>,
3257+
public APIntStorage {
32553258
Stmt *Init; // an integer constant expression
3256-
llvm::APSInt Val; // The value.
3259+
bool IsUnsigned;
32573260

32583261
protected:
3259-
EnumConstantDecl(DeclContext *DC, SourceLocation L,
3262+
EnumConstantDecl(const ASTContext &C, DeclContext *DC, SourceLocation L,
32603263
IdentifierInfo *Id, QualType T, Expr *E,
3261-
const llvm::APSInt &V)
3262-
: ValueDecl(EnumConstant, DC, L, Id, T), Init((Stmt*)E), Val(V) {}
3264+
const llvm::APSInt &V);
32633265

32643266
public:
32653267
friend class StmtIteratorBase;
@@ -3272,10 +3274,15 @@ class EnumConstantDecl : public ValueDecl, public Mergeable<EnumConstantDecl> {
32723274

32733275
const Expr *getInitExpr() const { return (const Expr*) Init; }
32743276
Expr *getInitExpr() { return (Expr*) Init; }
3275-
const llvm::APSInt &getInitVal() const { return Val; }
3277+
llvm::APSInt getInitVal() const {
3278+
return llvm::APSInt(getValue(), IsUnsigned);
3279+
}
32763280

32773281
void setInitExpr(Expr *E) { Init = (Stmt*) E; }
3278-
void setInitVal(const llvm::APSInt &V) { Val = V; }
3282+
void setInitVal(const ASTContext &C, const llvm::APSInt &V) {
3283+
setValue(C, V);
3284+
IsUnsigned = V.isUnsigned();
3285+
}
32793286

32803287
SourceRange getSourceRange() const override LLVM_READONLY;
32813288

clang/include/clang/AST/Expr.h

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_CLANG_AST_EXPR_H
1414
#define LLVM_CLANG_AST_EXPR_H
1515

16+
#include "clang/AST/APNumericStorage.h"
1617
#include "clang/AST/APValue.h"
1718
#include "clang/AST/ASTVector.h"
1819
#include "clang/AST/ComputeDependence.h"
@@ -1479,57 +1480,6 @@ class DeclRefExpr final
14791480
}
14801481
};
14811482

1482-
/// Used by IntegerLiteral/FloatingLiteral to store the numeric without
1483-
/// leaking memory.
1484-
///
1485-
/// For large floats/integers, APFloat/APInt will allocate memory from the heap
1486-
/// to represent these numbers. Unfortunately, when we use a BumpPtrAllocator
1487-
/// to allocate IntegerLiteral/FloatingLiteral nodes the memory associated with
1488-
/// the APFloat/APInt values will never get freed. APNumericStorage uses
1489-
/// ASTContext's allocator for memory allocation.
1490-
class APNumericStorage {
1491-
union {
1492-
uint64_t VAL; ///< Used to store the <= 64 bits integer value.
1493-
uint64_t *pVal; ///< Used to store the >64 bits integer value.
1494-
};
1495-
unsigned BitWidth;
1496-
1497-
bool hasAllocation() const { return llvm::APInt::getNumWords(BitWidth) > 1; }
1498-
1499-
APNumericStorage(const APNumericStorage &) = delete;
1500-
void operator=(const APNumericStorage &) = delete;
1501-
1502-
protected:
1503-
APNumericStorage() : VAL(0), BitWidth(0) { }
1504-
1505-
llvm::APInt getIntValue() const {
1506-
unsigned NumWords = llvm::APInt::getNumWords(BitWidth);
1507-
if (NumWords > 1)
1508-
return llvm::APInt(BitWidth, NumWords, pVal);
1509-
else
1510-
return llvm::APInt(BitWidth, VAL);
1511-
}
1512-
void setIntValue(const ASTContext &C, const llvm::APInt &Val);
1513-
};
1514-
1515-
class APIntStorage : private APNumericStorage {
1516-
public:
1517-
llvm::APInt getValue() const { return getIntValue(); }
1518-
void setValue(const ASTContext &C, const llvm::APInt &Val) {
1519-
setIntValue(C, Val);
1520-
}
1521-
};
1522-
1523-
class APFloatStorage : private APNumericStorage {
1524-
public:
1525-
llvm::APFloat getValue(const llvm::fltSemantics &Semantics) const {
1526-
return llvm::APFloat(Semantics, getIntValue());
1527-
}
1528-
void setValue(const ASTContext &C, const llvm::APFloat &Val) {
1529-
setIntValue(C, Val.bitcastToAPInt());
1530-
}
1531-
};
1532-
15331483
class IntegerLiteral : public Expr, public APIntStorage {
15341484
SourceLocation Loc;
15351485

clang/lib/AST/Decl.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5369,16 +5369,23 @@ void CapturedDecl::setBody(Stmt *B) { BodyAndNothrow.setPointer(B); }
53695369
bool CapturedDecl::isNothrow() const { return BodyAndNothrow.getInt(); }
53705370
void CapturedDecl::setNothrow(bool Nothrow) { BodyAndNothrow.setInt(Nothrow); }
53715371

5372+
EnumConstantDecl::EnumConstantDecl(const ASTContext &C, DeclContext *DC,
5373+
SourceLocation L, IdentifierInfo *Id,
5374+
QualType T, Expr *E, const llvm::APSInt &V)
5375+
: ValueDecl(EnumConstant, DC, L, Id, T), Init((Stmt *)E) {
5376+
setInitVal(C, V);
5377+
}
5378+
53725379
EnumConstantDecl *EnumConstantDecl::Create(ASTContext &C, EnumDecl *CD,
53735380
SourceLocation L,
53745381
IdentifierInfo *Id, QualType T,
53755382
Expr *E, const llvm::APSInt &V) {
5376-
return new (C, CD) EnumConstantDecl(CD, L, Id, T, E, V);
5383+
return new (C, CD) EnumConstantDecl(C, CD, L, Id, T, E, V);
53775384
}
53785385

53795386
EnumConstantDecl *
53805387
EnumConstantDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
5381-
return new (C, ID) EnumConstantDecl(nullptr, SourceLocation(), nullptr,
5388+
return new (C, ID) EnumConstantDecl(C, nullptr, SourceLocation(), nullptr,
53825389
QualType(), nullptr, llvm::APSInt());
53835390
}
53845391

clang/lib/CodeGen/CGBuiltin.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13047,7 +13047,7 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID,
1304713047
const auto *DR = cast<DeclRefExpr>(CE->getSubExpr());
1304813048
const auto *Enumerator = cast<EnumConstantDecl>(DR->getDecl());
1304913049

13050-
auto &InitVal = Enumerator->getInitVal();
13050+
auto InitVal = Enumerator->getInitVal();
1305113051
std::string InitValStr;
1305213052
if (InitVal.isNegative() || InitVal > uint64_t(INT64_MAX))
1305313053
InitValStr = std::to_string(InitVal.getSExtValue());

clang/lib/Sema/SemaDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20214,7 +20214,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
2021420214
// Adjust the APSInt value.
2021520215
InitVal = InitVal.extOrTrunc(NewWidth);
2021620216
InitVal.setIsSigned(NewSign);
20217-
ECD->setInitVal(InitVal);
20217+
ECD->setInitVal(Context, InitVal);
2021820218

2021920219
// Adjust the Expr initializer and type.
2022020220
if (ECD->getInitExpr() &&

clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ void ASTDeclReader::VisitEnumConstantDecl(EnumConstantDecl *ECD) {
910910
VisitValueDecl(ECD);
911911
if (Record.readInt())
912912
ECD->setInitExpr(Record.readExpr());
913-
ECD->setInitVal(Record.readAPSInt());
913+
ECD->setInitVal(Reader.getContext(), Record.readAPSInt());
914914
mergeMergeable(ECD);
915915
}
916916

0 commit comments

Comments
 (0)