Skip to content

Commit

Permalink
Use llvm::SmallVector instead of std::vector for bounds checking (#51579
Browse files Browse the repository at this point in the history 
)

This should catch issues like #51561,
at least when running with `FORCE_ASSERTIONS := 1` (as PkgEval does).
  • Loading branch information
@maleadt
maleadt authored Oct 10, 2023
1 parent f82f0d4 commit 651aef9
Show file tree
Hide file tree
Showing 24 changed files with 357 additions and 349 deletions.
43 changes: 22 additions & 21 deletions src/aotcompile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ static void addComdat(GlobalValue *G, Triple &T)

typedef struct {
orc::ThreadSafeModule M;
std::vector<GlobalValue*> jl_sysimg_fvars;
std::vector<GlobalValue*> jl_sysimg_gvars;
SmallVector<GlobalValue*, 0> jl_sysimg_fvars;
SmallVector<GlobalValue*, 0> jl_sysimg_gvars;
std::map<jl_code_instance_t*, std::tuple<uint32_t, uint32_t>> jl_fvar_map;
std::vector<void*> jl_value_to_llvm;
std::vector<jl_code_instance_t*> jl_external_to_llvm;
SmallVector<void*, 0> jl_value_to_llvm;
SmallVector<jl_code_instance_t*, 0> jl_external_to_llvm;
} jl_native_code_desc_t;

extern "C" JL_DLLEXPORT_CODEGEN
Expand Down Expand Up @@ -145,12 +145,13 @@ GlobalValue* jl_get_llvm_function_impl(void *native_code, uint32_t idx)
}


static void emit_offset_table(Module &mod, const std::vector<GlobalValue*> &vars, StringRef name, Type *T_psize)
static void emit_offset_table(Module &mod, ArrayRef<GlobalValue*> vars,
StringRef name, Type *T_psize)
{
// Emit a global variable with all the variable addresses.
// The cloning pass will convert them into offsets.
size_t nvars = vars.size();
std::vector<Constant*> addrs(nvars);
SmallVector<Constant*, 0> addrs(nvars);
for (size_t i = 0; i < nvars; i++) {
Constant *var = vars[i];
addrs[i] = ConstantExpr::getBitCast(var, T_psize);
Expand Down Expand Up @@ -354,7 +355,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm
JL_GC_POP();

// process the globals array, before jl_merge_module destroys them
std::vector<std::string> gvars(params.global_targets.size());
SmallVector<std::string, 0> gvars(params.global_targets.size());
data->jl_value_to_llvm.resize(params.global_targets.size());
StringSet<> gvars_names;
DenseSet<GlobalValue *> gvars_set;
Expand Down Expand Up @@ -705,8 +706,8 @@ static bool canPartition(const GlobalValue &G) {
static inline bool verify_partitioning(const SmallVectorImpl<Partition> &partitions, const Module &M, size_t fvars_size, size_t gvars_size) {
bool bad = false;
#ifndef JL_NDEBUG
SmallVector<uint32_t> fvars(fvars_size);
SmallVector<uint32_t> gvars(gvars_size);
SmallVector<uint32_t, 0> fvars(fvars_size);
SmallVector<uint32_t, 0> gvars(gvars_size);
StringMap<uint32_t> GVNames;
for (uint32_t i = 0; i < partitions.size(); i++) {
for (auto &name : partitions[i].globals) {
Expand Down Expand Up @@ -794,7 +795,7 @@ static SmallVector<Partition, 32> partitionModule(Module &M, unsigned threads) {
unsigned size;
size_t weight;
};
std::vector<Node> nodes;
SmallVector<Node, 0> nodes;
DenseMap<GlobalValue *, unsigned> node_map;
unsigned merged;

Expand Down Expand Up @@ -865,12 +866,12 @@ static SmallVector<Partition, 32> partitionModule(Module &M, unsigned threads) {
auto pcomp = [](const Partition *p1, const Partition *p2) {
return p1->weight > p2->weight;
};
std::priority_queue<Partition *, std::vector<Partition *>, decltype(pcomp)> pq(pcomp);
std::priority_queue<Partition *, SmallVector<Partition *, 0>, decltype(pcomp)> pq(pcomp);
for (unsigned i = 0; i < threads; ++i) {
pq.push(&partitions[i]);
}

std::vector<unsigned> idxs(partitioner.nodes.size());
SmallVector<unsigned, 0> idxs(partitioner.nodes.size());
std::iota(idxs.begin(), idxs.end(), 0);
std::sort(idxs.begin(), idxs.end(), [&](unsigned a, unsigned b) {
//because roots have more weight than their children,
Expand Down Expand Up @@ -1212,33 +1213,33 @@ static void materializePreserved(Module &M, Partition &partition) {

// Reconstruct jl_fvars, jl_gvars, jl_fvars_idxs, and jl_gvars_idxs from the partition
static void construct_vars(Module &M, Partition &partition) {
std::vector<std::pair<uint32_t, GlobalValue *>> fvar_pairs;
SmallVector<std::pair<uint32_t, GlobalValue *>> fvar_pairs;
fvar_pairs.reserve(partition.fvars.size());
for (auto &fvar : partition.fvars) {
auto F = M.getFunction(fvar.first());
assert(F);
assert(!F->isDeclaration());
fvar_pairs.push_back({ fvar.second, F });
}
std::vector<GlobalValue *> fvars;
std::vector<uint32_t> fvar_idxs;
SmallVector<GlobalValue *, 0> fvars;
SmallVector<uint32_t, 0> fvar_idxs;
fvars.reserve(fvar_pairs.size());
fvar_idxs.reserve(fvar_pairs.size());
std::sort(fvar_pairs.begin(), fvar_pairs.end());
for (auto &fvar : fvar_pairs) {
fvars.push_back(fvar.second);
fvar_idxs.push_back(fvar.first);
}
std::vector<std::pair<uint32_t, GlobalValue *>> gvar_pairs;
SmallVector<std::pair<uint32_t, GlobalValue *>, 0> gvar_pairs;
gvar_pairs.reserve(partition.gvars.size());
for (auto &gvar : partition.gvars) {
auto GV = M.getNamedGlobal(gvar.first());
assert(GV);
assert(!GV->isDeclaration());
gvar_pairs.push_back({ gvar.second, GV });
}
std::vector<GlobalValue *> gvars;
std::vector<uint32_t> gvar_idxs;
SmallVector<GlobalValue *, 0> gvars;
SmallVector<uint32_t, 0> gvar_idxs;
gvars.reserve(gvar_pairs.size());
gvar_idxs.reserve(gvar_pairs.size());
std::sort(gvar_pairs.begin(), gvar_pairs.end());
Expand Down Expand Up @@ -1630,7 +1631,7 @@ void jl_dump_native_impl(void *native_code,
ngvars = data->jl_sysimg_gvars.size();
emit_offset_table(dataM, data->jl_sysimg_gvars, "jl_gvars", T_psize);
emit_offset_table(dataM, data->jl_sysimg_fvars, "jl_fvars", T_psize);
std::vector<uint32_t> idxs;
SmallVector<uint32_t, 0> idxs;
idxs.resize(data->jl_sysimg_gvars.size());
std::iota(idxs.begin(), idxs.end(), 0);
auto gidxs = ConstantDataArray::get(Context, idxs);
Expand Down Expand Up @@ -1712,7 +1713,7 @@ void jl_dump_native_impl(void *native_code,
if (imaging_mode) {
auto specs = jl_get_llvm_clone_targets();
const uint32_t base_flags = has_veccall ? JL_TARGET_VEC_CALL : 0;
std::vector<uint8_t> data;
SmallVector<uint8_t, 0> data;
auto push_i32 = [&] (uint32_t v) {
uint8_t buff[4];
memcpy(buff, &v, 4);
Expand Down Expand Up @@ -1766,7 +1767,7 @@ void jl_dump_native_impl(void *native_code,
object::Archive::Kind Kind = getDefaultForHost(TheTriple);
#define WRITE_ARCHIVE(fname, field, prefix, suffix) \
if (fname) {\
std::vector<NewArchiveMember> archive; \
SmallVector<NewArchiveMember, 0> archive; \
SmallVector<std::string, 16> filenames; \
SmallVector<StringRef, 16> buffers; \
for (size_t i = 0; i < threads; i++) { \
Expand Down
28 changes: 14 additions & 14 deletions src/ccall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
* If the argument type is immutable (including bitstype), we pass the loaded llvm value
* type. Otherwise we pass a pointer to a jl_value_t.
*/
std::vector<llvm::Type*> argtypes;
SmallVector<llvm::Type*, 0> argtypes;
SmallVector<Value *, 8> argvals(nargt);
for (size_t i = 0; i < nargt; ++i) {
jl_value_t *tti = jl_svecref(tt,i);
Expand Down Expand Up @@ -862,7 +862,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
// we only have function IR, which we should put in a function

bool first = true;
for (std::vector<Type *>::iterator it = argtypes.begin(); it != argtypes.end(); ++it) {
for (SmallVector<Type *, 0>::iterator it = argtypes.begin(); it != argtypes.end(); ++it) {
if (!first)
argstream << ",";
else
Expand Down Expand Up @@ -943,7 +943,7 @@ static jl_cgval_t emit_llvmcall(jl_codectx_t &ctx, jl_value_t **args, size_t nar
assert(!f->isDeclaration());
assert(f->getReturnType() == rettype);
int i = 0;
for (std::vector<Type *>::iterator it = argtypes.begin();
for (SmallVector<Type *, 0>::iterator it = argtypes.begin();
it != argtypes.end(); ++it, ++i)
assert(*it == f->getFunctionType()->getParamType(i));
}
Expand Down Expand Up @@ -1022,10 +1022,10 @@ static jl_cgval_t mark_or_box_ccall_result(jl_codectx_t &ctx, Value *result, boo

class function_sig_t {
public:
std::vector<Type*> fargt; // vector of llvm output types (julia_struct_to_llvm) for arguments
std::vector<Type*> fargt_sig; // vector of ABI coercion types for call signature
std::vector<bool> fargt_isboxed; // vector of whether the llvm output type is a Julia-box for each argument
std::vector<bool> byRefList; // vector of "byref" parameters
SmallVector<Type*, 0> fargt; // vector of llvm output types (julia_struct_to_llvm) for arguments
SmallVector<Type*, 0> fargt_sig; // vector of ABI coercion types for call signature
SmallVector<bool, 0> fargt_isboxed; // vector of whether the llvm output type is a Julia-box for each argument
SmallVector<bool, 0> byRefList; // vector of "byref" parameters
AttributeList attributes; // vector of function call site attributes
Type *lrt; // input parameter of the llvm return type (from julia_struct_to_llvm)
bool retboxed; // input parameter indicating whether lrt is jl_value_t*
Expand Down Expand Up @@ -1071,7 +1071,7 @@ std::string generate_func_sig(const char *fname)
{
assert(rt && !jl_is_abstract_ref_type(rt));

std::vector<AttributeSet> paramattrs;
SmallVector<AttributeSet, 0> paramattrs;
std::unique_ptr<AbiLayout> abi;
if (llvmcall)
abi.reset(new ABI_LLVMLayout());
Expand Down Expand Up @@ -1938,10 +1938,10 @@ jl_cgval_t function_sig_t::emit_a_ccall(
// Current C function parameter
jl_cgval_t &arg = argv[ai];
jl_value_t *jargty = jl_svecref(at, ai); // Julia type of the current parameter
Type *largty = fargt.at(ai); // LLVM type of the current parameter
bool toboxed = fargt_isboxed.at(ai);
Type *pargty = fargt_sig.at(ai + sret); // LLVM coercion type
bool byRef = byRefList.at(ai); // Argument attributes
Type *largty = fargt[ai]; // LLVM type of the current parameter
bool toboxed = fargt_isboxed[ai];
Type *pargty = fargt_sig[ai + sret]; // LLVM coercion type
bool byRef = byRefList[ai]; // Argument attributes

// if we know the function sparams, try to fill those in now
// so that the julia_to_native type checks are more likely to be doable (e.g. concrete types) at compile-time
Expand Down Expand Up @@ -1993,7 +1993,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
result = emit_static_alloca(ctx, lrt);
setName(ctx.emission_context, result, "ccall_sret");
sretty = lrt;
argvals[0] = ctx.builder.CreateBitCast(result, fargt_sig.at(0));
argvals[0] = ctx.builder.CreateBitCast(result, fargt_sig[0]);
}
else {
// XXX: result needs to be zero'd and given a GC root here
Expand All @@ -2004,7 +2004,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
sretty = ctx.types().T_jlvalue;
sretboxed = true;
gc_uses.push_back(result);
argvals[0] = ctx.builder.CreateBitCast(emit_pointer_from_objref(ctx, result), fargt_sig.at(0));
argvals[0] = ctx.builder.CreateBitCast(emit_pointer_from_objref(ctx, result), fargt_sig[0]);
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ static DIType *_julia_type_to_di(jl_codegen_params_t *ctx, jl_debugcache_t &debu
}
else if (jl_is_structtype(jt) && !jl_is_layout_opaque(jdt->layout)) {
size_t ntypes = jl_datatype_nfields(jdt);
std::vector<llvm::Metadata*> Elements(ntypes);
SmallVector<llvm::Metadata*, 0> Elements(ntypes);
for (unsigned i = 0; i < ntypes; i++) {
jl_value_t *el = jl_field_type_concrete(jdt, i);
DIType *di;
Expand Down Expand Up @@ -278,7 +278,7 @@ void jl_debugcache_t::initialize(Module *m) {
__alignof__(jl_value_t*) * 8);

SmallVector<llvm::Metadata *, 1> Elts;
std::vector<Metadata*> diargs(0);
SmallVector<Metadata*, 0> diargs(0);
Elts.push_back(jl_pvalue_dillvmt);
dbuilder.replaceArrays(jl_value_dillvmt,
dbuilder.getOrCreateArray(Elts));
Expand Down Expand Up @@ -722,7 +722,7 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, LLVMContext &ctxt,
Type *&struct_decl = (ctx && !llvmcall ? ctx->llvmtypes[jst] : _struct_decl);
if (struct_decl)
return struct_decl;
std::vector<Type*> latypes(0);
SmallVector<Type*, 0> latypes(0);
bool isarray = true;
bool isvector = true;
jl_value_t *jlasttype = NULL;
Expand Down Expand Up @@ -821,7 +821,7 @@ static Type *_julia_struct_to_llvm(jl_codegen_params_t *ctx, LLVMContext &ctxt,
// // pick an Integer type size such that alignment will be correct
// // and always end with an Int8 (selector byte)
// lty = ArrayType::get(IntegerType::get(lty->getContext(), 8 * al), fsz / al);
// std::vector<Type*> Elements(2);
// SmallVector<Type*, 0> Elements(2);
// Elements[0] = lty;
// Elements[1] = getInt8Ty(ctxt);
// unsigned remainder = fsz % al;
Expand Down Expand Up @@ -1793,7 +1793,7 @@ static void emit_write_barrier(jl_codectx_t&, Value*, ArrayRef<Value*>);
static void emit_write_barrier(jl_codectx_t&, Value*, Value*);
static void emit_write_multibarrier(jl_codectx_t&, Value*, Value*, jl_value_t*);

std::vector<unsigned> first_ptr(Type *T)
SmallVector<unsigned, 0> first_ptr(Type *T)
{
if (isa<StructType>(T) || isa<ArrayType>(T) || isa<VectorType>(T)) {
if (!isa<StructType>(T)) {
Expand All @@ -1811,7 +1811,7 @@ std::vector<unsigned> first_ptr(Type *T)
unsigned i = 0;
for (Type *ElTy : T->subtypes()) {
if (isa<PointerType>(ElTy) && ElTy->getPointerAddressSpace() == AddressSpace::Tracked) {
return std::vector<unsigned>{i};
return SmallVector<unsigned, 0>{i};
}
auto path = first_ptr(ElTy);
if (!path.empty()) {
Expand Down Expand Up @@ -3010,7 +3010,7 @@ static Value *emit_array_nd_index(
endBB = BasicBlock::Create(ctx.builder.getContext(), "idxend");
}
#endif
SmallVector<Value *> idxs(nidxs);
SmallVector<Value *, 0> idxs(nidxs);
for (size_t k = 0; k < nidxs; k++) {
idxs[k] = emit_unbox(ctx, ctx.types().T_size, argv[k], (jl_value_t*)jl_long_type); // type asserted by caller
}
Expand Down Expand Up @@ -3733,7 +3733,7 @@ static void emit_write_barrier(jl_codectx_t &ctx, Value *parent, ArrayRef<Value*
ctx.builder.CreateCall(prepare_call(jl_write_barrier_func), decay_ptrs);
}

static void find_perm_offsets(jl_datatype_t *typ, SmallVector<unsigned,4> &res, unsigned offset)
static void find_perm_offsets(jl_datatype_t *typ, SmallVectorImpl<unsigned> &res, unsigned offset)
{
// This is a inlined field at `offset`.
if (!typ->layout || typ->layout->npointers == 0)
Expand Down
Loading

4 comments on commit 651aef9

@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(isdaily = true)

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(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 benchmark job has completed - possible performance regressions 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.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.