Skip to content

Commit 3894699

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 result 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 72854dc commit 3894699

File tree

5 files changed

+229
-110
lines changed

5 files changed

+229
-110
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: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,10 +1064,20 @@ static void raise_exception_unless(jl_codectx_t &ctx, Value *cond, Value *exc)
10641064
raise_exception(ctx, exc, passBB);
10651065
}
10661066

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

@@ -1378,9 +1388,12 @@ Value *extract_first_ptr(jl_codectx_t &ctx, Value *V)
13781388
return ctx.builder.CreateExtractValue(V, path);
13791389
}
13801390

1391+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1392+
// store the pointer to be checked in `*nullcheck` instead of checking it
13811393
static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, jl_value_t *jltype,
13821394
MDNode *tbaa, MDNode *aliasscope,
1383-
bool maybe_null_if_boxed = true, unsigned alignment = 0)
1395+
bool maybe_null_if_boxed = true, unsigned alignment = 0,
1396+
Value **nullcheck = nullptr)
13841397
{
13851398
bool isboxed;
13861399
Type *elty = julia_type_to_llvm(ctx, jltype, &isboxed);
@@ -1416,7 +1429,7 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
14161429
if (maybe_null_if_boxed) {
14171430
Value *first_ptr = isboxed ? load : extract_first_ptr(ctx, load);
14181431
if (first_ptr)
1419-
null_pointer_check(ctx, first_ptr);
1432+
null_pointer_check(ctx, first_ptr, nullcheck);
14201433
}
14211434
//}
14221435
if (jltype == (jl_value_t*)jl_bool_type) { // "freeze" undef memory to a valid value
@@ -1580,7 +1593,9 @@ static void emit_memcpy(jl_codectx_t &ctx, Value *dst, MDNode *tbaa_dst, const j
15801593

15811594

15821595

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

15851600
static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
15861601
jl_cgval_t *ret, jl_cgval_t strct,
@@ -1711,7 +1726,11 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
17111726
return false;
17121727
}
17131728

1714-
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt)
1729+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1730+
// store the pointer to be checked in `*nullcheck` instead of checking it
1731+
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
1732+
unsigned idx, jl_datatype_t *jt,
1733+
Value **nullcheck)
17151734
{
17161735
jl_value_t *jfty = jl_field_type(jt, idx);
17171736
if (jfty == jl_bottom_type) {
@@ -1759,7 +1778,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17591778
maybe_mark_load_dereferenceable(Load, maybe_null, jl_field_type(jt, idx));
17601779
Value *fldv = tbaa_decorate(tbaa, Load);
17611780
if (maybe_null)
1762-
null_pointer_check(ctx, fldv);
1781+
null_pointer_check(ctx, fldv, nullcheck);
17631782
return mark_julia_type(ctx, fldv, true, jfty);
17641783
}
17651784
else if (jl_is_uniontype(jfty)) {
@@ -1796,7 +1815,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17961815
return mark_julia_slot(addr, jfty, NULL, tbaa);
17971816
}
17981817
unsigned align = jl_field_align(jt, idx);
1799-
return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align);
1818+
return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align, nullcheck);
18001819
}
18011820
else if (isa<UndefValue>(strct.V)) {
18021821
return jl_cgval_t();
@@ -1858,7 +1877,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
18581877
if (maybe_null) {
18591878
Value *first_ptr = jl_field_isptr(jt, idx) ? fldv : extract_first_ptr(ctx, fldv);
18601879
if (first_ptr)
1861-
null_pointer_check(ctx, first_ptr);
1880+
null_pointer_check(ctx, first_ptr, nullcheck);
18621881
}
18631882
return mark_julia_type(ctx, fldv, jl_field_isptr(jt, idx), jfty);
18641883
}

0 commit comments

Comments
 (0)