Skip to content

Commit

Permalink
Fixup static analyzer warnings in staticdata.c (JuliaLang#51984)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gbaraldi
gbaraldi authored Nov 3, 2023
1 parent 09cbae8 commit b3fe970
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 40 deletions.
4 changes: 1 addition & 3 deletions src/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,6 @@ SA_EXCEPTIONS-jloptions.c := -Xanalyzer -analyzer-config -Xana
SA_EXCEPTIONS-subtype.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core.uninitialized.Assign;core.UndefinedBinaryOperatorResult"
SA_EXCEPTIONS-codegen.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core"
# these need to be annotated (and possibly fixed)
SKIP_IMPLICIT_ATOMICS := staticdata.c
# these need to be annotated (and possibly fixed)
SKIP_GC_CHECK := codegen.cpp rtutils.c

# make sure LLVM's invariant information is not discarded with -DNDEBUG
Expand Down Expand Up @@ -540,7 +538,7 @@ $(addprefix clang-sagc-,$(CODEGEN_SRCS)): DEBUGFLAGS_CLANG += -DJL_LIBRARY_EXPOR
$(addprefix clang-tidy-,$(CODEGEN_SRCS)): DEBUGFLAGS_CLANG += -DJL_LIBRARY_EXPORTS_CODEGEN

# Add C files as a target of `analyzesrc` and `analyzegc` and `tidysrc`
tidysrc: $(addprefix clang-tidy-,$(filter-out $(basename $(SKIP_IMPLICIT_ATOMICS)),$(CODEGEN_SRCS) $(SRCS)))
tidysrc: $(addprefix clang-tidy-,$(CODEGEN_SRCS) $(SRCS))
analyzesrc: $(addprefix clang-sa-,$(CODEGEN_SRCS) $(SRCS))
analyzegc: $(addprefix clang-sagc-,$(filter-out $(basename $(SKIP_GC_CHECK)),$(CODEGEN_SRCS) $(SRCS)))
analyze: analyzesrc analyzegc tidysrc
Expand Down
66 changes: 33 additions & 33 deletions src/staticdata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,8 +722,8 @@ static void jl_queue_module_for_serialization(jl_serializer_state *s, jl_module_
{
jl_queue_for_serialization(s, m->name);
jl_queue_for_serialization(s, m->parent);
jl_queue_for_serialization(s, m->bindings);
jl_queue_for_serialization(s, m->bindingkeyset);
jl_queue_for_serialization(s, jl_atomic_load_relaxed(&m->bindings));
jl_queue_for_serialization(s, jl_atomic_load_relaxed(&m->bindingkeyset));
if (jl_options.strip_metadata) {
jl_svec_t *table = jl_atomic_load_relaxed(&m->bindings);
for (size_t i = 0; i < jl_svec_len(table); i++) {
Expand Down Expand Up @@ -810,8 +810,8 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_
if (jl_is_typename(v)) {
jl_typename_t *tn = (jl_typename_t*)v;
// don't recurse into several fields (yet)
jl_queue_for_serialization_(s, (jl_value_t*)tn->cache, 0, 1);
jl_queue_for_serialization_(s, (jl_value_t*)tn->linearcache, 0, 1);
jl_queue_for_serialization_(s, (jl_value_t*)jl_atomic_load_relaxed(&tn->cache), 0, 1);
jl_queue_for_serialization_(s, (jl_value_t*)jl_atomic_load_relaxed(&tn->linearcache), 0, 1);
if (s->incremental) {
assert(!jl_object_in_image((jl_value_t*)tn->module));
assert(!jl_object_in_image((jl_value_t*)tn->wrapper));
Expand Down Expand Up @@ -1143,12 +1143,12 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t
newm->parent = NULL;
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, parent)));
arraylist_push(&s->relocs_list, (void*)backref_id(s, m->parent, s->link_ids_relocs));
newm->bindings = NULL;
jl_atomic_store_relaxed(&newm->bindings, NULL);
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, bindings)));
arraylist_push(&s->relocs_list, (void*)backref_id(s, m->bindings, s->link_ids_relocs));
newm->bindingkeyset = NULL;
arraylist_push(&s->relocs_list, (void*)backref_id(s, jl_atomic_load_relaxed(&m->bindings), s->link_ids_relocs));
jl_atomic_store_relaxed(&newm->bindingkeyset, NULL);
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, bindingkeyset)));
arraylist_push(&s->relocs_list, (void*)backref_id(s, m->bindingkeyset, s->link_ids_relocs));
arraylist_push(&s->relocs_list, (void*)backref_id(s, jl_atomic_load_relaxed(&m->bindingkeyset), s->link_ids_relocs));
newm->primary_world = ~(size_t)0;

// write out the usings list
Expand Down Expand Up @@ -1584,26 +1584,25 @@ static void jl_write_values(jl_serializer_state *s) JL_GC_DISABLED
if (m->max_world != ~(size_t)0)
newm->max_world = 0;
else {
if (m->inferred && ptrhash_has(&s->callers_with_edges, m->def))
if (jl_atomic_load_relaxed(&m->inferred) && ptrhash_has(&s->callers_with_edges, m->def))
newm->max_world = 1; // sentinel value indicating this will need validation
if (m->min_world > 0 && m->inferred) {
if (m->min_world > 0 && jl_atomic_load_relaxed(&m->inferred) ) {
// TODO: also check if this object is part of the codeinst cache
// will check on deserialize if this cache entry is still valid
}
}
}

newm->invoke = NULL;
newm->specsigflags = 0;
newm->specptr.fptr = NULL;
jl_atomic_store_relaxed(&newm->invoke, NULL);
jl_atomic_store_relaxed(&newm->specsigflags, 0);
jl_atomic_store_relaxed(&newm->specptr.fptr, NULL);
int8_t fptr_id = JL_API_NULL;
int8_t builtin_id = 0;
if (m->invoke == jl_fptr_const_return) {
if (jl_atomic_load_relaxed(&m->invoke) == jl_fptr_const_return) {
fptr_id = JL_API_CONST;
}
else {
if (jl_is_method(m->def->def.method)) {
builtin_id = jl_fptr_id(m->specptr.fptr);
builtin_id = jl_fptr_id(jl_atomic_load_relaxed(&m->specptr.fptr));
if (builtin_id) { // found in the table of builtins
assert(builtin_id >= 2);
fptr_id = JL_API_BUILTIN;
Expand Down Expand Up @@ -1643,7 +1642,7 @@ static void jl_write_values(jl_serializer_state *s) JL_GC_DISABLED
}
}
}
newm->invoke = NULL; // relocation offset
jl_atomic_store_relaxed(&newm->invoke, NULL); // relocation offset
if (fptr_id != JL_API_NULL) {
assert(fptr_id < BuiltinFunctionTag && "too many functions to serialize");
arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_code_instance_t, invoke))); // relocation location
Expand Down Expand Up @@ -2138,8 +2137,8 @@ static void jl_update_all_fptrs(jl_serializer_state *s, jl_image_t *image)
}
jl_code_instance_t *codeinst = (jl_code_instance_t*)(base + offset);
uintptr_t base = (uintptr_t)fvars.base;
assert(jl_is_method(codeinst->def->def.method) && codeinst->invoke != jl_fptr_const_return);
assert(specfunc ? codeinst->invoke != NULL : codeinst->invoke == NULL);
assert(jl_is_method(codeinst->def->def.method) && jl_atomic_load_relaxed(&codeinst->invoke) != jl_fptr_const_return);
assert(specfunc ? jl_atomic_load_relaxed(&codeinst->invoke) != NULL : jl_atomic_load_relaxed(&codeinst->invoke) == NULL);
linfos[i] = codeinst->def; // now it's a MethodInstance
int32_t offset = fvars.offsets[i];
for (; clone_idx < fvars.nclones; clone_idx++) {
Expand All @@ -2152,11 +2151,11 @@ static void jl_update_all_fptrs(jl_serializer_state *s, jl_image_t *image)
}
void *fptr = (void*)(base + offset);
if (specfunc) {
codeinst->specptr.fptr = fptr;
codeinst->specsigflags = 0b111; // TODO: set only if confirmed to be true
jl_atomic_store_relaxed(&codeinst->specptr.fptr, fptr);
jl_atomic_store_relaxed(&codeinst->specsigflags, 0b111); // TODO: set only if confirmed to be true
}
else {
codeinst->invoke = (jl_callptr_t)fptr;
jl_atomic_store_relaxed(&codeinst->invoke,(jl_callptr_t)fptr);
}
}
}
Expand Down Expand Up @@ -2227,8 +2226,8 @@ static void jl_root_new_gvars(jl_serializer_state *s, jl_image_t *image, uint32_
v = (uintptr_t)jl_as_global_root((jl_value_t*)v);
} else {
jl_code_instance_t *codeinst = (jl_code_instance_t*) v;
assert(codeinst && (codeinst->specsigflags & 0b01) && codeinst->specptr.fptr);
v = (uintptr_t)codeinst->specptr.fptr;
assert(codeinst && (jl_atomic_load_relaxed(&codeinst->specsigflags) & 0b01) && jl_atomic_load_relaxed(&codeinst->specptr.fptr));
v = (uintptr_t)jl_atomic_load_relaxed(&codeinst->specptr.fptr);
}
*gv = v;
}
Expand Down Expand Up @@ -2363,8 +2362,8 @@ static int strip_all_codeinfos__(jl_typemap_entry_t *def, void *_env)
if (m->source) {
int stripped_ir = 0;
if (jl_options.strip_ir) {
if (m->unspecialized) {
jl_code_instance_t *unspec = jl_atomic_load_relaxed(&m->unspecialized->cache);
if (jl_atomic_load_relaxed(&m->unspecialized)) {
jl_code_instance_t *unspec = jl_atomic_load_relaxed(&jl_atomic_load_relaxed(&m->unspecialized)->cache);
if (unspec && jl_atomic_load_relaxed(&unspec->invoke)) {
// we have a generic compiled version, so can remove the IR
record_field_change(&m->source, jl_nothing);
Expand All @@ -2385,7 +2384,7 @@ static int strip_all_codeinfos__(jl_typemap_entry_t *def, void *_env)
jl_gc_wb(m, m->source);
}
}
jl_value_t *specializations = m->specializations;
jl_value_t *specializations = jl_atomic_load_relaxed(&m->specializations);
if (!jl_is_svec(specializations)) {
strip_specializations_((jl_method_instance_t*)specializations);
}
Expand All @@ -2397,8 +2396,8 @@ static int strip_all_codeinfos__(jl_typemap_entry_t *def, void *_env)
strip_specializations_((jl_method_instance_t*)mi);
}
}
if (m->unspecialized)
strip_specializations_(m->unspecialized);
if (jl_atomic_load_relaxed(&m->unspecialized))
strip_specializations_(jl_atomic_load_relaxed(&m->unspecialized));
if (jl_options.strip_ir && m->root_blocks)
record_field_change((jl_value_t**)&m->root_blocks, NULL);
return 1;
Expand All @@ -2408,7 +2407,7 @@ static int strip_all_codeinfos_(jl_methtable_t *mt, void *_env)
{
if (jl_options.strip_ir && mt->backedges)
record_field_change((jl_value_t**)&mt->backedges, NULL);
return jl_typemap_visitor(mt->defs, strip_all_codeinfos__, NULL);
return jl_typemap_visitor(jl_atomic_load_relaxed(&mt->defs), strip_all_codeinfos__, NULL);
}

static void jl_strip_all_codeinfos(void)
Expand Down Expand Up @@ -2646,9 +2645,10 @@ static void jl_save_system_image_to_stream(ios_t *f, jl_array_t *mod_array,
for (i = 0; i < serialization_queue.len; i++) {
jl_typename_t *tn = (jl_typename_t*)serialization_queue.items[i];
if (jl_is_typename(tn)) {
tn->cache = jl_prune_type_cache_hash(tn->cache);
jl_gc_wb(tn, tn->cache);
jl_prune_type_cache_linear(tn->linearcache);
jl_atomic_store_relaxed(&tn->cache,
jl_prune_type_cache_hash(jl_atomic_load_relaxed(&tn->cache)));
jl_gc_wb(tn, jl_atomic_load_relaxed(&tn->cache));
jl_prune_type_cache_linear(jl_atomic_load_relaxed(&tn->linearcache));
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/staticdata_utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static int has_backedge_to_worklist(jl_method_instance_t *mi, htable_t *visited,
if (jl_is_method(mod))
mod = ((jl_method_t*)mod)->module;
assert(jl_is_module(mod));
if (mi->precompiled || !jl_object_in_image((jl_value_t*)mod) || type_in_worklist(mi->specTypes)) {
if (jl_atomic_load_relaxed(&mi->precompiled) || !jl_object_in_image((jl_value_t*)mod) || type_in_worklist(mi->specTypes)) {
return 1;
}
if (!mi->backedges) {
Expand Down Expand Up @@ -234,7 +234,7 @@ static jl_array_t *queue_external_cis(jl_array_t *list)
continue;
jl_method_instance_t *mi = ci->def;
jl_method_t *m = mi->def.method;
if (ci->inferred && jl_is_method(m) && jl_object_in_image((jl_value_t*)m->module)) {
if (jl_atomic_load_relaxed(&ci->inferred) && jl_is_method(m) && jl_object_in_image((jl_value_t*)m->module)) {
int found = has_backedge_to_worklist(mi, &visited, &stack);
assert(found == 0 || found == 1 || found == 2);
assert(stack.len == 0);
Expand Down Expand Up @@ -1119,7 +1119,7 @@ static void jl_insert_backedges(jl_array_t *edges, jl_array_t *ext_targets, jl_a
else {
assert(ci->max_world == ~(size_t)0);
jl_method_instance_t *caller = ci->def;
if (ci->inferred && jl_rettype_inferred(caller, minworld, ~(size_t)0) == jl_nothing) {
if (jl_atomic_load_relaxed(&ci->inferred) && jl_rettype_inferred(caller, minworld, ~(size_t)0) == jl_nothing) {
jl_mi_cache_insert(caller, ci);
}
//jl_static_show((jl_stream*)ios_stderr, (jl_value_t*)caller);
Expand Down Expand Up @@ -1162,7 +1162,7 @@ static void jl_insert_backedges(jl_array_t *edges, jl_array_t *ext_targets, jl_a
// have some new external code to use
assert(jl_is_code_instance(ci));
jl_code_instance_t *codeinst = (jl_code_instance_t*)ci;
assert(codeinst->min_world == minworld && codeinst->inferred);
assert(codeinst->min_world == minworld && jl_atomic_load_relaxed(&codeinst->inferred) );
codeinst->max_world = maxvalid;
if (jl_rettype_inferred(caller, minworld, maxvalid) == jl_nothing) {
jl_mi_cache_insert(caller, codeinst);
Expand Down

0 comments on commit b3fe970

Please sign in to comment.