ccall: handle Union appearing as a field-type without crashing (#46787)

We disallow union as the direct type, so perhaps we should disallow it
as a field-type also, but since we do allow references in those cases
typically, we will allow this also.

Also DRY the emit_global code, since it had bit-rotted relative to the
usual code path through emit_globalref (and apparently could still be
run though for handling the first argument to cfunction).

Fix #46786

Author: vtjnash

src/abi_aarch64.cpp

@@ -43,9 +43,11 @@ Type *get_llvm_vectype(jl_datatype_t *dt, LLVMContext &ctx) const
     // the homogeneity check.
     jl_datatype_t *ft0 = (jl_datatype_t*)jl_field_type(dt, 0);
     // `ft0` should be a `VecElement` type and the true element type
-    // should be a primitive type
-    if (ft0->name != jl_vecelement_typename ||
-        ((jl_datatype_t*)jl_field_type(ft0, 0))->layout->nfields)
+    // should be a primitive type (nfields == 0)
+    if (!jl_is_datatype(ft0) || ft0->name != jl_vecelement_typename)
+        return nullptr;
+    jl_datatype_t *ft00 = (jl_datatype_t*)jl_field_type(ft0, 0);
+    if (!jl_is_datatype(ft00) || ft00->layout->nfields)
         return nullptr;
     for (size_t i = 1; i < nfields; i++) {
         if (jl_field_type(dt, i) != (jl_value_t*)ft0) {
@@ -120,15 +122,17 @@ bool isHFAorHVA(jl_datatype_t *dt, size_t dsz, size_t &nele, ElementType &ele, L
         // For composite types, find the first non zero sized member
         size_t i;
         size_t fieldsz;
-        for (i = 0;i < nfields;i++) {
+        for (i = 0; i < nfields; i++) {
             if ((fieldsz = jl_field_size(dt, i))) {
                 break;
             }
         }
         assert(i < nfields);
-        // If there's only one non zero sized member, try again on this member
+        // If there's only one non-zero sized member, try again on this member
         if (fieldsz == dsz) {
             dt = (jl_datatype_t*)jl_field_type(dt, i);
+            if (!jl_is_datatype(dt))
+                return false;
             continue;
         }
         if (Type *vectype = get_llvm_vectype(dt, ctx)) {
@@ -140,11 +144,13 @@ bool isHFAorHVA(jl_datatype_t *dt, size_t dsz, size_t &nele, ElementType &ele, L
             return true;
         }
         // Otherwise, process each members
-        for (;i < nfields;i++) {
+        for (; i < nfields; i++) {
             size_t fieldsz = jl_field_size(dt, i);
             if (fieldsz == 0)
                 continue;
             jl_datatype_t *fieldtype = (jl_datatype_t*)jl_field_type(dt, i);
+            if (!jl_is_datatype(dt))
+                return false;
             // Check element count.
             // This needs to be done after the zero size member check
             if (nele > 3 || !isHFAorHVA(fieldtype, fieldsz, nele, ele, ctx)) {

src/abi_arm.cpp

@@ -91,6 +91,8 @@ size_t isLegalHA(jl_datatype_t *dt, Type *&base, LLVMContext &ctx) const
         size_t parent_members = jl_datatype_nfields(dt);
         for (size_t i = 0; i < parent_members; ++i) {
             jl_datatype_t *fdt = (jl_datatype_t*)jl_field_type(dt,i);
+            if (!jl_is_datatype(fdt))
+                return 0;
 
             Type *T = isLegalHAType(fdt, ctx);
             if (T)

src/abi_ppc64le.cpp

@@ -44,6 +44,9 @@ struct ABI_PPC64leLayout : AbiLayout {
 // count the homogeneous floating aggregate size (saturating at max count of 8)
 unsigned isHFA(jl_datatype_t *ty, jl_datatype_t **ty0, bool *hva) const
 {
+    if (jl_datatype_size(ty) > 128 || ty->layout->npointers || ty->layout->haspadding)
+        return 9;
+
     size_t i, l = ty->layout->nfields;
     // handle homogeneous float aggregates
     if (l == 0) {
@@ -52,7 +55,7 @@ unsigned isHFA(jl_datatype_t *ty, jl_datatype_t **ty0, bool *hva) const
         *hva = false;
         if (*ty0 == NULL)
             *ty0 = ty;
-        else if (*hva || ty->size != (*ty0)->size)
+        else if (*hva || jl_datatype_size(ty) != jl_datatype_size(*ty0))
             return 9;
         return 1;
     }
@@ -69,7 +72,7 @@ unsigned isHFA(jl_datatype_t *ty, jl_datatype_t **ty0, bool *hva) const
         *hva = true;
         if (*ty0 == NULL)
             *ty0 = ty;
-        else if (!*hva || ty->size != (*ty0)->size)
+        else if (!*hva || jl_datatype_size(ty) != jl_datatype_size(*ty0))
             return 9;
         for (i = 1; i < l; i++) {
             jl_datatype_t *fld = (jl_datatype_t*)jl_field_type(ty, i);

src/abi_x86_64.cpp

@@ -153,6 +153,10 @@ void classifyType(Classification& accum, jl_datatype_t *dt, uint64_t offset) con
             jl_value_t *ty = jl_field_type(dt, i);
             if (jl_field_isptr(dt, i))
                 ty = (jl_value_t*)jl_voidpointer_type;
+            else if (!jl_is_datatype(ty)) { // inline union
+                accum.addField(offset, Memory);
+                continue;
+            }
             classifyType(accum, (jl_datatype_t*)ty, offset + jl_field_offset(dt, i));
         }
     }

src/codegen.cpp

@@ -4258,26 +4258,6 @@ static jl_cgval_t emit_sparam(jl_codectx_t &ctx, size_t i)
     return mark_julia_type(ctx, sp, true, jl_any_type);
 }
 
-static jl_cgval_t emit_global(jl_codectx_t &ctx, jl_sym_t *sym)
-{
-    jl_binding_t *jbp = NULL;
-    Value *bp = global_binding_pointer(ctx, ctx.module, sym, &jbp, false);
-    if (bp == NULL)
-        return jl_cgval_t();
-    if (jbp && jbp->value != NULL) {
-        if (jbp->constp)
-            return mark_julia_const(ctx, jbp->value);
-        // double-check that a global variable is actually defined. this
-        // can be a problem in parallel when a definition is missing on
-        // one machine.
-        LoadInst *v = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, bp, Align(sizeof(void*)));
-        v->setOrdering(AtomicOrdering::Unordered);
-        tbaa_decorate(ctx.tbaa().tbaa_binding, v);
-        return mark_julia_type(ctx, v, true, jl_any_type);
-    }
-    return emit_checked_var(ctx, bp, sym, false, ctx.tbaa().tbaa_binding);
-}
-
 static jl_cgval_t emit_isdefined(jl_codectx_t &ctx, jl_value_t *sym)
 {
     Value *isnull = NULL;
@@ -4928,7 +4908,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
 {
     if (jl_is_symbol(expr)) {
         jl_sym_t *sym = (jl_sym_t*)expr;
-        return emit_global(ctx, sym);
+        return emit_globalref(ctx, ctx.module, sym, AtomicOrdering::Unordered);
     }
     if (jl_is_slot(expr) || jl_is_argument(expr)) {
         return emit_local(ctx, expr);

test/ccall.jl

@@ -1590,6 +1590,32 @@ function caller22734(ptr)
 end
 @test caller22734(ptr22734) === 32.0
 
+# issue #46786 -- non-isbitstypes passed "by-value"
+struct NonBits46786
+    x::Union{Int16,NTuple{3,UInt8}}
+end
+let ptr = @cfunction(identity, NonBits46786, (NonBits46786,))
+    obj1 = NonBits46786((0x01,0x02,0x03))
+    obj2 = ccall(ptr, NonBits46786, (NonBits46786,), obj1)
+    @test obj1 === obj2
+end
+let ptr = @cfunction(identity, Base.RefValue{NonBits46786}, (Base.RefValue{NonBits46786},))
+    obj1 = Base.RefValue(NonBits46786((0x01,0x02,0x03)))
+    obj2 = ccall(ptr, Base.RefValue{NonBits46786}, (Base.RefValue{NonBits46786},), obj1)
+    @test obj1 !== obj2
+    @test obj1.x === obj2.x
+end
+
+mutable struct MutNonBits46786
+    x::Union{Int16,NTuple{3,UInt8}}
+end
+let ptr = @cfunction(identity, MutNonBits46786, (MutNonBits46786,))
+    obj1 = MutNonBits46786((0x01,0x02,0x03))
+    obj2 = ccall(ptr, MutNonBits46786, (MutNonBits46786,), obj1)
+    @test obj1 !== obj2
+    @test obj1.x === obj2.x
+end
+
 # 26297#issuecomment-371165725
 #   test that the first argument to cglobal is recognized as a tuple literal even through
 #   macro expansion