Skip to content

Commit

Permalink
run clang static analyzer default checks (#42278)
Browse files Browse the repository at this point in the history
also, make it happy, and fix the missing `free` call that it found
  • Loading branch information
vtjnash authored and KristofferC committed Sep 28, 2021
1 parent 7425ef3 commit c7d39b7
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 26 deletions.
15 changes: 13 additions & 2 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,21 @@ clang-sagc-%: $(SRCDIR)/%.c $(build_shlibdir)/libGCCheckerPlugin.$(SHLIB_EXT) .F
clang-sagc-%: $(SRCDIR)/%.cpp $(build_shlibdir)/libGCCheckerPlugin.$(SHLIB_EXT) .FORCE | analyzegc-deps-check
@$(call PRINT_ANALYZE, $(build_bindir)/clang -D__clang_gcanalyzer__ --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text -Xclang -load -Xclang $(build_shlibdir)/libGCCheckerPlugin.$(SHLIB_EXT) $(CLANGSA_FLAGS) $(CLANGSA_CXXFLAGS) $(LLVM_CXXFLAGS) $(JCPPFLAGS) $(JCXXFLAGS) $(DEBUGFLAGS) -Xclang -analyzer-checker=core$(COMMA)julia.GCChecker --analyzer-no-default-checks -fcolor-diagnostics -Werror -x c++ $<)

# optarg is a required_argument for these
SA_EXCEPTIONS-jloptions.c := -Xanalyzer -analyzer-disable-checker=core.NonNullParamChecker,unix.cstring.NullArg
# clang doesn't understand that e->vars has the same value in save_env (NULL) and restore_env (assumed non-NULL)
SA_EXCEPTIONS-subtype.c := -Xanalyzer -analyzer-disable-checker=core.uninitialized.Assign,core.UndefinedBinaryOperatorResult

clang-sa-%: $(SRCDIR)/%.c .FORCE | analyzegc-deps-check
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text $(CLANGSA_FLAGS) $(JCPPFLAGS) $(JCFLAGS) $(DEBUGFLAGS) --analyzer-no-default-checks -fcolor-diagnostics -Werror -x c $<)
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text \
-Xanalyzer -analyzer-disable-checker=deadcode.DeadStores \
$(SA_EXCEPTIONS-$(notdir $<)) \
$(CLANGSA_FLAGS) $(JCPPFLAGS) $(JCFLAGS) $(DEBUGFLAGS) -fcolor-diagnostics -Werror -x c $<)
clang-sa-%: $(SRCDIR)/%.cpp .FORCE | analyzegc-deps-check
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text $(CLANGSA_FLAGS) $(CLANGSA_CXXFLAGS) $(LLVM_CXXFLAGS) $(JCPPFLAGS) $(JCXXFLAGS) $(DEBUGFLAGS) --analyzer-no-default-checks -fcolor-diagnostics -Werror -x c++ $<)
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text \
-Xanalyzer -analyzer-disable-checker=deadcode.DeadStores \
$(SA_EXCEPTIONS-$(notdir $<)) \
$(CLANGSA_FLAGS) $(CLANGSA_CXXFLAGS) $(LLVM_CXXFLAGS) $(JCPPFLAGS) $(JCXXFLAGS) $(DEBUGFLAGS) -fcolor-diagnostics -Werror -x c++ $<)


# Add C files as a target of `analyzesrc` and `analyzegc`
Expand Down
8 changes: 6 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,11 @@ static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_v
return im1;
}

static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt, Value* dest = NULL, MDNode *tbaa_dest = nullptr, bool isVolatile = false);
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt, Value* dest, MDNode *tbaa_dest, bool isVolatile = false);
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt)
{
return emit_unbox(ctx, to, x, jt, nullptr, nullptr, false);
}
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*);
Expand Down Expand Up @@ -3075,7 +3079,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
if (jt == (jl_value_t*)jl_nothing_type)
return track_pjlvalue(ctx, literal_pointer_val(ctx, jl_nothing));
if (vinfo.isboxed) {
assert(vinfo.V == vinfo.Vboxed);
assert(vinfo.V == vinfo.Vboxed && vinfo.V != nullptr);
assert(vinfo.V->getType() == T_prjlvalue);
return vinfo.V;
}
Expand Down
26 changes: 18 additions & 8 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2077,6 +2077,7 @@ static void cg_bdw(jl_codectx_t &ctx, jl_binding_t *b)

static jl_value_t *static_apply_type(jl_codectx_t &ctx, const jl_cgval_t *args, size_t nargs)
{
assert(nargs > 1);
jl_value_t **v = (jl_value_t**)alloca(sizeof(jl_value_t*) * nargs);
for (size_t i = 0; i < nargs; i++) {
if (!args[i].constant)
Expand Down Expand Up @@ -4582,14 +4583,17 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)

jl_expr_t *ex = (jl_expr_t*)expr;
jl_value_t **args = (jl_value_t**)jl_array_data(ex->args);
size_t nargs = jl_array_len(ex->args);
jl_sym_t *head = ex->head;
// this is object-disoriented.
// however, this is a good way to do it because it should *not* be easy
// to add new node types.
if (head == isdefined_sym) {
assert(nargs == 1);
return emit_isdefined(ctx, args[0]);
}
else if (head == throw_undef_if_not_sym) {
assert(nargs == 2);
jl_sym_t *var = (jl_sym_t*)args[0];
Value *cond = ctx.builder.CreateTrunc(emit_unbox(ctx, T_int8, emit_expr(ctx, args[1]), (jl_value_t*)jl_bool_type), T_int1);
if (var == getfield_undefref_sym) {
Expand Down Expand Up @@ -4633,20 +4637,23 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
return emit_ccall(ctx, args, jl_array_dim0(ex->args));
}
else if (head == cfunction_sym) {
assert(nargs == 5);
jl_cgval_t fexpr_rt = emit_expr(ctx, args[1]);
return emit_cfunction(ctx, args[0], fexpr_rt, args[2], (jl_svec_t*)args[3]);
}
else if (head == assign_sym) {
assert(nargs == 2);
emit_assignment(ctx, args[0], args[1], ssaval);
return ghostValue(jl_nothing_type);
}
else if (head == static_parameter_sym) {
assert(nargs == 1);
return emit_sparam(ctx, jl_unbox_long(args[0]) - 1);
}
else if (head == method_sym) {
if (jl_expr_nargs(ex) == 1) {
if (nargs == 1) {
jl_value_t *mn = args[0];
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(mn) || jl_is_slot(mn));
assert(jl_is_symbol(mn) || jl_is_slot(mn));

Value *bp = NULL, *name, *bp_owner = V_null;
jl_binding_t *bnd = NULL;
Expand Down Expand Up @@ -4695,6 +4702,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
emit_error(ctx, "method: invalid declaration");
return jl_cgval_t();
}
assert(nargs == 3);
Value *a1 = boxed(ctx, emit_expr(ctx, args[1]));
Value *a2 = boxed(ctx, emit_expr(ctx, args[2]));
Value *mdargs[4] = {
Expand All @@ -4711,6 +4719,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
return meth;
}
else if (head == const_sym) {
assert(nargs == 1);
jl_sym_t *sym = (jl_sym_t*)args[0];
jl_module_t *mod = ctx.module;
if (jl_is_globalref(sym)) {
Expand All @@ -4725,7 +4734,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
}
}
else if (head == new_sym) {
size_t nargs = jl_array_len(ex->args);
assert(nargs > 0);
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs);
for (size_t i = 0; i < nargs; ++i) {
argv[i] = emit_expr(ctx, args[i]);
Expand All @@ -4744,6 +4753,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
}
else if (head == splatnew_sym) {
jl_cgval_t argv[2];
assert(nargs == 2);
argv[0] = emit_expr(ctx, args[0]);
argv[1] = emit_expr(ctx, args[1]);
Value *typ = boxed(ctx, argv[0]);
Expand All @@ -4754,7 +4764,6 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
return mark_julia_type(ctx, val, true, (jl_value_t*)jl_any_type);
}
else if (head == new_opaque_closure_sym) {
size_t nargs = jl_array_len(ex->args);
assert(nargs >= 5 && "Not enough arguments in new_opaque_closure");
SmallVector<jl_cgval_t, 5> argv(nargs);
for (size_t i = 0; i < nargs; ++i) {
Expand Down Expand Up @@ -4894,11 +4903,13 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
true, jl_any_type);
}
else if (head == exc_sym) {
assert(nargs == 0);
return mark_julia_type(ctx,
ctx.builder.CreateCall(prepare_call(jl_current_exception_func)),
true, jl_any_type);
}
else if (head == copyast_sym) {
assert(nargs == 1);
jl_cgval_t ast = emit_expr(ctx, args[0]);
if (ast.typ != (jl_value_t*)jl_expr_type && ast.typ != (jl_value_t*)jl_any_type) {
// elide call to jl_copy_ast when possible
Expand All @@ -4911,7 +4922,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
else if (head == loopinfo_sym) {
// parse Expr(:loopinfo, "julia.simdloop", ("llvm.loop.vectorize.width", 4))
SmallVector<Metadata *, 8> MDs;
for (int i = 0, ie = jl_expr_nargs(ex); i < ie; ++i) {
for (int i = 0, ie = nargs; i < ie; ++i) {
Metadata *MD = to_md_tree(args[i]);
if (MD)
MDs.push_back(MD);
Expand All @@ -4931,7 +4942,6 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
return mark_julia_const(bounds_check_enabled(ctx, jl_true) ? jl_true : jl_false);
}
else if (head == gc_preserve_begin_sym) {
size_t nargs = jl_array_len(ex->args);
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs);
for (size_t i = 0; i < nargs; ++i) {
argv[i] = emit_expr(ctx, args[i]);
Expand Down Expand Up @@ -7452,7 +7462,7 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
assert(lty != T_prjlvalue);
Value *isvalid = emit_isa(ctx, val, phiType, NULL).first;
emit_guarded_test(ctx, isvalid, nullptr, [&] {
(void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest));
(void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest), tbaa_stack);
return nullptr;
});
}
Expand Down Expand Up @@ -7480,7 +7490,7 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
V = V_rnull;
Type *lty = julia_type_to_llvm(ctx, val.typ);
if (dest && !type_is_ghost(lty)) // basically, if !ghost union
emit_unbox(ctx, lty, val, val.typ, dest);
emit_unbox(ctx, lty, val, val.typ, dest, tbaa_stack);
RTindex = ConstantInt::get(T_int8, tindex);
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/debuginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1477,20 +1477,24 @@ void register_eh_frames(uint8_t *Addr, size_t Size)
jl_profile_atomic([&]() {
__register_frame(Addr);
});

// Now first count the number of FDEs
size_t nentries = 0;
processFDEs((char*)Addr, Size, [&](const char*){ nentries++; });
if (nentries == 0)
return;

// Our unwinder
unw_dyn_info_t *di = new unw_dyn_info_t;
// In a shared library, this is set to the address of the PLT.
// For us, just put 0 to emulate a static library. This field does
// not seem to be used on our supported architectures.
di->gp = 0;
// I'm not a great fan of the naming of this constant, but it means the
// right thing, which is a table of FDEs and ips.
// right thing, which is a table of FDEs and IPs.
di->format = UNW_INFO_FORMAT_IP_OFFSET;
di->u.rti.name_ptr = 0;
di->u.rti.segbase = (unw_word_t)Addr;
// Now first count the number of FDEs
size_t nentries = 0;
processFDEs((char*)Addr, Size, [&](const char*){ nentries++; });

uintptr_t start_ip = (uintptr_t)-1;
uintptr_t end_ip = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ static void NOINLINE gc_mark_stack_resize(jl_gc_mark_cache_t *gc_cache, jl_gc_ma

sp->pc_start = gc_cache->pc_stack = (void**)realloc_s(pc_stack, stack_size * 2 * sizeof(void*));
gc_cache->pc_stack_end = sp->pc_end = sp->pc_start + stack_size * 2;
sp->pc += sp->pc_start - pc_stack;
sp->pc = sp->pc_start + (sp->pc - pc_stack);
JL_UNLOCK_NOGC(&gc_cache->stack_lock);
}

Expand Down
1 change: 1 addition & 0 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2577,6 +2577,7 @@ JL_DLLEXPORT jl_function_t *jl_get_kwsorter(jl_value_t *ty)
strcpy(&suffixed[0], name);
strcpy(&suffixed[l], "##kw");
jl_sym_t *fname = jl_symbol(suffixed);
free(suffixed);
mt->kwsorter = jl_new_generic_function_with_supertype(fname, mt->module, jl_function_type);
jl_gc_wb(mt, mt->kwsorter);
}
Expand Down
16 changes: 16 additions & 0 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1074,47 +1074,63 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar

switch (f) {
case arraylen: {
assert(nargs == 1);
const jl_cgval_t &x = argv[0];
jl_value_t *typ = jl_unwrap_unionall(x.typ);
if (!jl_is_datatype(typ) || ((jl_datatype_t*)typ)->name != jl_array_typename)
return emit_runtime_call(ctx, f, argv, nargs);
return mark_julia_type(ctx, emit_arraylen(ctx, x), false, jl_long_type);
}
case pointerref:
assert(nargs == 3);
return emit_pointerref(ctx, argv);
case pointerset:
assert(nargs == 4);
return emit_pointerset(ctx, argv);
case atomic_fence:
assert(nargs == 1);
return emit_atomicfence(ctx, argv);
case atomic_pointerref:
assert(nargs == 2);
return emit_atomic_pointerref(ctx, argv);
case atomic_pointerset:
case atomic_pointerswap:
case atomic_pointermodify:
case atomic_pointerreplace:
return emit_atomic_pointerop(ctx, f, argv, nargs, nullptr);
case bitcast:
assert(nargs == 2);
return generic_bitcast(ctx, argv);
case trunc_int:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::Trunc, argv, true, true);
case sext_int:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::SExt, argv, true, true);
case zext_int:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::ZExt, argv, true, true);
case uitofp:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::UIToFP, argv, false, true);
case sitofp:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::SIToFP, argv, false, true);
case fptoui:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::FPToUI, argv, true, false);
case fptosi:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::FPToSI, argv, true, false);
case fptrunc:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::FPTrunc, argv, false, false);
case fpext:
assert(nargs == 2);
return generic_cast(ctx, f, Instruction::FPExt, argv, false, false);

case not_int: {
assert(nargs == 1);
const jl_cgval_t &x = argv[0];
if (!jl_is_primitivetype(x.typ))
return emit_runtime_call(ctx, f, argv, nargs);
Expand Down
10 changes: 5 additions & 5 deletions src/jitlayers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,8 @@ uint64_t JuliaOJIT::getFunctionAddress(StringRef Name)
static int globalUniqueGeneratedNames;
StringRef JuliaOJIT::getFunctionAtAddress(uint64_t Addr, jl_code_instance_t *codeinst)
{
auto &fname = ReverseLocalSymbolTable[(void*)(uintptr_t)Addr];
if (fname.empty()) {
std::string *fname = &ReverseLocalSymbolTable[(void*)(uintptr_t)Addr];
if (fname->empty()) {
std::string string_fname;
raw_string_ostream stream_fname(string_fname);
// try to pick an appropriate name that describes it
Expand All @@ -851,10 +851,10 @@ StringRef JuliaOJIT::getFunctionAtAddress(uint64_t Addr, jl_code_instance_t *cod
}
const char* unadorned_name = jl_symbol_name(codeinst->def->def.method->name);
stream_fname << unadorned_name << "_" << globalUniqueGeneratedNames++;
fname = strdup(stream_fname.str().c_str());
addGlobalMapping(fname, Addr);
*fname = std::move(stream_fname.str()); // store to ReverseLocalSymbolTable
addGlobalMapping(*fname, Addr);
}
return fname;
return *fname;
}


Expand Down
2 changes: 1 addition & 1 deletion src/jitlayers.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ class JuliaOJIT {
ObjLayerT ObjectLayer;
CompileLayerT CompileLayer;

DenseMap<void*, StringRef> ReverseLocalSymbolTable;
DenseMap<void*, std::string> ReverseLocalSymbolTable;
};
extern JuliaOJIT *jl_ExecutionEngine;

Expand Down
2 changes: 1 addition & 1 deletion src/llvm-final-gc-lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ bool FinalLowerGC::doFinalization(Module &M)
functionList + sizeof(functionList) / sizeof(void*));
bool changed = false;
SmallVector<Constant*, 16> init;
ConstantArray *CA = dyn_cast<ConstantArray>(used->getInitializer());
ConstantArray *CA = cast<ConstantArray>(used->getInitializer());
for (auto &Op : CA->operands()) {
Constant *C = cast_or_null<Constant>(Op);
if (InitAsSet.count(C->stripPointerCasts())) {
Expand Down
4 changes: 2 additions & 2 deletions src/support/dtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ STATIC_INLINE void *malloc_s(size_t sz) JL_NOTSAFEPOINT {
#ifdef _OS_WINDOWS_
DWORD last_error = GetLastError();
#endif
void *p = malloc(sz);
void *p = malloc(sz == 0 ? 1 : sz);
if (p == NULL) {
perror("(julia) malloc");
abort();
Expand All @@ -386,7 +386,7 @@ STATIC_INLINE void *realloc_s(void *p, size_t sz) JL_NOTSAFEPOINT {
#ifdef _OS_WINDOWS_
DWORD last_error = GetLastError();
#endif
p = realloc(p, sz);
p = realloc(p, sz == 0 ? 1 : sz);
if (p == NULL) {
perror("(julia) realloc");
abort();
Expand Down

0 comments on commit c7d39b7

Please sign in to comment.