Skip to content

Commit bc973a5

Browse files
author
Jianchun Xu
committed
[MERGE #1775 @jianchun] swb,plugin: print error on unbarried type
Merge pull request #1775 from jianchun:ubtype For any write barrier allocations, the allocated type declaration should be write-barrier decorated. Otherwise we'd see it in "pointerClasses" and print an error on this type. Move "RecyclerPointers.h" header file include location earlier to start examing more GC types.
2 parents a5b44a1 + 51f1d77 commit bc973a5

File tree

3 files changed

+101
-66
lines changed

3 files changed

+101
-66
lines changed

‎lib/Common/Common.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ template<> struct IntMath<int64> { using Type = Int64Math; };
9797
#include "Core/FinalizableObject.h"
9898
#include "Memory/RecyclerRootPtr.h"
9999
#include "Memory/RecyclerFastAllocator.h"
100-
#include "Memory/RecyclerPointers.h"
101100
#include "Util/Pinned.h"
102101

103102
// Data Structures 2

‎lib/Common/CommonMin.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,12 @@
5858
namespace Memory
5959
{
6060
class ArenaAllocator;
61+
class Recycler;
6162
}
6263
using namespace Memory;
6364
#include "Memory/Allocator.h"
6465
#include "Memory/HeapAllocator.h"
66+
#include "Memory/RecyclerPointers.h"
6567

6668
// === Data structures Header Files ===
6769
#include "DataStructures/DefaultContainerLockPolicy.h"

‎tools/RecyclerChecker/RecyclerChecker.cpp‎

Lines changed: 99 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -44,79 +44,82 @@ class MainVisitor:
4444
{
4545
std::string typeName = recordDecl->getQualifiedNameAsString();
4646

47+
// Ignore (system/non-GC types) before seeing "Memory::NoWriteBarrierField"
4748
if (!_barrierTypeDefined)
4849
{
49-
if (typeName == "Memory::NoWriteBarrierField")
50-
{
51-
_barrierTypeDefined = true;
52-
}
53-
else
50+
if (typeName != "Memory::NoWriteBarrierField")
5451
{
5552
return true;
5653
}
54+
55+
_barrierTypeDefined = true;
5756
}
5857

59-
if (recordDecl->hasDefinition())
58+
if (!recordDecl->hasDefinition())
6059
{
61-
bool hasUnbarrieredPointer = false;
62-
const FieldDecl* unbarrieredPointerField = nullptr;
63-
bool hasBarrieredField = false;
60+
return true;
61+
}
6462

65-
for (auto field : recordDecl->fields())
66-
{
67-
const QualType qualType = field->getType();
68-
const Type* type = qualType.getTypePtr();
63+
bool hasUnbarrieredPointer = false;
64+
const FieldDecl* unbarrieredPointerField = nullptr;
65+
bool hasBarrieredField = false;
6966

70-
if (type->isPointerType())
67+
for (auto field : recordDecl->fields())
68+
{
69+
const QualType qualType = field->getType();
70+
const Type* type = qualType.getTypePtr();
71+
72+
if (type->isPointerType())
73+
{
74+
hasUnbarrieredPointer = true;
75+
if (!unbarrieredPointerField)
7176
{
72-
hasUnbarrieredPointer = true;
73-
if (!unbarrieredPointerField)
74-
{
75-
unbarrieredPointerField = field;
76-
}
77+
unbarrieredPointerField = field;
7778
}
78-
else
79+
}
80+
else
81+
{
82+
auto fieldTypeName = qualType.getAsString();
83+
if (fieldTypeName.find("WriteBarrierPtr") == 0) // starts with
7984
{
80-
auto fieldTypeName = qualType.getAsString();
81-
if (fieldTypeName.find("WriteBarrierPtr") == 0)
85+
hasBarrieredField = true;
86+
}
87+
else if (type->isCompoundType())
88+
{
89+
// If the field is a compound type,
90+
// check if it is a fully barriered type or
91+
// has unprotected pointer fields
92+
if (_pointerClasses.find(fieldTypeName) != _pointerClasses.end())
8293
{
83-
hasBarrieredField = true;
94+
hasUnbarrieredPointer = true;
95+
if (!unbarrieredPointerField)
96+
unbarrieredPointerField = field;
8497
}
85-
else if (type->isCompoundType())
98+
else if (_barrieredClasses.find(fieldTypeName) != _barrieredClasses.end())
8699
{
87-
// If the field is a compound type,
88-
// check if it is a fully barriered type or
89-
// has unprotected pointer fields
90-
if (_pointerClasses.find(fieldTypeName) != _pointerClasses.end())
91-
{
92-
hasUnbarrieredPointer = true;
93-
if (!unbarrieredPointerField)
94-
unbarrieredPointerField = field;
95-
}
96-
else if (_barrieredClasses.find(fieldTypeName) != _barrieredClasses.end())
97-
{
98-
hasBarrieredField = true;
99-
}
100+
hasBarrieredField = true;
100101
}
101102
}
102103
}
104+
}
103105

104-
if (hasUnbarrieredPointer)
106+
if (hasUnbarrieredPointer)
107+
{
108+
_pointerClasses.insert(typeName);
109+
}
110+
111+
if (hasBarrieredField)
112+
{
113+
llvm::outs() << "Type with barrier found: \"" << typeName << "\"\n";
114+
if (!hasUnbarrieredPointer)
105115
{
106-
_pointerClasses.insert(typeName);
116+
_barrieredClasses.insert(typeName);
107117
}
108-
109-
if (hasBarrieredField)
118+
else
110119
{
111-
llvm::outs()<<"Type with barrier found: \""<<typeName<<"\"\n";
112-
if (!hasUnbarrieredPointer)
113-
{
114-
_barrieredClasses.insert(typeName);
115-
}
116-
else
117-
{
118-
llvm::outs()<<"Unbarriered field: \""<<unbarrieredPointerField->getQualifiedNameAsString()<<"\"\n";
119-
}
120+
llvm::outs() << "ERROR: Unbarriered field: \""
121+
<< unbarrieredPointerField->getQualifiedNameAsString()
122+
<< "\"\n";
120123
}
121124
}
122125

@@ -135,9 +138,9 @@ class MainVisitor:
135138
return true;
136139
}
137140

138-
void RecordWriteBarrierAllocation(const string& allocationType)
141+
void RecordWriteBarrierAllocation(const QualType& allocationType)
139142
{
140-
_barrierAllocations.insert(allocationType);
143+
_barrierAllocations.insert(allocationType.getTypePtr());
141144
}
142145

143146
void RecordRecyclerAllocation(const string& allocationFunction, const string& type)
@@ -151,40 +154,71 @@ class MainVisitor:
151154
}
152155

153156
#define Dump(coll) \
154-
dump(_##coll, #coll)
157+
dump(#coll, _##coll)
155158

156159
void Inspect()
157160
{
158161
Dump(pointerClasses);
159162
Dump(barrieredClasses);
160163

161-
llvm::outs()<<"Recycler allocations\n";
164+
llvm::outs() << "Recycler allocations\n";
162165
for (auto item : _allocatorTypeMap)
163166
{
164-
dump(item.second, item.first.c_str());
167+
dump(item.first.c_str(), item.second);
165168
}
166169

167170
Dump(barrierAllocations);
168171
}
169172

170173
private:
171-
void dump(const set<string>& set, const char* name)
174+
template <class Set, class DumpItemFunc>
175+
void dump(const char* name, const Set& set, const DumpItemFunc& func)
172176
{
173-
llvm::outs()<<"-------------------------\n\n";
174-
llvm::outs()<<name<<"\n";
175-
llvm::outs()<<"-------------------------\n\n";
177+
llvm::outs() << "-------------------------\n\n";
178+
llvm::outs() << name << "\n";
179+
llvm::outs() << "-------------------------\n\n";
176180
for (auto item : set)
177181
{
178-
llvm::outs()<<" "<<item<<"\n";
182+
func(llvm::outs(), item);
179183
}
180-
llvm::outs()<<"-------------------------\n\n";
184+
llvm::outs() << "-------------------------\n\n";
185+
}
186+
187+
template <class Item>
188+
void dump(const char* name, const set<Item>& set)
189+
{
190+
dump(name, set, [](raw_ostream& out, const Item& item)
191+
{
192+
out << " " << item << "\n";
193+
});
194+
}
195+
196+
void dump(const char* name, const set<const Type*> set)
197+
{
198+
dump(name, set, [this](raw_ostream& out, const Type* type)
199+
{
200+
out << " " << QualType(type, 0).getAsString() << "\n";
201+
202+
// Examine underlying "canonical" type (strip typedef) and try to
203+
// match declaration. If we find it in _pointerClasses, the type
204+
// is missing write-barrier decorations.
205+
auto r = type->getCanonicalTypeInternal()->getAsCXXRecordDecl();
206+
if (r)
207+
{
208+
auto typeName = r->getQualifiedNameAsString();
209+
if (_pointerClasses.find(typeName) != _pointerClasses.end())
210+
{
211+
out << " ERROR: Unbarried type " << typeName << "\n";
212+
}
213+
}
214+
});
181215
}
182216

183217
bool _barrierTypeDefined;
184218
map<string, set<string> > _allocatorTypeMap;
185219
set<string> _pointerClasses;
186220
set<string> _barrieredClasses;
187-
set<string> _barrierAllocations;
221+
set<const Type*> _barrierAllocations;
188222
};
189223

190224
static bool isCastToRecycler(const CXXStaticCastExpr* castNode)
@@ -240,9 +274,9 @@ bool CheckAllocationsInFunctionVisitor::VisitCXXNewExpr(CXXNewExpr* newExpr)
240274

241275
if (allocationFunctionStr.find("WithBarrier") != string::npos)
242276
{
243-
llvm::outs()<<"In \""<<_functionDecl->getQualifiedNameAsString()<<"\"\n";
244-
llvm::outs()<<" Allocating \""<<allocatedTypeStr<<"\" in write barriered memory\n";
245-
_mainVisitor->RecordWriteBarrierAllocation(allocatedTypeStr);
277+
llvm::outs() << "In \"" << _functionDecl->getQualifiedNameAsString() << "\"\n";
278+
llvm::outs() << " Allocating \"" << allocatedTypeStr << "\" in write barriered memory\n";
279+
_mainVisitor->RecordWriteBarrierAllocation(allocatedType);
246280
}
247281
}
248282
else

0 commit comments

Comments
 (0)