Hi Petr, On Mon, Jul 22, 2024 at 8:20 AM Petr Pavlu <petr.pavlu@xxxxxxxx> wrote: > > From my perspective, I'm okay if gendwarfksyms doesn't provide > functionality to compare a new object file with its reference symtypes > file. > > As mentioned, genksyms has this functionality but I actually think the > way it works is not ideal. Its design is to operate on one compilation > unit at the time. This has the advantage that a comparison of each file > is performed in parallel during the build, simply because of the make > job system. On the other hand, it has two problems. > > The first one is that genksyms doesn't provide a comparison of the > kernel as a whole. This means that the tool gives rather scattered and > duplicated output about changed structs in the build log. Ideally, one > would like to see a single compact report about what changed at the end > of the build. Sure, that makes sense. Android uses STG for this, which might be useful to other folks too: https://android.googlesource.com/platform/external/stg/ https://android.googlesource.com/platform/external/stg/+/refs/heads/main/doc/stgdiff.md#output-formats > A few months ago, I also started working on a tool inspired by this > script. The goal is to have similar functionality but hopefully with > a much faster implementation. Hence, this tool is written in a compiled > language (Rust at the moment) and should also become multi-threaded. I'm > hoping to find some time to make progress on it and make the code > public. It could later be added to the upstream kernel to replace the > comparison functionality implemented by genksyms, if there is interest. > > So as mentioned, I'm fine if gendwarfksyms doesn't have this > functionality. However, for distributions that rely on the symtypes > format, I'd be interested in having gendwarfksyms output its dump data > in this format as well. We can definitely tweak the output format, but I'm not sure if making it fully compatible with the genksyms symtypes format is feasible, especially for Rust code. I also intentionally decided to use DWARF tag names in the output instead of shorthands like s# etc. to make it a bit more readable. > For example, instead of producing: > > gendwarfksyms: process_exported_symbols: _some_mangled_func_name (@ XYZ) > subprogram( > [formal parameters...] > ) > -> structure_type core::result::Result<(), core::fmt::Error> { > [a description of the structure...] > }; > > .. the output could be something like this: > > S#'core::result::Result<(), core::fmt::Error>' structure_type core::result::Result<(), core::fmt::Error> { [a description of the structure...] } > _some_mangled_func_name subprogram _some_mangled_func_name ( [formal parameters...] ) -> S#'core::result::Result<(), core::fmt::Error>' This wouldn't be enough to make the output format compatible with symtypes though. genksyms basically produces a simple key-value pair database while gendwarfksyms currently outputs the fully expanded type string for each symbol. If you need the tool to produce a type database, it might also be worth discussing if we should use a bit less ad hoc format in that case. One more thing to note about the current --debug output is that it directly correlates with the debugging information and thus may not contain all aliases. For example, the Rust compiler deduplicates identical function implementations (e.g. Deref::deref and DerefMut::deref_mut etc.), but only one of the symbol names appears in DWARF. We use symbol addresses to print out #SYMVERs also for the aliases, but they don't show up in the debugging output right now. > > If using unions here is acceptable to everyone, a simple solution > > would be to use a known name prefix for the reserved members and teach > > gendwarfksyms to only print out the original type for the replaced > > ones. For example: > > > > The initial placeholder: > > > > u8 __kabi_reserved_1[8]; > > > > After replacement: > > > > union { > > u64 new_member; > > struct { > > u8 __kabi_reserved_1[8]; > > }; > > } > > > > Here gendwarfksyms would see the __kabi_reserved prefix and only use > > u8 [8] for the CRC calculation. Does this sound reasonable? > > I like this idea. I think it's good that the necessary kABI information > about an updated member can be expressed at the source code level in > place of the actual change, and it isn't needed to feed additional input > to the tool. OK, cool. I agree that being able to specify these details in source code is much cleaner. I'll add an implementation for this, and for the definition visibility issue Greg mentioned in v2. Sami
