Skip to content

Commit ba2f681

Browse files
committed
Fix === and objectid of object with undef inline immutable field
An undef field should always be treated equal to another undef field of the same type since there's no other way for the user to tell the difference between these. These could previously cause inconsistent comparison results or crashes. * Mark these types as `haspadding` so that they'll not hit the `memcmp` fast path. * Make sure `jl_egal` and `jl_object_id_` doesn't read bits fields in undef inline immutable field * Use `emit_getfield_knownidx` in `emit_bits_compare` so that the check can be done more easily Handle union fields of the same type in `emit_f_isa` to avoid regression. * Allow input to `emit_f_isa` to be NULL and lazily emit NULL check if necessary
1 parent a0a68a5 commit ba2f681

File tree

5 files changed

+255
-118
lines changed

5 files changed

+255
-118
lines changed

src/builtins.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,16 @@ static int NOINLINE compare_fields(jl_value_t *a, jl_value_t *b, jl_datatype_t *
102102
return 0;
103103
ft = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)ft, asel);
104104
}
105+
else if (ft->layout->first_ptr >= 0) {
106+
// If the field is a inline immutable that can be can be undef
107+
// we need to check to check for undef first since undef struct
108+
// may have fields that are different but should still be treated as equal.
109+
jl_value_t *ptra = ((jl_value_t**)ao)[ft->layout->first_ptr];
110+
jl_value_t *ptrb = ((jl_value_t**)bo)[ft->layout->first_ptr];
111+
if (ptra == NULL && ptrb == NULL) {
112+
return 1;
113+
}
114+
}
105115
if (!ft->layout->haspadding) {
106116
if (!bits_equal(ao, bo, ft->size))
107117
return 0;
@@ -314,7 +324,16 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
314324
fieldtype = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)fieldtype, sel);
315325
}
316326
assert(jl_is_datatype(fieldtype) && !fieldtype->abstract && !fieldtype->mutabl);
317-
u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
327+
int32_t first_ptr = fieldtype->layout->first_ptr;
328+
if (first_ptr >= 0 && ((jl_value_t**)vo)[first_ptr] == NULL) {
329+
// If the field is a inline immutable that can be can be undef
330+
// we need to check to check for undef first since undef struct
331+
// may have fields that are different but should still be treated as equal.
332+
u = 0;
333+
}
334+
else {
335+
u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
336+
}
318337
}
319338
h = bitmix(h, u);
320339
}

src/cgutils.cpp

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,12 @@ static bool for_each_uniontype_small(
765765
return false;
766766
}
767767

768+
static bool is_uniontype_allunboxed(jl_value_t *typ)
769+
{
770+
unsigned counter = 0;
771+
return for_each_uniontype_small([&](unsigned, jl_datatype_t*) {}, typ, counter);
772+
}
773+
768774
static Value *emit_typeof_boxed(jl_codectx_t &ctx, const jl_cgval_t &p);
769775

770776
static unsigned get_box_tindex(jl_datatype_t *jt, jl_value_t *ut)
@@ -841,13 +847,9 @@ static jl_cgval_t emit_typeof(jl_codectx_t &ctx, const jl_cgval_t &p)
841847
}
842848
if (p.TIndex) {
843849
Value *tindex = ctx.builder.CreateAnd(p.TIndex, ConstantInt::get(T_int8, 0x7f));
844-
unsigned counter = 0;
845-
bool allunboxed = for_each_uniontype_small(
846-
[&](unsigned idx, jl_datatype_t *jt) { },
847-
p.typ,
848-
counter);
850+
bool allunboxed = is_uniontype_allunboxed(p.typ);
849851
Value *datatype_or_p = imaging_mode ? Constant::getNullValue(T_ppjlvalue) : V_rnull;
850-
counter = 0;
852+
unsigned counter = 0;
851853
for_each_uniontype_small(
852854
[&](unsigned idx, jl_datatype_t *jt) {
853855
Value *cmp = ctx.builder.CreateICmpEQ(tindex, ConstantInt::get(T_int8, idx));
@@ -1064,10 +1066,20 @@ static void raise_exception_unless(jl_codectx_t &ctx, Value *cond, Value *exc)
10641066
raise_exception(ctx, exc, passBB);
10651067
}
10661068

1067-
static void null_pointer_check(jl_codectx_t &ctx, Value *v)
1069+
static Value *null_pointer_cmp(jl_codectx_t &ctx, Value *v)
10681070
{
1069-
raise_exception_unless(ctx,
1070-
ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType())),
1071+
return ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType()));
1072+
}
1073+
1074+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1075+
// store the pointer to be checked in `*nullcheck` instead of checking it
1076+
static void null_pointer_check(jl_codectx_t &ctx, Value *v, Value **nullcheck = nullptr)
1077+
{
1078+
if (nullcheck) {
1079+
*nullcheck = v;
1080+
return;
1081+
}
1082+
raise_exception_unless(ctx, null_pointer_cmp(ctx, v),
10711083
literal_pointer_val(ctx, jl_undefref_exception));
10721084
}
10731085

@@ -1378,9 +1390,12 @@ Value *extract_first_ptr(jl_codectx_t &ctx, Value *V)
13781390
return ctx.builder.CreateExtractValue(V, path);
13791391
}
13801392

1393+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1394+
// store the pointer to be checked in `*nullcheck` instead of checking it
13811395
static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, jl_value_t *jltype,
13821396
MDNode *tbaa, MDNode *aliasscope,
1383-
bool maybe_null_if_boxed = true, unsigned alignment = 0)
1397+
bool maybe_null_if_boxed = true, unsigned alignment = 0,
1398+
Value **nullcheck = nullptr)
13841399
{
13851400
bool isboxed;
13861401
Type *elty = julia_type_to_llvm(ctx, jltype, &isboxed);
@@ -1416,7 +1431,7 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
14161431
if (maybe_null_if_boxed) {
14171432
Value *first_ptr = isboxed ? load : extract_first_ptr(ctx, load);
14181433
if (first_ptr)
1419-
null_pointer_check(ctx, first_ptr);
1434+
null_pointer_check(ctx, first_ptr, nullcheck);
14201435
}
14211436
//}
14221437
if (jltype == (jl_value_t*)jl_bool_type) { // "freeze" undef memory to a valid value
@@ -1580,7 +1595,9 @@ static void emit_memcpy(jl_codectx_t &ctx, Value *dst, MDNode *tbaa_dst, const j
15801595

15811596

15821597

1583-
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt);
1598+
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
1599+
unsigned idx, jl_datatype_t *jt,
1600+
Value **nullcheck = nullptr);
15841601

15851602
static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
15861603
jl_cgval_t *ret, jl_cgval_t strct,
@@ -1711,7 +1728,11 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
17111728
return false;
17121729
}
17131730

1714-
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt)
1731+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1732+
// store the pointer to be checked in `*nullcheck` instead of checking it
1733+
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
1734+
unsigned idx, jl_datatype_t *jt,
1735+
Value **nullcheck)
17151736
{
17161737
jl_value_t *jfty = jl_field_type(jt, idx);
17171738
if (jfty == jl_bottom_type) {
@@ -1759,7 +1780,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17591780
maybe_mark_load_dereferenceable(Load, maybe_null, jl_field_type(jt, idx));
17601781
Value *fldv = tbaa_decorate(tbaa, Load);
17611782
if (maybe_null)
1762-
null_pointer_check(ctx, fldv);
1783+
null_pointer_check(ctx, fldv, nullcheck);
17631784
return mark_julia_type(ctx, fldv, true, jfty);
17641785
}
17651786
else if (jl_is_uniontype(jfty)) {
@@ -1796,7 +1817,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17961817
return mark_julia_slot(addr, jfty, NULL, tbaa);
17971818
}
17981819
unsigned align = jl_field_align(jt, idx);
1799-
return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align);
1820+
return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align, nullcheck);
18001821
}
18011822
else if (isa<UndefValue>(strct.V)) {
18021823
return jl_cgval_t();
@@ -1858,7 +1879,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
18581879
if (maybe_null) {
18591880
Value *first_ptr = jl_field_isptr(jt, idx) ? fldv : extract_first_ptr(ctx, fldv);
18601881
if (first_ptr)
1861-
null_pointer_check(ctx, first_ptr);
1882+
null_pointer_check(ctx, first_ptr, nullcheck);
18621883
}
18631884
return mark_julia_type(ctx, fldv, jl_field_isptr(jt, idx), jfty);
18641885
}

0 commit comments

Comments
 (0)