From d7527048a81ed6997e96747cb93c590ff928c0e3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 13 Mar 2025 13:38:21 -0700 Subject: [PATCH] maint: port to strict C function type checking Violation of C standard detected by clang -fsanitize=undefined with clang 19.1.7 on Fedora 41 x86-64. * src/counterexample.c (si_bfs_free): * src/files.c (prefix_map_free, add_prefix_map): * src/fixits.c (fixit_cmp, fixit_free, fixits_register): (expand_to_conflict, nonunifying_shift_path) (search_state_free_children, search_state_free, ssb_free) (ssb_hasher, ssb_comp, ssb_equals, visited_hasher) (visited_comparator, ssb_append, unifying_example): * src/lssi.c (lssi_free, lssi_hasher, lssi_comparator) (shortest_path_from_start): * src/parse-simulation.c (free_parse_state) (parse_state_list_new, parser_pop): * src/state-item.c (hash_pair_hasher, hash_pair_comparator) (hash_pair_free, hash_pair_table_create): Avoid undefined behavior in C, which does not allow you to cast a function pointer to some other function type and then call it via that other type. Instead, use functions with correct types according to the C standard, and cast their parameters. * src/getargs.c (xargmatch_fn): Return int const, not int, to match what ARGMATCH_DEFINE_GROUP does. In all uses of ARGMATCH_DEFINE_GROUP, say that they return int, to match xargmatch_fn. (FLAGS_ARGMATCH): Do not cast function pointer. * src/parse-simulation.c (vc_derivation_list_append): New function. * src/system.h (deconst): New static function. --- src/counterexample.c | 62 ++++++++++++++++++++++-------------------- src/files.c | 10 +++---- src/fixits.c | 16 +++++------ src/getargs.c | 12 ++++---- src/lssi.c | 17 ++++++------ src/parse-simulation.c | 15 +++++++--- src/parse-simulation.h | 2 +- src/state-item.c | 17 ++++++------ src/system.h | 9 ++++++ 9 files changed, 90 insertions(+), 70 deletions(-) diff --git a/src/counterexample.c b/src/counterexample.c index 82ee43da..594849c1 100644 --- a/src/counterexample.c +++ b/src/counterexample.c @@ -206,8 +206,9 @@ si_bfs_contains (const si_bfs_node *n, state_item_number sin) } static void -si_bfs_free (si_bfs_node *n) +si_bfs_free (void const *vn) { + si_bfs_node *n = deconst (vn); if (n == NULL) return; --n->reference_count; @@ -231,12 +232,10 @@ typedef gl_list_t si_bfs_node_list; static inline derivation_list expand_to_conflict (state_item_number start, symbol_number conflict_sym) { - si_bfs_node *init = si_bfs_new (start, NULL); - + void const *init = si_bfs_new (start, NULL); si_bfs_node_list queue = gl_list_create (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn) si_bfs_free, - true, 1, (const void **) &init); + si_bfs_free, true, 1, &init); si_bfs_node *node = NULL; // breadth-first search for a path of productions to the conflict symbol while (gl_list_size (queue) > 0) @@ -471,11 +470,10 @@ nonunifying_shift_path (state_item_list reduce_path, state_item *shift_conflict) continue; // bfs to find a shift to the right state - si_bfs_node *init = si_bfs_new (si - state_items, NULL); + void const *init = si_bfs_new (si - state_items, NULL); si_bfs_node_list queue = gl_list_create (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn) si_bfs_free, - true, 1, (const void **) &init); + si_bfs_free, true, 1, &init); si_bfs_node *sis = NULL; state_item *prevsi = NULL; while (gl_list_size (queue) > 0) @@ -604,15 +602,17 @@ copy_search_state (search_state *parent) } static void -search_state_free_children (search_state *ss) +search_state_free_children (void const *vss) { + search_state const *ss = vss; free_parse_state (ss->states[0]); free_parse_state (ss->states[1]); } static void -search_state_free (search_state *ss) +search_state_free (void *vss) { + search_state *ss = vss; if (ss == NULL) return; search_state_free_children (ss); @@ -691,42 +691,51 @@ typedef struct } search_state_bundle; static void -ssb_free (search_state_bundle *ssb) +ssb_free (void const *vssb) { + search_state_bundle *ssb = deconst (vssb); gl_list_free (ssb->states); free (ssb); } static size_t -ssb_hasher (search_state_bundle *ssb) +ssb_hasher (void const *vssb) { + search_state_bundle const *ssb = vssb; return ssb->complexity; } static int -ssb_comp (const search_state_bundle *s1, const search_state_bundle *s2) +ssb_comp (void const *vs1, void const *vs2) { + search_state_bundle const *s1 = vs1; + search_state_bundle const *s2 = vs2; return s1->complexity - s2->complexity; } static bool -ssb_equals (const search_state_bundle *s1, const search_state_bundle *s2) +ssb_equals (void const *vs1, void const *vs2) { + search_state_bundle const *s1 = vs1; + search_state_bundle const *s2 = vs2; return s1->complexity == s2->complexity; } typedef gl_list_t ssb_list; static size_t -visited_hasher (const search_state *ss, size_t max) +visited_hasher (void const *vss, size_t max) { + search_state const *ss = vss; return (parse_state_hasher (ss->states[0], max) + parse_state_hasher (ss->states[1], max)) % max; } static bool -visited_comparator (const search_state *ss1, const search_state *ss2) +visited_comparator (void const *vss1, void const *vss2) { + search_state const *ss1 = vss1; + search_state const *ss2 = vss2; return parse_state_comparator (ss1->states[0], ss2->states[0]) && parse_state_comparator (ss1->states[1], ss2->states[1]); } @@ -762,9 +771,8 @@ ssb_append (search_state *ss) { ssb->states = gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn)search_state_free_children, - true); - gl_sortedlist_add (ssb_queue, (gl_listelement_compar_fn) ssb_comp, ssb); + search_state_free_children, true); + gl_sortedlist_add (ssb_queue, ssb_comp, ssb); } else { @@ -1109,15 +1117,12 @@ unifying_example (state_item_number itm1, state_item *conflict1 = &state_items[itm1]; state_item *conflict2 = &state_items[itm2]; search_state *initial = initial_search_state (conflict1, conflict2); - ssb_queue = gl_list_create_empty (GL_RBTREEHASH_LIST, - (gl_listelement_equals_fn) ssb_equals, - (gl_listelement_hashcode_fn) ssb_hasher, - (gl_listelement_dispose_fn) ssb_free, - false); + ssb_queue = gl_list_create_empty (GL_RBTREEHASH_LIST, ssb_equals, + ssb_hasher, ssb_free, false); visited = - hash_initialize (32, NULL, (Hash_hasher) visited_hasher, - (Hash_comparator) visited_comparator, - (Hash_data_freer) search_state_free); + hash_initialize (32, NULL, visited_hasher, + visited_comparator, + search_state_free); ssb_append (initial); xtime_t start = gethrxtime (); bool assurance_printed = false; @@ -1181,8 +1186,7 @@ unifying_example (state_item_number itm1, } generate_next_states (ss, conflict1, conflict2); } - gl_sortedlist_remove (ssb_queue, - (gl_listelement_compar_fn) ssb_comp, ssb); + gl_sortedlist_remove (ssb_queue, ssb_comp, ssb); } cex_search_end:; if (!cex) diff --git a/src/files.c b/src/files.c index 52fb7bc8..32ea5e9e 100644 --- a/src/files.c +++ b/src/files.c @@ -261,8 +261,9 @@ map_file_name (char const *filename) } static void -prefix_map_free (struct prefix_map *p) +prefix_map_free (void const *vp) { + struct prefix_map *p = deconst (vp); free (p->oldprefix); free (p->newprefix); free (p); @@ -273,11 +274,8 @@ add_prefix_map (char const *oldprefix, char const *newprefix) { if (!prefix_maps) prefix_maps - = gl_list_create_empty (GL_ARRAY_LIST, - /* equals */ NULL, - /* hashcode */ NULL, - (gl_listelement_dispose_fn) prefix_map_free, - true); + = gl_list_create_empty (GL_ARRAY_LIST, NULL, NULL, + prefix_map_free, true); struct prefix_map *p = xmalloc (sizeof (*p)); p->oldprefix = xstrdup (oldprefix); diff --git a/src/fixits.c b/src/fixits.c index 0605da41..0b58bb43 100644 --- a/src/fixits.c +++ b/src/fixits.c @@ -53,14 +53,17 @@ fixit_new (location const *loc, char const* fix) } static int -fixit_cmp (const fixit *a, const fixit *b) +fixit_cmp (void const *va, void const *vb) { + fixit const *a = va; + fixit const *b = vb; return location_cmp (a->location, b->location); } static void -fixit_free (fixit *f) +fixit_free (void const *vf) { + fixit *f = deconst (vf); free (f->fix); free (f); } @@ -84,13 +87,10 @@ void fixits_register (location const *loc, char const* fix) { if (!fixits) - fixits = gl_list_create_empty (GL_ARRAY_LIST, - /* equals */ NULL, - /* hashcode */ NULL, - (gl_listelement_dispose_fn) fixit_free, - true); + fixits = gl_list_create_empty (GL_ARRAY_LIST, NULL, NULL, + fixit_free, true); fixit *f = fixit_new (loc, fix); - gl_sortedlist_add (fixits, (gl_listelement_compar_fn) fixit_cmp, f); + gl_sortedlist_add (fixits, fixit_cmp, f); if (feature_flag & feature_fixit) fixit_print (f, stderr); } diff --git a/src/getargs.c b/src/getargs.c index ad9ded9c..dff57fe1 100644 --- a/src/getargs.c +++ b/src/getargs.c @@ -69,7 +69,7 @@ const char *skeleton = NULL; int language_prio = default_prio; struct bison_language const *language = &valid_languages[0]; -typedef int* (xargmatch_fn) (const char *context, const char *arg); +typedef int const *(xargmatch_fn) (char const *context, char const *arg); void set_yacc (location loc) @@ -164,7 +164,7 @@ flags_argmatch (const char *opt, */ #define FLAGS_ARGMATCH(FlagName, Args, All) \ flags_argmatch ("--" #FlagName, \ - (xargmatch_fn*) argmatch_## FlagName ## _value, \ + argmatch_ ## FlagName ## _value, \ argmatch_ ## FlagName ## _usage, \ All, &FlagName ## _flag, Args) @@ -179,7 +179,7 @@ enum color color_auto }; -ARGMATCH_DEFINE_GROUP (color, enum color) +ARGMATCH_DEFINE_GROUP (color, int) static const argmatch_color_doc argmatch_color_docs[] = { @@ -215,7 +215,7 @@ const argmatch_color_group_type argmatch_color_group = | --report's handling. | `----------------------*/ -ARGMATCH_DEFINE_GROUP (report, enum report) +ARGMATCH_DEFINE_GROUP (report, int) static const argmatch_report_doc argmatch_report_docs[] = { @@ -256,7 +256,7 @@ const argmatch_report_group_type argmatch_report_group = | --trace's handling. | `---------------------*/ -ARGMATCH_DEFINE_GROUP (trace, enum trace) +ARGMATCH_DEFINE_GROUP (trace, int) static const argmatch_trace_doc argmatch_trace_docs[] = { @@ -319,7 +319,7 @@ const argmatch_trace_group_type argmatch_trace_group = | --feature's handling. | `-----------------------*/ -ARGMATCH_DEFINE_GROUP (feature, enum feature) +ARGMATCH_DEFINE_GROUP (feature, int) static const argmatch_feature_doc argmatch_feature_docs[] = { diff --git a/src/lssi.c b/src/lssi.c index 79bbba18..2f35c53c 100644 --- a/src/lssi.c +++ b/src/lssi.c @@ -50,8 +50,9 @@ new_lssi (state_item_number si, lssi *p, bitset l, bool free_l) } static void -lssi_free (lssi *sn) +lssi_free (void *vsn) { + lssi *sn = vsn; if (sn == NULL) return; if (sn->free_lookahead) @@ -60,8 +61,9 @@ lssi_free (lssi *sn) } static size_t -lssi_hasher (lssi *sn, size_t max) +lssi_hasher (void const *vsn, size_t max) { + lssi const *sn = vsn; size_t hash = sn->si; bitset_iterator biter; symbol_number syn; @@ -71,8 +73,10 @@ lssi_hasher (lssi *sn, size_t max) } static bool -lssi_comparator (lssi *s1, lssi *s2) +lssi_comparator (void const *vs1, void const *vs2) { + lssi const *s1 = vs1; + lssi const *s2 = vs2; if (s1->si == s2->si) { if (s1->lookahead == s2->lookahead) @@ -154,11 +158,8 @@ state_item_list shortest_path_from_start (state_item_number target, symbol_number next_sym) { bitset eligible = eligible_state_items (&state_items[target]); - Hash_table *visited = hash_initialize (32, - NULL, - (Hash_hasher) lssi_hasher, - (Hash_comparator) lssi_comparator, - (Hash_data_freer) lssi_free); + Hash_table *visited = hash_initialize (32, NULL, lssi_hasher, + lssi_comparator, lssi_free); bitset il = bitset_create (nsyms, BITSET_FIXED); bitset_set (il, 0); lssi *init = new_lssi (0, NULL, il, true); diff --git a/src/parse-simulation.c b/src/parse-simulation.c index 9e1f66df..7c9fe382 100644 --- a/src/parse-simulation.c +++ b/src/parse-simulation.c @@ -194,8 +194,9 @@ parse_state_free_contents_early (parse_state *ps) } void -free_parse_state (parse_state *original_ps) +free_parse_state (void const *voriginal_ps) { + parse_state *original_ps = deconst (voriginal_ps); bool free_contents = true; parse_state *parent_ps = NULL; for (parse_state *ps = original_ps; ps && free_contents; ps = parent_ps) @@ -311,8 +312,7 @@ static parse_state_list parse_state_list_new (void) { return gl_list_create_empty (GL_LINKED_LIST, NULL, NULL, - (gl_listelement_dispose_fn)free_parse_state, - true); + free_parse_state, true); } static void @@ -322,6 +322,13 @@ parse_state_list_append (parse_state_list pl, parse_state *ps) gl_list_add_last (pl, ps); } +static void +vc_derivation_list_append (derivation_list dl, void const *vcd) +{ + derivation *d = deconst (vcd); + return derivation_list_append (dl, d); +} + // Emulates a reduction on a parse state by popping some amount of // derivations and state_items off of the parse_state and returning // the result in ret. Returns the derivation of what's popped. @@ -351,7 +358,7 @@ parser_pop (parse_state *ps, int deriv_index, list_flatten_and_split (chunks, ret_chunks, si_index, 2, list_add_last); list_flatten_and_split (chunks + 2, ret_chunks + 2, deriv_index, 2, - (chunk_append_fn)derivation_list_append); + vc_derivation_list_append); size_t s_size = gl_list_size (ret->state_items.contents); ret->state_items.total_size = s_size; if (s_size > 0) diff --git a/src/parse-simulation.h b/src/parse-simulation.h index ac746cd6..7d66bd9b 100644 --- a/src/parse-simulation.h +++ b/src/parse-simulation.h @@ -99,7 +99,7 @@ void parse_state_retain (parse_state *ps); * when its reference count reaches 1. This is used to * free memory while the parse state is in a hash set. */ void parse_state_free_contents_early (parse_state *ps); -void free_parse_state (parse_state *ps); +void free_parse_state (void const *ps); /* counts the amount of shift and production steps in this parse state */ void parse_state_completed_steps (const parse_state *ps, int *shifts, int *productions); diff --git a/src/state-item.c b/src/state-item.c index cc1602cc..addf4dc3 100644 --- a/src/state-item.c +++ b/src/state-item.c @@ -44,20 +44,24 @@ typedef struct } hash_pair; static size_t -hash_pair_hasher (const hash_pair *sl, size_t max) +hash_pair_hasher (void const *vsl, size_t max) { + hash_pair const *sl = vsl; return sl->key % max; } static bool -hash_pair_comparator (const hash_pair *l, const hash_pair *r) +hash_pair_comparator (void const *vl, void const *vr) { + hash_pair const *l = vl; + hash_pair const *r = vr; return l->key == r->key; } static void -hash_pair_free (hash_pair *hp) +hash_pair_free (void *vhp) { + hash_pair *hp = vhp; bitset_free (hp->l); free (hp); } @@ -65,11 +69,8 @@ hash_pair_free (hash_pair *hp) static Hash_table * hash_pair_table_create (int size) { - return hash_xinitialize (size, - NULL, - (Hash_hasher) hash_pair_hasher, - (Hash_comparator) hash_pair_comparator, - (Hash_data_freer) hash_pair_free); + return hash_xinitialize (size, NULL, hash_pair_hasher, + hash_pair_comparator, hash_pair_free); } static bitset diff --git a/src/system.h b/src/system.h index fd30926b..8ff10c19 100644 --- a/src/system.h +++ b/src/system.h @@ -308,4 +308,13 @@ obstack_escape (struct obstack* obs, const char *cp) } \ } while (0) +/* Gnulib generic list functions sometimes want args to be void const *. + We sometimes want void *, for 'free', or possibly because we plan + to cheat and modify the storage. Cast to satisfy C's static checking. */ +static inline void * +deconst (void const *p) +{ + return (void *) p; +} + #endif /* ! BISON_SYSTEM_H */