Message-ID: <8578805.OV4Wx5bFTl@skuld-framework>
Date: Wed, 31 Jul 2024 16:46:18 -0400
From: Neal Gompa <neal@...pa.dev>
To: Petr Pavlu <petr.pavlu@...e.com>, Sami Tolvanen <samitolvanen@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Masahiro Yamada <masahiroy@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>,
 Miguel Ojeda <ojeda@...nel.org>, Matthew Maurer <mmaurer@...gle.com>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Wedson Almeida Filho <wedsonaf@...il.com>, Gary Guo <gary@...yguo.net>,
 linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-modules@...r.kernel.org, rust-for-linux@...r.kernel.org,
 Asahi Linux <asahi@...ts.linux.dev>, Hector Martin <marcan@...can.st>,
 Janne Grunau <j@...nau.net>
Subject: Re: [PATCH 00/15] Implement MODVERSIONS for Rust

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@...e.com> 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!

Powered by blists - more mailing lists