On Friday, July 26, 2024 5:05:22 PM EDT Sami Tolvanen wrote: > 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. > Could you please add myself, Hector, and Janne (along with the Asahi Linux mailing list) to the recipients when you send the v2 patch set? We're also interested in this. :) Thanks in advance! -- 真実はいつも一つ!/ Always, there's only one truth!