On 11/21/24 21:42, Sami Tolvanen wrote: > Expand each structure type only once per exported symbol. This > is necessary to support self-referential structures, which would > otherwise result in infinite recursion, and it's sufficient for > catching ABI changes. > > Types defined in .c files are opaque to external users and thus > cannot affect the ABI. Consider type definitions in .c files to > be declarations to prevent opaque types from changing symbol > versions. Thanks for adding support for skipping types defined in .c files. That is a useful feature that genksyms has. I was also recently thinking that it would be great if genksyms could skip definitions that are in internal header files, for example, kernel/events/internal.h. Perhaps something that could be added in the future.. I've noted one nit below, but the patch looks sensible to me, feel free to use: Reviewed-by: Petr Pavlu <petr.pavlu@xxxxxxxx> > @@ -79,6 +80,55 @@ static bool match_export_symbol(struct state *state, Dwarf_Die *die) > return !!state->sym; > } > > +/* DW_AT_decl_file -> struct srcfile */ > +static struct cache srcfile_cache; > + > +static bool is_definition_private(Dwarf_Die *die) > +{ > + Dwarf_Word filenum; > + Dwarf_Files *files; > + Dwarf_Die cudie; > + const char *s; > + int res; > + > + /* > + * Definitions in .c files cannot change the public ABI, > + * so consider them private. > + */ > + if (!get_udata_attr(die, DW_AT_decl_file, &filenum)) > + return false; > + > + res = cache_get(&srcfile_cache, filenum); > + if (res >= 0) > + return !!res; > + > + if (!dwarf_cu_die(die->cu, &cudie, NULL, NULL, NULL, NULL, NULL, NULL)) > + error("dwarf_cu_die failed: '%s'", dwarf_errmsg(-1)); > + > + if (dwarf_getsrcfiles(&cudie, &files, NULL)) > + error("dwarf_getsrcfiles failed: '%s'", dwarf_errmsg(-1)); > + > + s = dwarf_filesrc(files, filenum, NULL, NULL); > + if (!s) > + error("dwarf_filesrc failed: '%s'", dwarf_errmsg(-1)); > + > + s = strrchr(s, '.'); > + res = s && !strcmp(s, ".c"); > + cache_set(&srcfile_cache, filenum, res); > + > + return !!res; > +} > + > +static bool is_declaration(Dwarf_Die *die) > +{ > + bool value; > + > + if (get_flag_attr(die, DW_AT_declaration, &value) && value) > + return true; > + > + return is_definition_private(die); > +} Nit: When I read the is_declaration() function in isolation, it is not clear to me what determining if a definition is private has to do with the type being a declaration. I think this and related logic in __process_structure_type() would be easier to follow if the return value of is_declaration() was negated and the function renamed, for example, to is_kabi_definition(). -- Thanks, Petr