[pkg-d-commits] [ldc] 112/211: Fix dynamic initialization of unions
Matthias Klumpp
mak at moszumanska.debian.org
Sun Apr 23 22:36:15 UTC 2017
This is an automated email from the git hooks/post-receive script.
mak pushed a commit to annotated tag v1.1.0
in repository ldc.
commit 0fda2d271f743d7d6a3de4e8f52a2d319e783aa7
Author: Martin <noone at nowhere.com>
Date: Sun Oct 23 19:26:27 2016 +0200
Fix dynamic initialization of unions
The front-end fills in missing init expressions (except for the nested
context pointer in most cases), so there are no implicitly initialized
fields for dynamically initialized struct literals.
The front-end is also supposed to make sure there are no overlapping
initializers by using null-expressions for overridden fields, but doesn't
in some cases (DMD issue 16471).
Instead of preferring the first one in lexical field order, now use the
lexically last one to mimic DMD.
The previous code iterated over the fields in lexical order and ignored
the initializer for a field if there were earlier declared fields with
greater offset (and an initializer expression), which is wrong for
anonymous structs inside a union:
union {
struct { int i1; long l = 123; }
struct { int i2; int x = 1; }
}
`x` was previously initialized with 0 (treated as padding).
---
gen/toir.cpp | 103 ++++++++++++++++++++++++++------------------------
ir/irtypeaggr.cpp | 28 +++++++-------
tests/codegen/union.d | 69 +++++++++++++++++++++++----------
3 files changed, 116 insertions(+), 84 deletions(-)
diff --git a/gen/toir.cpp b/gen/toir.cpp
index 0ca7a20..b242beb 100644
--- a/gen/toir.cpp
+++ b/gen/toir.cpp
@@ -75,57 +75,64 @@ static LLValue *write_zeroes(LLValue *mem, unsigned start, unsigned end) {
static void write_struct_literal(Loc loc, LLValue *mem, StructDeclaration *sd,
Expressions *elements) {
assert(elements && "struct literal has null elements");
+ const auto numMissingElements = sd->fields.dim - elements->dim;
+ assert(numMissingElements == 0 || (sd->vthis && numMissingElements == 1));
// might be reset to an actual i8* value so only a single bitcast is emitted
LLValue *voidptr = mem;
- unsigned offset = 0;
- // go through fields
+ struct Data {
+ VarDeclaration *field;
+ Expression *expr;
+ };
+ LLSmallVector<Data, 16> data;
+
+ // collect init expressions in fields declaration order
for (size_t index = 0; index < sd->fields.dim; ++index) {
- VarDeclaration *vd = sd->fields[index];
+ VarDeclaration *field = sd->fields[index];
// Skip zero-sized fields such as zero-length static arrays: `ubyte[0]
// data`.
- if (vd->size(loc) == 0)
+ if (field->type->size() == 0)
continue;
- // get initializer expression
- Expression *expr =
- (index < elements->dim) ? elements->data[index] : nullptr;
-
- if (vd->overlapped && !expr) {
- // In case of a union (overlapped field), we can't simply use the default
- // initializer.
- // Consider the type union U7727A1 { int i; double d; } and
- // the declaration U7727A1 u = { d: 1.225 };
- // The loop will first visit variable i and then d. Since d has an
- // explicit initializer, we must use this one. We should therefore skip
- // union fields with no explicit initializer.
- IF_LOG Logger::println(
- "skipping overlapped field without init expr: %s %s (+%u)",
- vd->type->toChars(), vd->toChars(), vd->offset);
- continue;
- }
+ // the initializer expression may be null for overridden overlapping fields
+ Expression *expr = (index < elements->dim ? (*elements)[index] : nullptr);
+ if (expr || field == sd->vthis) {
+ // DMD issue #16471:
+ // There may be overlapping initializer expressions in some cases.
+ // Prefer the last expression in lexical (declaration) order to mimic DMD.
+ if (field->overlapped) {
+ const unsigned f_begin = field->offset;
+ const unsigned f_end = f_begin + field->type->size();
+ const auto newEndIt =
+ std::remove_if(data.begin(), data.end(), [=](const Data &d) {
+ unsigned v_begin = d.field->offset;
+ unsigned v_end = v_begin + d.field->type->size();
+ return v_begin < f_end && v_end > f_begin;
+ });
+ data.erase(newEndIt, data.end());
+ }
- // don't re-initialize unions
- if (vd->offset < offset) {
- IF_LOG Logger::println("skipping field: %s %s (+%u)", vd->type->toChars(),
- vd->toChars(), vd->offset);
- continue;
+ data.push_back({field, expr});
}
+ }
+
+ // sort by offset
+ std::sort(data.begin(), data.end(), [](const Data &l, const Data &r) {
+ return l.field->offset < r.field->offset;
+ });
+
+ unsigned offset = 0;
+ for (const auto &d : data) {
+ const auto vd = d.field;
+ const auto expr = d.expr;
// initialize any padding so struct comparisons work
- if (vd->offset != offset)
+ if (vd->offset != offset) {
+ assert(vd->offset > offset);
voidptr = write_zeroes(voidptr, offset, vd->offset);
- offset = vd->offset + vd->type->size();
-
- // skip fields with a void initializer
- if (!expr && vd != sd->vthis && vd->_init &&
- vd->_init->isVoidInitializer()) {
- IF_LOG Logger::println(
- "skipping field with void initializer: %s %s (+%u)",
- vd->type->toChars(), vd->toChars(), vd->offset);
- continue;
+ offset = vd->offset;
}
IF_LOG Logger::println("initializing field: %s %s (+%u)",
@@ -137,30 +144,26 @@ static void write_struct_literal(Loc loc, LLValue *mem, StructDeclaration *sd,
"vars, although it can easily be made to.");
DLValue field(vd->type, DtoIndexAggregate(mem, sd, vd));
- // get initializer
+ // initialize the field
if (expr) {
- IF_LOG Logger::println("expr %llu = %s",
- static_cast<unsigned long long>(index),
- expr->toChars());
+ IF_LOG Logger::println("expr = %s", expr->toChars());
// try to construct it in-place
if (!toInPlaceConstruction(&field, expr)) {
DtoAssign(loc, &field, toElem(expr), TOKblit);
if (expr->isLvalue())
callPostblit(loc, expr, DtoLVal(&field));
}
- } else if (vd == sd->vthis) {
+ } else {
+ assert(vd == sd->vthis);
IF_LOG Logger::println("initializing vthis");
LOG_SCOPE
DImValue val(vd->type,
DtoBitCast(DtoNestedContext(loc, sd), DtoType(vd->type)));
DtoAssign(loc, &field, &val, TOKblit);
- } else {
- IF_LOG Logger::println("using default initializer");
- LOG_SCOPE
- DConstValue val(vd->type, IrAggr::getDefaultInitializer(vd));
- DtoAssign(loc, &field, &val, TOKblit);
}
+ offset += vd->type->size();
+
// Also zero out padding bytes counted as being part of the type in DMD
// but not in LLVM; e.g. real/x86_fp80.
int implicitPadding =
@@ -564,7 +567,7 @@ public:
\
errorOnIllegalArrayOp(e, e->e1, e->e2); \
\
- auto &PGO = gIR->funcGen().pgo; \
+ auto &PGO = gIR->funcGen().pgo; \
PGO.setCurrentStmt(e); \
\
result = Func(e->loc, e->type, toElem(e->e1), e->e2); \
@@ -605,7 +608,7 @@ public:
DtoAssign(e->loc, lhsLVal, assignedResult, TOKassign);
if (e->type->equals(lhsLVal->type))
- return lhsLVal;
+ return lhsLVal;
return new DLValue(e->type, DtoLVal(lhsLVal));
}
@@ -618,7 +621,7 @@ public:
\
errorOnIllegalArrayOp(e, e->e1, e->e2); \
\
- auto &PGO = gIR->funcGen().pgo; \
+ auto &PGO = gIR->funcGen().pgo; \
PGO.setCurrentStmt(e); \
\
result = binAssign<Func, useLValTypeForBinOp>(e); \
@@ -1413,7 +1416,7 @@ public:
// The real part of the complex number has already been updated, skip the
// store
if (!e1type->iscomplex()) {
- DtoStore(post, lval);
+ DtoStore(post, lval);
}
result = new DImValue(e->type, val);
}
diff --git a/ir/irtypeaggr.cpp b/ir/irtypeaggr.cpp
index 50f8092..7d14415 100644
--- a/ir/irtypeaggr.cpp
+++ b/ir/irtypeaggr.cpp
@@ -116,20 +116,22 @@ void AggrTypeBuilder::addAggregate(
// check for overlapping existing fields
bool overlaps = false;
- for (const auto vd : data) {
- if (!vd)
- continue;
-
- const size_t v_begin = vd->offset;
- const size_t v_end = v_begin + vd->type->size();
-
- if (v_begin < f_end && v_end > f_begin) {
- if (aliases == Aliases::AddToVarGEPIndices && v_begin == f_begin &&
- DtoMemType(vd->type) == DtoMemType(field->type)) {
- aliasPairs.push_back(std::make_pair(field, vd));
+ if (field->overlapped) {
+ for (const auto vd : data) {
+ if (!vd)
+ continue;
+
+ const size_t v_begin = vd->offset;
+ const size_t v_end = v_begin + vd->type->size();
+
+ if (v_begin < f_end && v_end > f_begin) {
+ if (aliases == Aliases::AddToVarGEPIndices && v_begin == f_begin &&
+ DtoMemType(vd->type) == DtoMemType(field->type)) {
+ aliasPairs.push_back(std::make_pair(field, vd));
+ }
+ overlaps = true;
+ break;
}
- overlaps = true;
- break;
}
}
diff --git a/tests/codegen/union.d b/tests/codegen/union.d
index 618d965..a812339 100644
--- a/tests/codegen/union.d
+++ b/tests/codegen/union.d
@@ -2,6 +2,7 @@
// structs with and without overlapping (union) fields.
// RUN: %ldc -c -output-ll -of=%t.ll %s && FileCheck %s < %t.ll
+// RUN: %ldc -run %s
struct S
{
@@ -18,7 +19,7 @@ struct S
__gshared S defaultS;
// CHECK-DAG: @_D5union9explicitSS5union1S = global %union.S { i8 3, [3 x i8] zeroinitializer, i32 56, [2 x i8] c"\00\01", i8 0, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }
-__gshared S explicitS = S(3, 56, [false, true], false /* implicit multidim */);
+__gshared S explicitS = { 3, 56, [false, true], false /* implicit multidim */ };
struct SWithUnion
@@ -28,32 +29,58 @@ struct SWithUnion
union
{
- struct { ubyte ub = 6; ushort us = 33; ulong ul_dummy = void; }
+ struct { ubyte ub = 6; ushort us = 33; ulong ul_dummy = void; ulong last = 123; }
struct { uint ui1; uint ui2 = 84; ulong ul = 666; }
- ubyte ub_direct;
}
+}
+// CHECK-DAG: %union.SWithUnion = type { i8, [3 x i8], %union.S, [4 x i8], i8, [1 x i8], i16, i32, i64, i64 }
+// CHECK-DAG: @_D5union10SWithUnion6__initZ = constant %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 6, [1 x i8] zeroinitializer, i16 33, i32 84, i64 666, i64 123 }
+
+// CHECK-DAG: @_D5union17defaultSWithUnionS5union10SWithUnion = global %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 6, [1 x i8] zeroinitializer, i16 33, i32 84, i64 666, i64 123 }
+__gshared SWithUnion defaultSWithUnion;
+
+// CHECK-DAG: @_D5union28explicitCompatibleSWithUnionS5union10SWithUnion = global %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 6, [1 x i8] zeroinitializer, i16 33, i32 84, i64 53, i64 123 }
+__gshared SWithUnion explicitCompatibleSWithUnion = { ul_dummy: 53 }; // ul_dummy is an alias for dominant ul
+
+// If a dominated union field is initialized and it isn't an alias for a dominant field,
+// the regular LL type cannot be used, and an anonymous one is used instead.
+// CHECK-DAG: @_D5union30explicitIncompatibleSWithUnionS5union10SWithUnion = global { i8, [3 x i8], %union.S, [4 x i8], i32, i32, i64, i64 } { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i32 23, i32 84, i64 666, i64 123 }
+__gshared SWithUnion explicitIncompatibleSWithUnion = { ui1: 23 }; // // ui1 dominated by ub and us
+
+
+void main()
+{
+ // test dynamic literals too
- this(ubyte ub)
{
- this.ub_direct = ub; // ub_direct is an alias for dominant ub
+ SWithUnion s = { 'y' };
+ assert(s.c == 'y');
+ assert(s.nested == S.init);
+ assert(s.ub == 6);
+ assert(s.us == 33);
+ assert(s.ui2 == 84);
+ assert(s.ul == 666);
+ assert(s.last == 123);
}
- this(uint ui1, uint ui2)
{
- this.ui1 = ui1; // dominated by ub and us
- this.ui2 = ui2;
+ SWithUnion s = { ul_dummy: 53 };
+ assert(s.c == char.init);
+ assert(s.nested == S.init);
+ assert(s.ub == 6);
+ assert(s.us == 33);
+ assert(s.ui2 == 84);
+ assert(s.ul_dummy == 53);
+ assert(s.last == 123);
}
-}
-// CHECK-DAG: %union.SWithUnion = type { i8, [3 x i8], %union.S, [4 x i8], i8, [1 x i8], i16, i32, i64 }
-// CHECK-DAG: @_D5union10SWithUnion6__initZ = constant %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 6, [1 x i8] zeroinitializer, i16 33, i32 84, i64 666 }
-
-// CHECK-DAG: @_D5union17defaultSWithUnionS5union10SWithUnion = global %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 6, [1 x i8] zeroinitializer, i16 33, i32 84, i64 666 }
-__gshared SWithUnion defaultSWithUnion;
-
-// CHECK-DAG: @_D5union28explicitCompatibleSWithUnionS5union10SWithUnion = global %union.SWithUnion { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i8 53, [1 x i8] zeroinitializer, i16 33, i32 84, i64 666 }
-__gshared SWithUnion explicitCompatibleSWithUnion = SWithUnion(53);
-// When using the 2nd constructor, a dominated union field (ui1) is initialized.
-// The regular LL type can thus not be used, an anonymous one is used instead.
-// CHECK-DAG: @_D5union30explicitIncompatibleSWithUnionS5union10SWithUnion = global { i8, [3 x i8], %union.S, [4 x i8], i32, i32, i64 } { i8 -1, [3 x i8] zeroinitializer, %union.S { i8 -1, [3 x i8] zeroinitializer, i32 0, [2 x i8] zeroinitializer, i8 1, [1 x [2 x i8]] {{\[}}[2 x i8] c"\FF\FF"], [3 x i8] zeroinitializer }, [4 x i8] zeroinitializer, i32 23, i32 256, i64 666 }
-__gshared SWithUnion explicitIncompatibleSWithUnion = SWithUnion(23, 256);
+ {
+ SWithUnion s = { ui1: 23 };
+ assert(s.c == char.init);
+ assert(s.nested == S.init);
+ assert(s.ui1 == 23);
+ assert(s.ui2 == 84);
+ assert(s.ul == 666);
+ assert(s.last == 123);
+ }
+}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-d/ldc.git
More information about the pkg-d-commits
mailing list