Skip to content

Commit 94af03e

Browse files
yuyichaovchuravy
authored andcommitted
Fix === and objectid of object with undef inline immutable field (#37557)
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 (cherry picked from commit e84fec4)
1 parent d179f48 commit 94af03e

File tree

5 files changed

+253
-114
lines changed

5 files changed

+253
-114
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;
@@ -316,7 +326,16 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
316326
fieldtype = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)fieldtype, sel);
317327
}
318328
assert(jl_is_datatype(fieldtype) && !fieldtype->abstract && !fieldtype->mutabl);
319-
u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
329+
int32_t first_ptr = fieldtype->layout->first_ptr;
330+
if (first_ptr >= 0 && ((jl_value_t**)vo)[first_ptr] == NULL) {
331+
// If the field is a inline immutable that can be can be undef
332+
// we need to check to check for undef first since undef struct
333+
// may have fields that are different but should still be treated as equal.
334+
u = 0;
335+
}
336+
else {
337+
u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
338+
}
320339
}
321340
h = bitmix(h, u);
322341
}

src/cgutils.cpp

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,12 @@ static bool for_each_uniontype_small(
833833
return false;
834834
}
835835

836+
static bool is_uniontype_allunboxed(jl_value_t *typ)
837+
{
838+
unsigned counter = 0;
839+
return for_each_uniontype_small([&](unsigned, jl_datatype_t*) {}, typ, counter);
840+
}
841+
836842
static Value *emit_typeof_boxed(jl_codectx_t &ctx, const jl_cgval_t &p);
837843

838844
static unsigned get_box_tindex(jl_datatype_t *jt, jl_value_t *ut)
@@ -915,14 +921,10 @@ static jl_cgval_t emit_typeof(jl_codectx_t &ctx, const jl_cgval_t &p)
915921
}
916922
if (p.TIndex) {
917923
Value *tindex = ctx.builder.CreateAnd(p.TIndex, ConstantInt::get(T_int8, 0x7f));
918-
unsigned counter = 0;
919-
bool allunboxed = for_each_uniontype_small(
920-
[&](unsigned idx, jl_datatype_t *jt) { },
921-
p.typ,
922-
counter);
924+
bool allunboxed = is_uniontype_allunboxed(p.typ);
923925
Value *datatype_or_p = (imaging_mode ? Constant::getNullValue(T_ppjlvalue) :
924926
Constant::getNullValue(T_prjlvalue));
925-
counter = 0;
927+
unsigned counter = 0;
926928
for_each_uniontype_small(
927929
[&](unsigned idx, jl_datatype_t *jt) {
928930
Value *cmp = ctx.builder.CreateICmpEQ(tindex, ConstantInt::get(T_int8, idx));
@@ -1145,10 +1147,20 @@ static void raise_exception_unless(jl_codectx_t &ctx, Value *cond, Value *exc)
11451147
raise_exception(ctx, exc, passBB);
11461148
}
11471149

1148-
static void null_pointer_check(jl_codectx_t &ctx, Value *v)
1150+
static Value *null_pointer_cmp(jl_codectx_t &ctx, Value *v)
11491151
{
1150-
raise_exception_unless(ctx,
1151-
ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType())),
1152+
return ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType()));
1153+
}
1154+
1155+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1156+
// store the pointer to be checked in `*nullcheck` instead of checking it
1157+
static void null_pointer_check(jl_codectx_t &ctx, Value *v, Value **nullcheck = nullptr)
1158+
{
1159+
if (nullcheck) {
1160+
*nullcheck = v;
1161+
return;
1162+
}
1163+
raise_exception_unless(ctx, null_pointer_cmp(ctx, v),
11521164
literal_pointer_val(ctx, jl_undefref_exception));
11531165
}
11541166

@@ -1382,9 +1394,12 @@ Value *extract_first_ptr(jl_codectx_t &ctx, Value *V)
13821394
return ctx.builder.CreateExtractValue(V, path);
13831395
}
13841396

1397+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1398+
// store the pointer to be checked in `*nullcheck` instead of checking it
13851399
static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, jl_value_t *jltype,
13861400
MDNode *tbaa, MDNode *aliasscope,
1387-
bool maybe_null_if_boxed = true, unsigned alignment = 0)
1401+
bool maybe_null_if_boxed = true, unsigned alignment = 0,
1402+
Value **nullcheck = nullptr)
13881403
{
13891404
bool isboxed;
13901405
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
@@ -1575,7 +1590,9 @@ static void emit_memcpy(jl_codectx_t &ctx, Value *dst, MDNode *tbaa_dst, const j
15751590

15761591

15771592

1578-
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt);
1593+
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
1594+
unsigned idx, jl_datatype_t *jt,
1595+
Value **nullcheck = nullptr);
15791596

15801597
static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
15811598
jl_cgval_t *ret, jl_cgval_t strct,
@@ -1706,7 +1723,11 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
17061723
return false;
17071724
}
17081725

1709-
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt)
1726+
// If `nullcheck` is not NULL and a pointer NULL check is necessary
1727+
// store the pointer to be checked in `*nullcheck` instead of checking it
1728+
static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
1729+
unsigned idx, jl_datatype_t *jt,
1730+
Value **nullcheck)
17101731
{
17111732
jl_value_t *jfty = jl_field_type(jt, idx);
17121733
if (jfty == jl_bottom_type) {
@@ -1751,7 +1772,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17511772
maybe_null, jl_field_type(jt, idx));
17521773
Value *fldv = tbaa_decorate(strct.tbaa, Load);
17531774
if (maybe_null)
1754-
null_pointer_check(ctx, fldv);
1775+
null_pointer_check(ctx, fldv, nullcheck);
17551776
return mark_julia_type(ctx, fldv, true, jfty);
17561777
}
17571778
else if (jl_is_uniontype(jfty)) {
@@ -1788,7 +1809,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
17881809
return mark_julia_slot(addr, jfty, NULL, strct.tbaa);
17891810
}
17901811
unsigned align = jl_field_align(jt, idx);
1791-
return typed_load(ctx, addr, NULL, jfty, strct.tbaa, nullptr, maybe_null, align);
1812+
return typed_load(ctx, addr, NULL, jfty, strct.tbaa, nullptr, maybe_null, align, nullcheck);
17921813
}
17931814
else if (isa<UndefValue>(strct.V)) {
17941815
return jl_cgval_t();
@@ -1850,7 +1871,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
18501871
if (maybe_null) {
18511872
Value *first_ptr = jl_field_isptr(jt, idx) ? fldv : extract_first_ptr(ctx, fldv);
18521873
if (first_ptr)
1853-
null_pointer_check(ctx, first_ptr);
1874+
null_pointer_check(ctx, first_ptr, nullcheck);
18541875
}
18551876
return mark_julia_type(ctx, fldv, jl_field_isptr(jt, idx), jfty);
18561877
}

0 commit comments

Comments
 (0)