[clang][Serialization] Fix the serialization of ConstantExpr.

The serialization of ConstantExpr has currently a number of problems:

- Some fields are just not serialized (ConstantExprBits.APValueKind and
  ConstantExprBits.IsImmediateInvocation).

- ASTStmtReader::VisitConstantExpr forgets to add the trailing APValue
  to the list of objects to be destroyed when the APValue needs cleanup.

While we are at it, bring the serialization of ConstantExpr more in-line
with what is done with the other expressions by doing the following NFCs:

- Get rid of ConstantExpr::DefaultInit. It is better to not initialize
  the fields of an empty ConstantExpr since this will allow msan to
  detect if a field was not deserialized.

- Move the initialization of the fields of ConstantExpr to the constructor;
  ConstantExpr::Create allocates the memory and ConstantExpr::ConstantExpr
  is responsible for the initialization.

Review after commit since this is a straightforward mechanical fix
similar to the other serialization fixes.
This commit is contained in:
Bruno Ricci 2020-06-21 13:02:48 +01:00
parent ef3adbfc70
commit e7ce052820
No known key found for this signature in database
GPG Key ID: D58C906B2F684D92
6 changed files with 147 additions and 41 deletions

View File

@ -990,6 +990,9 @@ class ConstantExpr final
static_assert(std::is_same<uint64_t, llvm::APInt::WordType>::value,
"ConstantExpr assumes that llvm::APInt::WordType is uint64_t "
"for tail-allocated storage");
friend TrailingObjects;
friend class ASTStmtReader;
friend class ASTStmtWriter;
public:
/// Describes the kind of result that can be tail-allocated.
@ -1003,7 +1006,6 @@ private:
return ConstantExprBits.ResultKind == ConstantExpr::RSK_Int64;
}
void DefaultInit(ResultStorageKind StorageKind);
uint64_t &Int64Result() {
assert(ConstantExprBits.ResultKind == ConstantExpr::RSK_Int64 &&
"invalid accessor");
@ -1021,21 +1023,18 @@ private:
return const_cast<ConstantExpr *>(this)->APValueResult();
}
ConstantExpr(Expr *subexpr, ResultStorageKind StorageKind);
ConstantExpr(ResultStorageKind StorageKind, EmptyShell Empty);
ConstantExpr(Expr *SubExpr, ResultStorageKind StorageKind,
bool IsImmediateInvocation);
ConstantExpr(EmptyShell Empty, ResultStorageKind StorageKind);
public:
friend TrailingObjects;
friend class ASTStmtReader;
friend class ASTStmtWriter;
static ConstantExpr *Create(const ASTContext &Context, Expr *E,
const APValue &Result);
static ConstantExpr *Create(const ASTContext &Context, Expr *E,
ResultStorageKind Storage = RSK_None,
bool IsImmediateInvocation = false);
static ConstantExpr *CreateEmpty(const ASTContext &Context,
ResultStorageKind StorageKind,
EmptyShell Empty);
ResultStorageKind StorageKind);
static ResultStorageKind getStorageKind(const APValue &Value);
static ResultStorageKind getStorageKind(const Type *T,

View File

@ -325,7 +325,7 @@ protected:
/// The kind of result that is tail-allocated.
unsigned ResultKind : 2;
/// The kind of Result as defined by APValue::Kind
/// The kind of Result as defined by APValue::Kind.
unsigned APValueKind : 4;
/// When ResultKind == RSK_Int64, true if the tail-allocated integer is

View File

@ -244,6 +244,7 @@ static void AssertResultStorageKind(ConstantExpr::ResultStorageKind Kind) {
assert((Kind == ConstantExpr::RSK_APValue ||
Kind == ConstantExpr::RSK_Int64 || Kind == ConstantExpr::RSK_None) &&
"Invalid StorageKind Value");
(void)Kind;
}
ConstantExpr::ResultStorageKind
@ -268,33 +269,31 @@ ConstantExpr::getStorageKind(const Type *T, const ASTContext &Context) {
return ConstantExpr::RSK_APValue;
}
void ConstantExpr::DefaultInit(ResultStorageKind StorageKind) {
ConstantExpr::ConstantExpr(Expr *SubExpr, ResultStorageKind StorageKind,
bool IsImmediateInvocation)
: FullExpr(ConstantExprClass, SubExpr) {
ConstantExprBits.ResultKind = StorageKind;
ConstantExprBits.APValueKind = APValue::None;
ConstantExprBits.IsUnsigned = false;
ConstantExprBits.BitWidth = 0;
ConstantExprBits.HasCleanup = false;
ConstantExprBits.IsImmediateInvocation = false;
ConstantExprBits.IsImmediateInvocation = IsImmediateInvocation;
if (StorageKind == ConstantExpr::RSK_APValue)
::new (getTrailingObjects<APValue>()) APValue();
}
ConstantExpr::ConstantExpr(Expr *subexpr, ResultStorageKind StorageKind)
: FullExpr(ConstantExprClass, subexpr) {
DefaultInit(StorageKind);
}
ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
ResultStorageKind StorageKind,
bool IsImmediateInvocation) {
assert(!isa<ConstantExpr>(E));
AssertResultStorageKind(StorageKind);
unsigned Size = totalSizeToAlloc<APValue, uint64_t>(
StorageKind == ConstantExpr::RSK_APValue,
StorageKind == ConstantExpr::RSK_Int64);
void *Mem = Context.Allocate(Size, alignof(ConstantExpr));
ConstantExpr *Self = new (Mem) ConstantExpr(E, StorageKind);
Self->ConstantExprBits.IsImmediateInvocation =
IsImmediateInvocation;
return Self;
return new (Mem) ConstantExpr(E, StorageKind, IsImmediateInvocation);
}
ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
@ -305,21 +304,23 @@ ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
return Self;
}
ConstantExpr::ConstantExpr(ResultStorageKind StorageKind, EmptyShell Empty)
ConstantExpr::ConstantExpr(EmptyShell Empty, ResultStorageKind StorageKind)
: FullExpr(ConstantExprClass, Empty) {
DefaultInit(StorageKind);
ConstantExprBits.ResultKind = StorageKind;
if (StorageKind == ConstantExpr::RSK_APValue)
::new (getTrailingObjects<APValue>()) APValue();
}
ConstantExpr *ConstantExpr::CreateEmpty(const ASTContext &Context,
ResultStorageKind StorageKind,
EmptyShell Empty) {
ResultStorageKind StorageKind) {
AssertResultStorageKind(StorageKind);
unsigned Size = totalSizeToAlloc<APValue, uint64_t>(
StorageKind == ConstantExpr::RSK_APValue,
StorageKind == ConstantExpr::RSK_Int64);
void *Mem = Context.Allocate(Size, alignof(ConstantExpr));
ConstantExpr *Self = new (Mem) ConstantExpr(StorageKind, Empty);
return Self;
return new (Mem) ConstantExpr(EmptyShell(), StorageKind);
}
void ConstantExpr::MoveIntoResult(APValue &Value, const ASTContext &Context) {

View File

@ -541,18 +541,35 @@ void ASTStmtReader::VisitExpr(Expr *E) {
void ASTStmtReader::VisitConstantExpr(ConstantExpr *E) {
VisitExpr(E);
E->ConstantExprBits.ResultKind = Record.readInt();
switch (E->ConstantExprBits.ResultKind) {
case ConstantExpr::RSK_Int64: {
E->Int64Result() = Record.readInt();
uint64_t tmp = Record.readInt();
E->ConstantExprBits.IsUnsigned = tmp & 0x1;
E->ConstantExprBits.BitWidth = tmp >> 1;
auto StorageKind = Record.readInt();
assert(E->ConstantExprBits.ResultKind == StorageKind && "Wrong ResultKind!");
E->ConstantExprBits.APValueKind = Record.readInt();
E->ConstantExprBits.IsUnsigned = Record.readInt();
E->ConstantExprBits.BitWidth = Record.readInt();
E->ConstantExprBits.HasCleanup = false; // Not serialized, see below.
E->ConstantExprBits.IsImmediateInvocation = Record.readInt();
switch (StorageKind) {
case ConstantExpr::RSK_None:
break;
}
case ConstantExpr::RSK_Int64:
E->Int64Result() = Record.readInt();
break;
case ConstantExpr::RSK_APValue:
E->APValueResult() = Record.readAPValue();
if (E->APValueResult().needsCleanup()) {
E->ConstantExprBits.HasCleanup = true;
Record.getContext().addDestruction(&E->APValueResult());
}
break;
default:
llvm_unreachable("unexpected ResultKind!");
}
E->setSubExpr(Record.readSubExpr());
}
@ -2859,10 +2876,8 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
case EXPR_CONSTANT:
S = ConstantExpr::CreateEmpty(
Context,
static_cast<ConstantExpr::ResultStorageKind>(
Record[ASTStmtReader::NumExprFields]),
Empty);
Context, static_cast<ConstantExpr::ResultStorageKind>(
/*StorageKind=*/Record[ASTStmtReader::NumExprFields]));
break;
case EXPR_PREDEFINED:

View File

@ -548,16 +548,27 @@ void ASTStmtWriter::VisitExpr(Expr *E) {
void ASTStmtWriter::VisitConstantExpr(ConstantExpr *E) {
VisitExpr(E);
Record.push_back(static_cast<uint64_t>(E->ConstantExprBits.ResultKind));
Record.push_back(E->ConstantExprBits.ResultKind);
Record.push_back(E->ConstantExprBits.APValueKind);
Record.push_back(E->ConstantExprBits.IsUnsigned);
Record.push_back(E->ConstantExprBits.BitWidth);
// HasCleanup not serialized since we can just query the APValue.
Record.push_back(E->ConstantExprBits.IsImmediateInvocation);
switch (E->ConstantExprBits.ResultKind) {
case ConstantExpr::RSK_None:
break;
case ConstantExpr::RSK_Int64:
Record.push_back(E->Int64Result());
Record.push_back(E->ConstantExprBits.IsUnsigned |
E->ConstantExprBits.BitWidth << 1);
break;
case ConstantExpr::RSK_APValue:
Record.AddAPValue(E->APValueResult());
break;
default:
llvm_unreachable("unexpected ResultKind!");
}
Record.AddStmt(E->getSubExpr());
Code = serialization::EXPR_CONSTANT;
}

View File

@ -0,0 +1,80 @@
// Test without serialization:
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -ast-dump -ast-dump-filter Test %s \
// RUN: | FileCheck --strict-whitespace --match-full-lines %s
//
// Test with serialization:
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
// RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown -include-pch %t \
// RUN: -ast-dump-all -ast-dump-filter Test /dev/null \
// RUN: | FileCheck --strict-whitespace --match-full-lines %s
// FIXME: ASTRecordReader::readAPValue and ASTRecordWriter::AddAPValue
// just give up on some APValue kinds! This *really* should be fixed.
struct array_holder {
int i[2];
};
struct S {
int i = 42;
};
union U {
int i = 42;
float f;
};
struct SU {
S s[2];
U u[3];
};
consteval int test_Int() { return 42; }
consteval float test_Float() { return 1.0f; }
consteval _Complex int test_ComplexInt() { return 1+2i; }
consteval _Complex float test_ComplexFloat() { return 1.2f+3.4fi; }
consteval __int128 test_Int128() { return (__int128)0xFFFFFFFFFFFFFFFF + (__int128)1; }
// FIXME: consteval array_holder test_Array() { return array_holder(); }
// FIXME: consteval S test_Struct() { return S(); }
// FIXME: consteval U test_Union() { return U(); }
// FIXME: consteval SU test_SU() { return SU(); }
void Test() {
(void) test_Int();
(void) test_Float();
(void) test_ComplexInt();
(void) test_ComplexFloat();
(void) test_Int128();
//(void) test_Array();
//(void) test_Struct();
//(void) test_Union();
//(void) test_SU();
}
// CHECK:Dumping Test:
// CHECK-NEXT:FunctionDecl {{.*}} <{{.*}}ast-dump-constant-expr.cpp:42:1, line:52:1> line:42:6{{( imported)?}} Test 'void ()'
// CHECK-NEXT:`-CompoundStmt {{.*}} <col:13, line:52:1>
// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:43:3, col:19> 'void' <ToVoid>
// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:19> 'int' Int: 42
// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:19> 'int'
// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> 'int (*)()' <FunctionToPointerDecay>
// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> 'int ()' lvalue Function {{.*}} 'test_Int' 'int ()'
// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:44:3, col:21> 'void' <ToVoid>
// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:21> 'float' Float: 1.000000e+00
// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:21> 'float'
// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> 'float (*)()' <FunctionToPointerDecay>
// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> 'float ()' lvalue Function {{.*}} 'test_Float' 'float ()'
// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:45:3, col:26> 'void' <ToVoid>
// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:26> '_Complex int' ComplexInt: 1, 2
// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:26> '_Complex int'
// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> '_Complex int (*)()' <FunctionToPointerDecay>
// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> '_Complex int ()' lvalue Function {{.*}} 'test_ComplexInt' '_Complex int ()'
// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:46:3, col:28> 'void' <ToVoid>
// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:28> '_Complex float' ComplexFloat: 1.200000e+00, 3.400000e+00
// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:28> '_Complex float'
// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> '_Complex float (*)()' <FunctionToPointerDecay>
// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> '_Complex float ()' lvalue Function {{.*}} 'test_ComplexFloat' '_Complex float ()'
// CHECK-NEXT: `-CStyleCastExpr {{.*}} <line:47:3, col:22> 'void' <ToVoid>
// CHECK-NEXT: `-ConstantExpr {{.*}} <col:10, col:22> '__int128' Int: 18446744073709551616
// CHECK-NEXT: `-CallExpr {{.*}} <col:10, col:22> '__int128'
// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <col:10> '__int128 (*)()' <FunctionToPointerDecay>
// CHECK-NEXT: `-DeclRefExpr {{.*}} <col:10> '__int128 ()' lvalue Function {{.*}} 'test_Int128' '__int128 ()'