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

src/builtins.c

@@ -102,6 +102,16 @@ static int NOINLINE compare_fields(jl_value_t *a, jl_value_t *b, jl_datatype_t *
  return 0;
  ft = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)ft, asel);
  }
+ else if (ft->layout->first_ptr >= 0) {
+ // If the field is a inline immutable that can be can be undef
+ // we need to check to check for undef first since undef struct
+ // may have fields that are different but should still be treated as equal.
+ jl_value_t *ptra = ((jl_value_t**)ao)[ft->layout->first_ptr];
+ jl_value_t *ptrb = ((jl_value_t**)bo)[ft->layout->first_ptr];
+ if (ptra == NULL && ptrb == NULL) {
+ return 1;
+ }
+ }
  if (!ft->layout->haspadding) {
  if (!bits_equal(ao, bo, ft->size))
  return 0;
@@ -314,7 +324,16 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
  fieldtype = (jl_datatype_t*)jl_nth_union_component((jl_value_t*)fieldtype, sel);
  }
  assert(jl_is_datatype(fieldtype) && !fieldtype->abstract && !fieldtype->mutabl);
- u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
+ int32_t first_ptr = fieldtype->layout->first_ptr;
+ if (first_ptr >= 0 && ((jl_value_t**)vo)[first_ptr] == NULL) {
+ // If the field is a inline immutable that can be can be undef
+ // we need to check to check for undef first since undef struct
+ // may have fields that are different but should still be treated as equal.
+ u = 0;
+ }
+ else {
+ u = immut_id_(fieldtype, (jl_value_t*)vo, 0);
+ }
  }
  h = bitmix(h, u);
  }

src/cgutils.cpp

@@ -765,6 +765,12 @@ static bool for_each_uniontype_small(
  return false;
 }
 
+static bool is_uniontype_allunboxed(jl_value_t *typ)
+{
+ unsigned counter = 0;
+ return for_each_uniontype_small([&](unsigned, jl_datatype_t*) {}, typ, counter);
+}
+
 static Value *emit_typeof_boxed(jl_codectx_t &ctx, const jl_cgval_t &p);
 
 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)
  }
  if (p.TIndex) {
  Value *tindex = ctx.builder.CreateAnd(p.TIndex, ConstantInt::get(T_int8, 0x7f));
- unsigned counter = 0;
- bool allunboxed = for_each_uniontype_small(
- [&](unsigned idx, jl_datatype_t *jt) { },
- p.typ,
- counter);
+ bool allunboxed = is_uniontype_allunboxed(p.typ);
  Value *datatype_or_p = imaging_mode ? Constant::getNullValue(T_ppjlvalue) : V_rnull;
- counter = 0;
+ unsigned counter = 0;
  for_each_uniontype_small(
  [&](unsigned idx, jl_datatype_t *jt) {
  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)
  raise_exception(ctx, exc, passBB);
 }
 
-static void null_pointer_check(jl_codectx_t &ctx, Value *v)
+static Value *null_pointer_cmp(jl_codectx_t &ctx, Value *v)
 {
- raise_exception_unless(ctx,
- ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType())),
+ return ctx.builder.CreateICmpNE(v, Constant::getNullValue(v->getType()));
+}
+
+// If `nullcheck` is not NULL and a pointer NULL check is necessary
+// store the pointer to be checked in `*nullcheck` instead of checking it
+static void null_pointer_check(jl_codectx_t &ctx, Value *v, Value **nullcheck = nullptr)
+{
+ if (nullcheck) {
+ *nullcheck = v;
+ return;
+ }
+ raise_exception_unless(ctx, null_pointer_cmp(ctx, v),
  literal_pointer_val(ctx, jl_undefref_exception));
 }
 
@@ -1378,9 +1390,12 @@ Value *extract_first_ptr(jl_codectx_t &ctx, Value *V)
  return ctx.builder.CreateExtractValue(V, path);
 }
 
+// If `nullcheck` is not NULL and a pointer NULL check is necessary
+// store the pointer to be checked in `*nullcheck` instead of checking it
 static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, jl_value_t *jltype,
  MDNode *tbaa, MDNode *aliasscope,
- bool maybe_null_if_boxed = true, unsigned alignment = 0)
+ bool maybe_null_if_boxed = true, unsigned alignment = 0,
+ Value **nullcheck = nullptr)
 {
  bool isboxed;
  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
  if (maybe_null_if_boxed) {
  Value *first_ptr = isboxed ? load : extract_first_ptr(ctx, load);
  if (first_ptr)
- null_pointer_check(ctx, first_ptr);
+ null_pointer_check(ctx, first_ptr, nullcheck);
  }
  //}
  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
 
 
 
-static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt);
+static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
+ unsigned idx, jl_datatype_t *jt,
+ Value **nullcheck = nullptr);
 
 static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
  jl_cgval_t *ret, jl_cgval_t strct,
@@ -1711,7 +1728,11 @@ static bool emit_getfield_unknownidx(jl_codectx_t &ctx,
  return false;
 }
 
-static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct, unsigned idx, jl_datatype_t *jt)
+// If `nullcheck` is not NULL and a pointer NULL check is necessary
+// store the pointer to be checked in `*nullcheck` instead of checking it
+static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &strct,
+ unsigned idx, jl_datatype_t *jt,
+ Value **nullcheck)
 {
  jl_value_t *jfty = jl_field_type(jt, idx);
  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
  maybe_mark_load_dereferenceable(Load, maybe_null, jl_field_type(jt, idx));
  Value *fldv = tbaa_decorate(tbaa, Load);
  if (maybe_null)
- null_pointer_check(ctx, fldv);
+ null_pointer_check(ctx, fldv, nullcheck);
  return mark_julia_type(ctx, fldv, true, jfty);
  }
  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
  return mark_julia_slot(addr, jfty, NULL, tbaa);
  }
  unsigned align = jl_field_align(jt, idx);
- return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align);
+ return typed_load(ctx, addr, NULL, jfty, tbaa, nullptr, maybe_null, align, nullcheck);
  }
  else if (isa<UndefValue>(strct.V)) {
  return jl_cgval_t();
@@ -1858,7 +1879,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
  if (maybe_null) {
  Value *first_ptr = jl_field_isptr(jt, idx) ? fldv : extract_first_ptr(ctx, fldv);
  if (first_ptr)
- null_pointer_check(ctx, first_ptr);
+ null_pointer_check(ctx, first_ptr, nullcheck);
  }
  return mark_julia_type(ctx, fldv, jl_field_isptr(jt, idx), jfty);
  }