Skip to content

Commit

Permalink
Fix mismatched GC frame mgmt in llvmcall codegen (#46335)
Browse files Browse the repository at this point in the history
Unfortunately clang-sa is still disabled on .cpp files, otherwise
it would have caught this. Found by asan.
  • Loading branch information
Keno authored Aug 13, 2022
1 parent d2aedf4 commit e1fa6a5
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
ir = static_eval(ctx, ir_arg);
if (!ir) {
emit_error(ctx, "error statically evaluating llvm IR argument");
JL_GC_POP();
return jl_cgval_t();
}
if (jl_is_ssavalue(args[2]) && !jl_is_long(ctx.source->ssavaluetypes)) {
Expand All @@ -771,6 +772,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
rt = static_eval(ctx, args[2]);
if (!rt) {
emit_error(ctx, "error statically evaluating llvmcall return type");
JL_GC_POP();
return jl_cgval_t();
}
}
Expand All @@ -783,30 +785,35 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
at = static_eval(ctx, args[3]);
if (!at) {
emit_error(ctx, "error statically evaluating llvmcall argument tuple");
JL_GC_POP();
return jl_cgval_t();
}
}
if (jl_is_tuple(ir)) {
// if the IR is a tuple, we expect (mod, fn)
if (jl_nfields(ir) != 2) {
emit_error(ctx, "Tuple as first argument to llvmcall must have exactly two children");
JL_GC_POP();
return jl_cgval_t();
}
entry = jl_fieldref(ir, 1);
if (!jl_is_string(entry)) {
emit_error(ctx, "Function name passed to llvmcall must be a string");
JL_GC_POP();
return jl_cgval_t();
}
ir = jl_fieldref(ir, 0);

if (!jl_is_string(ir) && !jl_typeis(ir, jl_array_uint8_type)) {
emit_error(ctx, "Module IR passed to llvmcall must be a string or an array of bytes");
JL_GC_POP();
return jl_cgval_t();
}
}
else {
if (!jl_is_string(ir)) {
emit_error(ctx, "Function IR passed to llvmcall must be a string");
JL_GC_POP();
return jl_cgval_t();
}
}
Expand Down Expand Up @@ -835,6 +842,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
argtypes.push_back(t);
if (4 + i > nargs) {
emit_error(ctx, "Missing arguments to llvmcall!");
JL_GC_POP();
return jl_cgval_t();
}
jl_value_t *argi = args[4 + i];
Expand Down Expand Up @@ -889,6 +897,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
raw_string_ostream stream(message);
Err.print("", stream, true);
emit_error(ctx, stream.str());
JL_GC_POP();
return jl_cgval_t();
}

Expand All @@ -906,6 +915,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
raw_string_ostream stream(message);
Err.print("", stream, true);
emit_error(ctx, stream.str());
JL_GC_POP();
return jl_cgval_t();
}
}
Expand All @@ -923,6 +933,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
raw_string_ostream stream(message);
stream << Message;
emit_error(ctx, stream.str());
JL_GC_POP();
return jl_cgval_t();
}
Mod = std::move(ModuleOrErr.get());
Expand All @@ -931,6 +942,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
Function *f = Mod->getFunction(jl_string_data(entry));
if (!f) {
emit_error(ctx, "Module IR does not contain specified entry function");
JL_GC_POP();
return jl_cgval_t();
}
f->setName(ir_name);
Expand Down Expand Up @@ -959,6 +971,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
raw_string_ostream stream(message);
if (verifyFunction(*def, &stream)) {
emit_error(ctx, stream.str());
JL_GC_POP();
return jl_cgval_t();
}
def->setLinkage(GlobalVariable::LinkOnceODRLinkage);
Expand Down

4 comments on commit e1fa6a5

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.