Skip to content

Commit

Permalink
codegen: on alignment incongruence, replace assert by jl_error
Browse files Browse the repository at this point in the history
It is possible that LLVM and Julia disagree on the offsets of struct
fields. This has been observed in JuliaLang#32414 in cases like

    Tuple{Float64, NTuple{8, VecElement{Float64}}}

where LLVM wants to align the `vec` on 64 bytes (adding 56 bytes
of padding after the first `Float64`) but Julia refusing to
align like that because it can't guarantee that on the heap:

https://github.com/JuliaLang/julia/blob/c7e9d9f8ade647582c6635c8c9a587f2360af404/src/datatype.c#L453

There is no easy solution this I can see, but since this can be
triggered from pure Julia, the least we can do is not abort the program.

For this reason, this commit replaces the assert by jl_error.
To be able to give a sort-of meaningful error message, we return
a sentinel value from `convert_struct_offset` and generate
an error message in the caller.
  • Loading branch information
tkluck committed Jan 3, 2020
1 parent c7e9d9f commit fbd7c96
Showing 1 changed file with 32 additions and 5 deletions.
37 changes: 32 additions & 5 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,10 @@ static unsigned convert_struct_offset(Type *lty, unsigned byte_offset)
const DataLayout &DL = jl_data_layout;
const StructLayout *SL = DL.getStructLayout(cast<StructType>(lty));
unsigned idx = SL->getElementContainingOffset(byte_offset);
assert(SL->getElementOffset(idx) == byte_offset);
if(SL->getElementOffset(idx) != byte_offset) {
// sentinel error value
return (unsigned)(-1);
}
return idx;
}

Expand Down Expand Up @@ -1692,9 +1695,15 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
else if (!jl_field_isptr(jt, idx) && jl_is_uniontype(jfty)) {
int fsz = jl_field_size(jt, idx) - 1;
unsigned ptindex = convert_struct_offset(ctx, T, byte_offset + fsz);
if(ptindex == (unsigned)(-1)) {
jl_errorf("Cannot generate accessor code because of incompatible alignment requirements");
}
AllocaInst *lv = NULL;
if (fsz > 0) {
unsigned st_idx = convert_struct_offset(ctx, T, byte_offset);
if(ptindex == (unsigned)(-1)) {
jl_errorf("Cannot generate accessor code because of incompatible alignment requirements");
}
IntegerType *ET = cast<IntegerType>(T->getStructElementType(st_idx));
unsigned align = (ET->getBitWidth() + 7) / 8;
lv = emit_static_alloca(ctx, ET);
Expand Down Expand Up @@ -1724,12 +1733,16 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
}
else {
unsigned st_idx;
if (isa<ArrayType>(T))
if (isa<ArrayType>(T)) {
st_idx = idx;
else if (isa<StructType>(T))
} else if (isa<StructType>(T)) {
st_idx = convert_struct_offset(ctx, T, byte_offset);
else
if(st_idx == (unsigned)(-1)) {
jl_errorf("Cannot generate accessor code because of incompatible alignment requirements");
}
} else {
llvm_unreachable("encountered incompatible type for a struct");
}
fldv = ctx.builder.CreateExtractValue(obj, makeArrayRef(st_idx));
}
if (maybe_null && jl_field_isptr(jt, idx))
Expand Down Expand Up @@ -2107,8 +2120,12 @@ static jl_value_t *static_constant_instance(Constant *constant, jl_value_t *jt)
return NULL; // TODO: handle this?
}
unsigned llvm_idx = i;
if (i > 0 && isa<StructType>(constant->getType()))
if (i > 0 && isa<StructType>(constant->getType())) {
llvm_idx = convert_struct_offset(constant->getType(), jl_field_offset(jst, i));
if(llvm_idx == (unsigned)(-1)) {
jl_errorf("Cannot create an static instance of %s because of incompatible alignment requirements", jl_symbol_name(jst->name->name));
}
}
Constant *fld = constant->getAggregateElement(llvm_idx);
flds[i] = static_constant_instance(fld, ft);
if (flds[i] == NULL) {
Expand Down Expand Up @@ -2617,6 +2634,10 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
Value *dest = NULL;
unsigned offs = jl_field_offset(sty, i);
unsigned llvm_idx = (i > 0 && isa<StructType>(lt)) ? convert_struct_offset(ctx, lt, offs) : i;
if(llvm_idx == (unsigned)(-1)) {
jl_errorf("Cannot create an instance of %s because of incompatible alignment requirements", jl_symbol_name(sty->name->name));
}

if (!init_as_value) {
// avoid unboxing the argument explicitly
// and use memcpy instead
Expand All @@ -2640,6 +2661,9 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
// then load it and combine with the tindex.
// But more efficient to just store it directly.
unsigned ptindex = convert_struct_offset(ctx, lt, offs + fsz);
if(ptindex == (unsigned)(-1)) {
jl_errorf("Cannot create an instance of %s because of incompatible alignment requirements", jl_symbol_name(sty->name->name));
}
if (fsz > 0 && !fval_info.isghost) {
Type *ET = IntegerType::get(jl_LLVMContext, 8 * al);
assert(lt->getStructElementType(llvm_idx) == ET);
Expand Down Expand Up @@ -2694,6 +2718,9 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
unsigned offs = jl_field_offset(sty, i);
int fsz = jl_field_size(sty, i) - 1;
unsigned llvm_idx = convert_struct_offset(ctx, cast<StructType>(lt), offs + fsz);
if(llvm_idx == (unsigned)(-1)) {
jl_errorf("Cannot create an instance of %s because of incompatible alignment requirements", jl_symbol_name(sty->name->name));
}
if (init_as_value)
strct = ctx.builder.CreateInsertValue(strct, ConstantInt::get(T_int8, 0), makeArrayRef(llvm_idx));
else
Expand Down

0 comments on commit fbd7c96

Please sign in to comment.