Skip to content

Commit 506afff

Browse files
authored
run clang static analyzer default checks (#42278)
also, make it happy, and fix the missing `free` call that it found
1 parent 6893f21 commit 506afff

File tree

11 files changed

+72
-26
lines changed

11 files changed

+72
-26
lines changed

src/Makefile

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,10 +381,21 @@ clang-sagc-%: $(SRCDIR)/%.c $(build_shlibdir)/libGCCheckerPlugin.$(SHLIB_EXT) .F
381381
clang-sagc-%: $(SRCDIR)/%.cpp $(build_shlibdir)/libGCCheckerPlugin.$(SHLIB_EXT) .FORCE | analyzegc-deps-check
382382
@$(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++ $<)
383383

384+
# optarg is a required_argument for these
385+
SA_EXCEPTIONS-jloptions.c := -Xanalyzer -analyzer-disable-checker=core.NonNullParamChecker,unix.cstring.NullArg
386+
# clang doesn't understand that e->vars has the same value in save_env (NULL) and restore_env (assumed non-NULL)
387+
SA_EXCEPTIONS-subtype.c := -Xanalyzer -analyzer-disable-checker=core.uninitialized.Assign,core.UndefinedBinaryOperatorResult
388+
384389
clang-sa-%: $(SRCDIR)/%.c .FORCE | analyzegc-deps-check
385-
@$(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 $<)
390+
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text \
391+
-Xanalyzer -analyzer-disable-checker=deadcode.DeadStores \
392+
$(SA_EXCEPTIONS-$(notdir $<)) \
393+
$(CLANGSA_FLAGS) $(JCPPFLAGS) $(JCFLAGS) $(DEBUGFLAGS) -fcolor-diagnostics -Werror -x c $<)
386394
clang-sa-%: $(SRCDIR)/%.cpp .FORCE | analyzegc-deps-check
387-
@$(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++ $<)
395+
@$(call PRINT_ANALYZE, $(build_bindir)/clang --analyze -Xanalyzer -analyzer-werror -Xanalyzer -analyzer-output=text \
396+
-Xanalyzer -analyzer-disable-checker=deadcode.DeadStores \
397+
$(SA_EXCEPTIONS-$(notdir $<)) \
398+
$(CLANGSA_FLAGS) $(CLANGSA_CXXFLAGS) $(LLVM_CXXFLAGS) $(JCPPFLAGS) $(JCXXFLAGS) $(DEBUGFLAGS) -fcolor-diagnostics -Werror -x c++ $<)
388399

389400

390401
# Add C files as a target of `analyzesrc` and `analyzegc`

src/cgutils.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,7 +1407,11 @@ static Value *emit_bounds_check(jl_codectx_t &ctx, const jl_cgval_t &ainfo, jl_v
14071407
return im1;
14081408
}
14091409

1410-
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);
1410+
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);
1411+
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt)
1412+
{
1413+
return emit_unbox(ctx, to, x, jt, nullptr, nullptr, false);
1414+
}
14111415
static void emit_write_barrier(jl_codectx_t&, Value*, ArrayRef<Value*>);
14121416
static void emit_write_barrier(jl_codectx_t&, Value*, Value*);
14131417
static void emit_write_multibarrier(jl_codectx_t&, Value*, Value*, jl_value_t*);
@@ -3075,7 +3079,7 @@ static Value *boxed(jl_codectx_t &ctx, const jl_cgval_t &vinfo)
30753079
if (jt == (jl_value_t*)jl_nothing_type)
30763080
return track_pjlvalue(ctx, literal_pointer_val(ctx, jl_nothing));
30773081
if (vinfo.isboxed) {
3078-
assert(vinfo.V == vinfo.Vboxed);
3082+
assert(vinfo.V == vinfo.Vboxed && vinfo.V != nullptr);
30793083
assert(vinfo.V->getType() == T_prjlvalue);
30803084
return vinfo.V;
30813085
}

src/codegen.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,6 +2077,7 @@ static void cg_bdw(jl_codectx_t &ctx, jl_binding_t *b)
20772077

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

45834584
jl_expr_t *ex = (jl_expr_t*)expr;
45844585
jl_value_t **args = (jl_value_t**)jl_array_data(ex->args);
4586+
size_t nargs = jl_array_len(ex->args);
45854587
jl_sym_t *head = ex->head;
45864588
// this is object-disoriented.
45874589
// however, this is a good way to do it because it should *not* be easy
45884590
// to add new node types.
45894591
if (head == isdefined_sym) {
4592+
assert(nargs == 1);
45904593
return emit_isdefined(ctx, args[0]);
45914594
}
45924595
else if (head == throw_undef_if_not_sym) {
4596+
assert(nargs == 2);
45934597
jl_sym_t *var = (jl_sym_t*)args[0];
45944598
Value *cond = ctx.builder.CreateTrunc(emit_unbox(ctx, T_int8, emit_expr(ctx, args[1]), (jl_value_t*)jl_bool_type), T_int1);
45954599
if (var == getfield_undefref_sym) {
@@ -4633,20 +4637,23 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
46334637
return emit_ccall(ctx, args, jl_array_dim0(ex->args));
46344638
}
46354639
else if (head == cfunction_sym) {
4640+
assert(nargs == 5);
46364641
jl_cgval_t fexpr_rt = emit_expr(ctx, args[1]);
46374642
return emit_cfunction(ctx, args[0], fexpr_rt, args[2], (jl_svec_t*)args[3]);
46384643
}
46394644
else if (head == assign_sym) {
4645+
assert(nargs == 2);
46404646
emit_assignment(ctx, args[0], args[1], ssaval);
46414647
return ghostValue(jl_nothing_type);
46424648
}
46434649
else if (head == static_parameter_sym) {
4650+
assert(nargs == 1);
46444651
return emit_sparam(ctx, jl_unbox_long(args[0]) - 1);
46454652
}
46464653
else if (head == method_sym) {
4647-
if (jl_expr_nargs(ex) == 1) {
4654+
if (nargs == 1) {
46484655
jl_value_t *mn = args[0];
4649-
assert(jl_expr_nargs(ex) != 1 || jl_is_symbol(mn) || jl_is_slot(mn));
4656+
assert(jl_is_symbol(mn) || jl_is_slot(mn));
46504657

46514658
Value *bp = NULL, *name, *bp_owner = V_null;
46524659
jl_binding_t *bnd = NULL;
@@ -4695,6 +4702,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
46954702
emit_error(ctx, "method: invalid declaration");
46964703
return jl_cgval_t();
46974704
}
4705+
assert(nargs == 3);
46984706
Value *a1 = boxed(ctx, emit_expr(ctx, args[1]));
46994707
Value *a2 = boxed(ctx, emit_expr(ctx, args[2]));
47004708
Value *mdargs[4] = {
@@ -4711,6 +4719,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
47114719
return meth;
47124720
}
47134721
else if (head == const_sym) {
4722+
assert(nargs == 1);
47144723
jl_sym_t *sym = (jl_sym_t*)args[0];
47154724
jl_module_t *mod = ctx.module;
47164725
if (jl_is_globalref(sym)) {
@@ -4725,7 +4734,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
47254734
}
47264735
}
47274736
else if (head == new_sym) {
4728-
size_t nargs = jl_array_len(ex->args);
4737+
assert(nargs > 0);
47294738
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs);
47304739
for (size_t i = 0; i < nargs; ++i) {
47314740
argv[i] = emit_expr(ctx, args[i]);
@@ -4744,6 +4753,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
47444753
}
47454754
else if (head == splatnew_sym) {
47464755
jl_cgval_t argv[2];
4756+
assert(nargs == 2);
47474757
argv[0] = emit_expr(ctx, args[0]);
47484758
argv[1] = emit_expr(ctx, args[1]);
47494759
Value *typ = boxed(ctx, argv[0]);
@@ -4754,7 +4764,6 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
47544764
return mark_julia_type(ctx, val, true, (jl_value_t*)jl_any_type);
47554765
}
47564766
else if (head == new_opaque_closure_sym) {
4757-
size_t nargs = jl_array_len(ex->args);
47584767
assert(nargs >= 5 && "Not enough arguments in new_opaque_closure");
47594768
SmallVector<jl_cgval_t, 5> argv(nargs);
47604769
for (size_t i = 0; i < nargs; ++i) {
@@ -4894,11 +4903,13 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
48944903
true, jl_any_type);
48954904
}
48964905
else if (head == exc_sym) {
4906+
assert(nargs == 0);
48974907
return mark_julia_type(ctx,
48984908
ctx.builder.CreateCall(prepare_call(jl_current_exception_func)),
48994909
true, jl_any_type);
49004910
}
49014911
else if (head == copyast_sym) {
4912+
assert(nargs == 1);
49024913
jl_cgval_t ast = emit_expr(ctx, args[0]);
49034914
if (ast.typ != (jl_value_t*)jl_expr_type && ast.typ != (jl_value_t*)jl_any_type) {
49044915
// elide call to jl_copy_ast when possible
@@ -4911,7 +4922,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
49114922
else if (head == loopinfo_sym) {
49124923
// parse Expr(:loopinfo, "julia.simdloop", ("llvm.loop.vectorize.width", 4))
49134924
SmallVector<Metadata *, 8> MDs;
4914-
for (int i = 0, ie = jl_expr_nargs(ex); i < ie; ++i) {
4925+
for (int i = 0, ie = nargs; i < ie; ++i) {
49154926
Metadata *MD = to_md_tree(args[i]);
49164927
if (MD)
49174928
MDs.push_back(MD);
@@ -4931,7 +4942,6 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval)
49314942
return mark_julia_const(bounds_check_enabled(ctx, jl_true) ? jl_true : jl_false);
49324943
}
49334944
else if (head == gc_preserve_begin_sym) {
4934-
size_t nargs = jl_array_len(ex->args);
49354945
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nargs);
49364946
for (size_t i = 0; i < nargs; ++i) {
49374947
argv[i] = emit_expr(ctx, args[i]);
@@ -7452,7 +7462,7 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
74527462
assert(lty != T_prjlvalue);
74537463
Value *isvalid = emit_isa(ctx, val, phiType, NULL).first;
74547464
emit_guarded_test(ctx, isvalid, nullptr, [&] {
7455-
(void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest));
7465+
(void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest), tbaa_stack);
74567466
return nullptr;
74577467
});
74587468
}
@@ -7480,7 +7490,7 @@ static std::pair<std::unique_ptr<Module>, jl_llvm_functions_t>
74807490
V = V_rnull;
74817491
Type *lty = julia_type_to_llvm(ctx, val.typ);
74827492
if (dest && !type_is_ghost(lty)) // basically, if !ghost union
7483-
emit_unbox(ctx, lty, val, val.typ, dest);
7493+
emit_unbox(ctx, lty, val, val.typ, dest, tbaa_stack);
74847494
RTindex = ConstantInt::get(T_int8, tindex);
74857495
}
74867496
}

src/debuginfo.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,20 +1477,24 @@ void register_eh_frames(uint8_t *Addr, size_t Size)
14771477
jl_profile_atomic([&]() {
14781478
__register_frame(Addr);
14791479
});
1480+
1481+
// Now first count the number of FDEs
1482+
size_t nentries = 0;
1483+
processFDEs((char*)Addr, Size, [&](const char*){ nentries++; });
1484+
if (nentries == 0)
1485+
return;
1486+
14801487
// Our unwinder
14811488
unw_dyn_info_t *di = new unw_dyn_info_t;
14821489
// In a shared library, this is set to the address of the PLT.
14831490
// For us, just put 0 to emulate a static library. This field does
14841491
// not seem to be used on our supported architectures.
14851492
di->gp = 0;
14861493
// I'm not a great fan of the naming of this constant, but it means the
1487-
// right thing, which is a table of FDEs and ips.
1494+
// right thing, which is a table of FDEs and IPs.
14881495
di->format = UNW_INFO_FORMAT_IP_OFFSET;
14891496
di->u.rti.name_ptr = 0;
14901497
di->u.rti.segbase = (unw_word_t)Addr;
1491-
// Now first count the number of FDEs
1492-
size_t nentries = 0;
1493-
processFDEs((char*)Addr, Size, [&](const char*){ nentries++; });
14941498

14951499
uintptr_t start_ip = (uintptr_t)-1;
14961500
uintptr_t end_ip = 0;

src/gc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1674,7 +1674,7 @@ static void NOINLINE gc_mark_stack_resize(jl_gc_mark_cache_t *gc_cache, jl_gc_ma
16741674

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

src/gf.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,6 +2577,7 @@ JL_DLLEXPORT jl_function_t *jl_get_kwsorter(jl_value_t *ty)
25772577
strcpy(&suffixed[0], name);
25782578
strcpy(&suffixed[l], "##kw");
25792579
jl_sym_t *fname = jl_symbol(suffixed);
2580+
free(suffixed);
25802581
mt->kwsorter = jl_new_generic_function_with_supertype(fname, mt->module, jl_function_type);
25812582
jl_gc_wb(mt, mt->kwsorter);
25822583
}

src/intrinsics.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,47 +1074,63 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar
10741074

10751075
switch (f) {
10761076
case arraylen: {
1077+
assert(nargs == 1);
10771078
const jl_cgval_t &x = argv[0];
10781079
jl_value_t *typ = jl_unwrap_unionall(x.typ);
10791080
if (!jl_is_datatype(typ) || ((jl_datatype_t*)typ)->name != jl_array_typename)
10801081
return emit_runtime_call(ctx, f, argv, nargs);
10811082
return mark_julia_type(ctx, emit_arraylen(ctx, x), false, jl_long_type);
10821083
}
10831084
case pointerref:
1085+
assert(nargs == 3);
10841086
return emit_pointerref(ctx, argv);
10851087
case pointerset:
1088+
assert(nargs == 4);
10861089
return emit_pointerset(ctx, argv);
10871090
case atomic_fence:
1091+
assert(nargs == 1);
10881092
return emit_atomicfence(ctx, argv);
10891093
case atomic_pointerref:
1094+
assert(nargs == 2);
10901095
return emit_atomic_pointerref(ctx, argv);
10911096
case atomic_pointerset:
10921097
case atomic_pointerswap:
10931098
case atomic_pointermodify:
10941099
case atomic_pointerreplace:
10951100
return emit_atomic_pointerop(ctx, f, argv, nargs, nullptr);
10961101
case bitcast:
1102+
assert(nargs == 2);
10971103
return generic_bitcast(ctx, argv);
10981104
case trunc_int:
1105+
assert(nargs == 2);
10991106
return generic_cast(ctx, f, Instruction::Trunc, argv, true, true);
11001107
case sext_int:
1108+
assert(nargs == 2);
11011109
return generic_cast(ctx, f, Instruction::SExt, argv, true, true);
11021110
case zext_int:
1111+
assert(nargs == 2);
11031112
return generic_cast(ctx, f, Instruction::ZExt, argv, true, true);
11041113
case uitofp:
1114+
assert(nargs == 2);
11051115
return generic_cast(ctx, f, Instruction::UIToFP, argv, false, true);
11061116
case sitofp:
1117+
assert(nargs == 2);
11071118
return generic_cast(ctx, f, Instruction::SIToFP, argv, false, true);
11081119
case fptoui:
1120+
assert(nargs == 2);
11091121
return generic_cast(ctx, f, Instruction::FPToUI, argv, true, false);
11101122
case fptosi:
1123+
assert(nargs == 2);
11111124
return generic_cast(ctx, f, Instruction::FPToSI, argv, true, false);
11121125
case fptrunc:
1126+
assert(nargs == 2);
11131127
return generic_cast(ctx, f, Instruction::FPTrunc, argv, false, false);
11141128
case fpext:
1129+
assert(nargs == 2);
11151130
return generic_cast(ctx, f, Instruction::FPExt, argv, false, false);
11161131

11171132
case not_int: {
1133+
assert(nargs == 1);
11181134
const jl_cgval_t &x = argv[0];
11191135
if (!jl_is_primitivetype(x.typ))
11201136
return emit_runtime_call(ctx, f, argv, nargs);

src/jitlayers.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -831,8 +831,8 @@ uint64_t JuliaOJIT::getFunctionAddress(StringRef Name)
831831
static int globalUniqueGeneratedNames;
832832
StringRef JuliaOJIT::getFunctionAtAddress(uint64_t Addr, jl_code_instance_t *codeinst)
833833
{
834-
auto &fname = ReverseLocalSymbolTable[(void*)(uintptr_t)Addr];
835-
if (fname.empty()) {
834+
std::string *fname = &ReverseLocalSymbolTable[(void*)(uintptr_t)Addr];
835+
if (fname->empty()) {
836836
std::string string_fname;
837837
raw_string_ostream stream_fname(string_fname);
838838
// try to pick an appropriate name that describes it
@@ -851,10 +851,10 @@ StringRef JuliaOJIT::getFunctionAtAddress(uint64_t Addr, jl_code_instance_t *cod
851851
}
852852
const char* unadorned_name = jl_symbol_name(codeinst->def->def.method->name);
853853
stream_fname << unadorned_name << "_" << globalUniqueGeneratedNames++;
854-
fname = strdup(stream_fname.str().c_str());
855-
addGlobalMapping(fname, Addr);
854+
*fname = std::move(stream_fname.str()); // store to ReverseLocalSymbolTable
855+
addGlobalMapping(*fname, Addr);
856856
}
857-
return fname;
857+
return *fname;
858858
}
859859

860860

src/jitlayers.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ class JuliaOJIT {
223223
ObjLayerT ObjectLayer;
224224
CompileLayerT CompileLayer;
225225

226-
DenseMap<void*, StringRef> ReverseLocalSymbolTable;
226+
DenseMap<void*, std::string> ReverseLocalSymbolTable;
227227
};
228228
extern JuliaOJIT *jl_ExecutionEngine;
229229

src/llvm-final-gc-lowering.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ bool FinalLowerGC::doFinalization(Module &M)
227227
functionList + sizeof(functionList) / sizeof(void*));
228228
bool changed = false;
229229
SmallVector<Constant*, 16> init;
230-
ConstantArray *CA = dyn_cast<ConstantArray>(used->getInitializer());
230+
ConstantArray *CA = cast<ConstantArray>(used->getInitializer());
231231
for (auto &Op : CA->operands()) {
232232
Constant *C = cast_or_null<Constant>(Op);
233233
if (InitAsSet.count(C->stripPointerCasts())) {

0 commit comments

Comments
 (0)