Skip to content

Commit 9655813

Browse files
committed
Merge pull request #741 from AlexeyProkhin/issue726
Fix Issue #726 — Wrong alignment for struct fields on x86_64/amd64
2 parents 7fab503 + 9314fc1 commit 9655813

9 files changed

+305
-386
lines changed

ir/iraggr.cpp

+3-8
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
IrAggr::IrAggr(AggregateDeclaration* aggr)
2929
: aggrdecl(aggr),
3030
type(aggr->type),
31-
packed((type->ty == Tstruct) ? type->alignsize() == 1 : false),
3231
// above still need to be looked at
3332
init(0),
3433
constInit(0),
@@ -231,9 +230,9 @@ llvm::Constant* IrAggr::createInitializerConstant(
231230
for (itr = constants.begin(); itr != end; ++itr)
232231
types.push_back((*itr)->getType());
233232
if (!initializerType)
234-
initializerType = LLStructType::get(gIR->context(), types, packed);
233+
initializerType = LLStructType::get(gIR->context(), types, isPacked());
235234
else
236-
initializerType->setBody(types, packed);
235+
initializerType->setBody(types, isPacked());
237236
}
238237

239238
// build constant
@@ -258,10 +257,6 @@ void IrAggr::addFieldInitializers(
258257
}
259258
}
260259

261-
const bool packed = (type->ty == Tstruct)
262-
? type->alignsize() == 1
263-
: false;
264-
265260
// Build up vector with one-to-one mapping to field indices.
266261
const size_t n = decl->fields.dim;
267262
llvm::SmallVector<VarInitConst, 16> data(n);
@@ -342,7 +337,7 @@ void IrAggr::addFieldInitializers(
342337

343338
// get next aligned offset for this field
344339
size_t alignedoffset = offset;
345-
if (!packed)
340+
if (!isPacked())
346341
{
347342
alignedoffset = realignOffset(alignedoffset, vd->type);
348343
}

ir/iraggr.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ struct IrAggr
4141
/// Aggregate D type.
4242
Type* type;
4343

44-
/// true only for: align(1) struct S { ... }
45-
bool packed;
46-
4744
//////////////////////////////////////////////////////////////////////////
4845

4946
/// Create the __initZ symbol lazily.
@@ -138,6 +135,9 @@ struct IrAggr
138135
/// Create the Interface[] interfaces ClassInfo field initializer.
139136
LLConstant* getClassInfoInterfaces();
140137

138+
/// Returns true, if the LLVM struct type for the aggregate is declared as packed
139+
bool isPacked() const;
140+
141141
private:
142142
/// Recursively adds all the initializers for the given aggregate and, in
143143
/// case of a class type, all its base classes.

ir/irclass.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,11 @@ llvm::GlobalVariable * IrAggr::getInterfaceVtbl(BaseClass * b, bool new_instance
401401
return GV;
402402
}
403403

404+
bool IrAggr::isPacked() const
405+
{
406+
return static_cast<IrTypeAggr*>(type->ctype)->packed;
407+
}
408+
404409
//////////////////////////////////////////////////////////////////////////////
405410

406411
LLConstant * IrAggr::getClassInfoInterfaces()

ir/irtypeaggr.cpp

+229
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,225 @@
1616
#endif
1717

1818
#include "aggregate.h"
19+
#include "init.h"
1920

2021
#include "gen/irstate.h"
22+
#include "gen/logger.h"
23+
#include "gen/llvmhelpers.h"
24+
25+
//////////////////////////////////////////////////////////////////////////////
26+
//////////////////////////////////////////////////////////////////////////////
27+
28+
static bool isAligned(llvm::Type* type, size_t offset)
29+
{
30+
return gDataLayout->getABITypeAlignment(type) % offset == 0;
31+
}
32+
33+
size_t add_zeros(std::vector<llvm::Type*>& defaultTypes,
34+
size_t startOffset, size_t endOffset)
35+
{
36+
size_t const oldLength = defaultTypes.size();
37+
38+
llvm::Type* const eightByte = llvm::Type::getInt64Ty(gIR->context());
39+
llvm::Type* const fourByte = llvm::Type::getInt32Ty(gIR->context());
40+
llvm::Type* const twoByte = llvm::Type::getInt16Ty(gIR->context());
41+
42+
assert(startOffset <= endOffset);
43+
size_t paddingLeft = endOffset - startOffset;
44+
while (paddingLeft)
45+
{
46+
if (global.params.is64bit && paddingLeft >= 8 && isAligned(eightByte, startOffset))
47+
{
48+
defaultTypes.push_back(eightByte);
49+
startOffset += 8;
50+
}
51+
else if (paddingLeft >= 4 && isAligned(fourByte, startOffset))
52+
{
53+
defaultTypes.push_back(fourByte);
54+
startOffset += 4;
55+
}
56+
else if (paddingLeft >= 2 && isAligned(twoByte, startOffset))
57+
{
58+
defaultTypes.push_back(twoByte);
59+
startOffset += 2;
60+
}
61+
else
62+
{
63+
defaultTypes.push_back(llvm::Type::getInt8Ty(gIR->context()));
64+
startOffset += 1;
65+
}
66+
67+
paddingLeft = endOffset - startOffset;
68+
}
69+
70+
return defaultTypes.size() - oldLength;
71+
}
72+
73+
74+
bool var_offset_sort_cb(const VarDeclaration* v1, const VarDeclaration* v2)
75+
{
76+
if (v1 && v2)
77+
return v1->offset < v2->offset;
78+
// sort NULL pointers towards the end
79+
return v1 && !v2;
80+
}
81+
82+
AggrTypeBuilder::AggrTypeBuilder(bool packed) :
83+
m_offset(0), m_fieldIndex(0), m_packed(packed)
84+
{
85+
m_defaultTypes.reserve(32);
86+
}
87+
88+
void AggrTypeBuilder::addType(llvm::Type *type, unsigned size)
89+
{
90+
m_defaultTypes.push_back(type);
91+
m_offset += size;
92+
m_fieldIndex++;
93+
}
94+
95+
void AggrTypeBuilder::addAggregate(AggregateDeclaration *ad)
96+
{
97+
// mirror the ad->fields array but only fill in contributors
98+
const size_t n = ad->fields.dim;
99+
LLSmallVector<VarDeclaration*, 16> data(n, NULL);
100+
101+
// first fill in the fields with explicit initializers
102+
for (size_t index = 0; index < n; ++index)
103+
{
104+
VarDeclaration *field = ad->fields[index];
105+
106+
// init is !null for explicit inits
107+
if (field->init != NULL && !field->init->isVoidInitializer())
108+
{
109+
IF_LOG Logger::println("adding explicit initializer for struct field %s",
110+
field->toChars());
111+
112+
size_t f_size = field->type->size();
113+
size_t f_begin = field->offset;
114+
size_t f_end = f_begin + f_size;
115+
if (f_size == 0)
116+
continue;
117+
118+
data[index] = field;
119+
120+
// make sure there is no overlap
121+
for (size_t i = 0; i < index; i++)
122+
{
123+
if (data[i] != NULL)
124+
{
125+
VarDeclaration* vd = data[i];
126+
size_t v_begin = vd->offset;
127+
size_t v_end = v_begin + vd->type->size();
128+
129+
if (v_begin >= f_end || v_end <= f_begin)
130+
continue;
131+
132+
ad->error(vd->loc, "has overlapping initialization for %s and %s",
133+
field->toChars(), vd->toChars());
134+
}
135+
}
136+
}
137+
}
138+
139+
if (global.errors)
140+
{
141+
fatal();
142+
}
143+
144+
// fill in default initializers
145+
for (size_t index = 0; index < n; ++index)
146+
{
147+
if (data[index])
148+
continue;
149+
VarDeclaration *field = ad->fields[index];
150+
151+
size_t f_size = field->type->size();
152+
size_t f_begin = field->offset;
153+
size_t f_end = f_begin + f_size;
154+
if (f_size == 0)
155+
continue;
156+
157+
// make sure it doesn't overlap anything explicit
158+
bool overlaps = false;
159+
for (size_t i = 0; i < n; i++)
160+
{
161+
if (data[i])
162+
{
163+
size_t v_begin = data[i]->offset;
164+
size_t v_end = v_begin + data[i]->type->size();
165+
166+
if (v_begin >= f_end || v_end <= f_begin)
167+
continue;
168+
169+
overlaps = true;
170+
break;
171+
}
172+
}
173+
174+
// if no overlap was found, add the default initializer
175+
if (!overlaps)
176+
{
177+
IF_LOG Logger::println("adding default initializer for struct field %s",
178+
field->toChars());
179+
data[index] = field;
180+
}
181+
}
182+
183+
//
184+
// ok. now we can build a list of llvm types. and make sure zeros are inserted if necessary.
185+
//
186+
187+
// first we sort the list by offset
188+
std::sort(data.begin(), data.end(), var_offset_sort_cb);
189+
190+
// add types to list
191+
for (size_t i = 0; i < n; i++)
192+
{
193+
VarDeclaration* vd = data[i];
194+
195+
if (vd == NULL)
196+
continue;
197+
198+
assert(vd->offset >= m_offset && "it's a bug... most likely DMD bug 2481");
199+
200+
// get next aligned offset for this type
201+
size_t alignedoffset = m_offset;
202+
if (!m_packed)
203+
{
204+
alignedoffset = realignOffset(alignedoffset, vd->type);
205+
}
206+
207+
// insert explicit padding?
208+
if (alignedoffset < vd->offset)
209+
{
210+
m_fieldIndex += add_zeros(m_defaultTypes, alignedoffset, vd->offset);
211+
}
212+
213+
// add default type
214+
m_defaultTypes.push_back(DtoType(vd->type));
215+
216+
// advance offset to right past this field
217+
m_offset = vd->offset + vd->type->size();
218+
219+
// set the field index
220+
m_varGEPIndices[vd] = m_fieldIndex;
221+
++m_fieldIndex;
222+
}
223+
}
224+
225+
void AggrTypeBuilder::alignCurrentOffset(unsigned alignment)
226+
{
227+
m_offset = (m_offset + alignment - 1) & ~(alignment - 1);
228+
}
229+
230+
void AggrTypeBuilder::addTailPadding(unsigned aggregateSize)
231+
{
232+
// tail padding?
233+
if (m_offset < aggregateSize)
234+
{
235+
add_zeros(m_defaultTypes, m_offset, aggregateSize);
236+
}
237+
}
21238

22239
//////////////////////////////////////////////////////////////////////////////
23240
//////////////////////////////////////////////////////////////////////////////
@@ -28,6 +245,18 @@ IrTypeAggr::IrTypeAggr(AggregateDeclaration * ad)
28245
{
29246
}
30247

248+
bool IrTypeAggr::isPacked(AggregateDeclaration* ad)
249+
{
250+
for (unsigned i = 0; i < ad->fields.dim; i++)
251+
{
252+
VarDeclaration* vd = static_cast<VarDeclaration*>(ad->fields.data[i]);
253+
unsigned a = vd->type->alignsize() - 1;
254+
if (((vd->offset + a) & ~a) != vd->offset)
255+
return true;
256+
}
257+
return false;
258+
}
259+
31260
void IrTypeAggr::getMemberLocation(VarDeclaration* var, unsigned& fieldIndex,
32261
unsigned& byteOffset) const
33262
{

ir/irtypeaggr.h

+28
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,27 @@ namespace llvm {
3030
class AggregateDeclaration;
3131
class VarDeclaration;
3232

33+
typedef std::map<VarDeclaration*, unsigned> VarGEPIndices;
34+
35+
class AggrTypeBuilder
36+
{
37+
public:
38+
AggrTypeBuilder(bool packed);
39+
void addType(llvm::Type *type, unsigned size);
40+
void addAggregate(AggregateDeclaration *ad);
41+
void alignCurrentOffset(unsigned alignment);
42+
void addTailPadding(unsigned aggregateSize);
43+
unsigned currentFieldIndex() const { return m_fieldIndex; }
44+
std::vector<llvm::Type*> defaultTypes() const { return m_defaultTypes; }
45+
VarGEPIndices varGEPIndices() const { return m_varGEPIndices; }
46+
protected:
47+
std::vector<llvm::Type*> m_defaultTypes;
48+
VarGEPIndices m_varGEPIndices;
49+
unsigned m_offset;
50+
unsigned m_fieldIndex;
51+
bool m_packed;
52+
};
53+
3354
/// Base class of IrTypes for aggregate types.
3455
class IrTypeAggr : public IrType
3556
{
@@ -47,10 +68,17 @@ class IrTypeAggr : public IrType
4768
/// used for resolving forward references.
4869
llvm::DIType diCompositeType;
4970

71+
/// true, if the LLVM struct type for the aggregate is declared as packed
72+
bool packed;
73+
5074
protected:
5175
///
5276
IrTypeAggr(AggregateDeclaration* ad);
5377

78+
/// Returns true, if the LLVM struct type for the aggregate must be declared
79+
/// as packed.
80+
static bool isPacked(AggregateDeclaration* ad);
81+
5482
/// AggregateDeclaration this type represents.
5583
AggregateDeclaration* aggr;
5684

0 commit comments

Comments
 (0)