rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sami Tolvanen <samitolvanen@google.com>
To: Petr Pavlu <petr.pavlu@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	 Luis Chamberlain <mcgrof@kernel.org>,
	Miguel Ojeda <ojeda@kernel.org>,
	 Matthew Maurer <mmaurer@google.com>,
	Alex Gaynor <alex.gaynor@gmail.com>,
	 Wedson Almeida Filho <wedsonaf@gmail.com>,
	Gary Guo <gary@garyguo.net>,
	linux-kbuild@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-modules@vger.kernel.org,  rust-for-linux@vger.kernel.org
Subject: Re: [PATCH 00/15] Implement MODVERSIONS for Rust
Date: Thu, 1 Aug 2024 19:38:27 +0000	[thread overview]
Message-ID: <CABCJKueScjymx3TR-DCVynfgSg93-17cKC0s0b3A5KpiFEOiEA@mail.gmail.com> (raw)
In-Reply-To: <f08678b1-260f-4200-889b-a4ec016fc7e1@suse.com>

Hi Petr,

On Thu, Aug 1, 2024 at 4:22 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> STG is an interesting tool. I've played with it a bit last year. To be
> frank, I was surprised to see a new tool being proposed by Google to
> generate modversion CRCs from DWARF instead of potentially extending
> your STG project for this purpose. I'm not sure if it is something that
> you folks have considered and evaluated.

Yes. STG is fairly large and written in C++, so it didn't seem like a
great candidate for inclusion in the kernel tree, and it's presumably
not shipped by most distributions, so it's not really ideal as a
dependency either.

> Sure, it might be necessary to extend the symtypes format a bit, for
> example, by allowing spaces in type names. What other problems do you
> see?
>
> The example I showed preserves the DWARF tag names in type descriptions.
> Cross-references and the target type names use the s# prefix as they
> they need to be distinguished from other tokens.

What I meant is that genksyms output is basically preprocessed C with
references scattered in, so if you want something that's fully
backwards compatible, that might not be possible. However, if you're
happy with just the same database structure and don't care about the
exact type description format beyond that, that should be doable, but
like you said, still requires extending the format to support more
complex type names.

> What I think is needed is the ability to compare an updated kernel with
> some previous reference and have an output that clearly and accurately
> shows why CRCs of some symbols changed. The previous reference should be
> possible to store in Git together with the kernel source. It means it
> should be ideally some text format and limited in size. This is what
> distributions that care about stable kABI do in some form currently.
>
> This functionality would be needed if some distribution wants to
> maintain stable Rust kABI (not sure if it is actually feasible), or if
> the idea is for gendwarfksyms to be a general tool that could replace
> genksyms. I assume for the sake of argument that this is the case.
>
> Gendwarfksyms could implement this functionality on its own, or as
> discussed, I believe it could provide a symtypes-like dump and a second
> tool could be used to work with this format and for comparing it.
>
> From my point of view, the current --debug format is not suitable for
> this purpose because its expanded and unstructured form means it is
> bloated and hard to compare with a previous reference.

I do see your point, there's a ton of duplication in the --debug
output as each symbol expands all the types it uses.

> I'm also not quite yet sold on using separate DWARF tooling, such as
> libabigail or STG, to actually understand why gendwarfksyms produced
> a different CRC for some symbol. Using these tools makes sense in the
> genksyms world, where genksyms operates on the source code level and
> this additional tooling can only work on debug data.
>
> With gendwarfksyms working directly with DWARF data, it doesn't seem
> appealing to me to first run gendwarfksyms to produce CRCs, compare them
> with their reference, and if they are different, use a second tool to
> process the same DWARF data again and with some luck hopefully get an
> actual answer why the CRCs changed. I'm worried that users might
> encounter inaccurate answers if the two tools interpret the input data
> differently.

I agree, there might be other DWARF changes that we don't care about,
but that nevertheless make it harder to figure out what caused the
actual CRC to change.

My initial thought was that simply running a diff between the --debug
output would be sufficient, as it would directly tell you what
changed, but I didn't consider that the size of the output dump would
be of concern as well. I'll look into adding a symtypes-style output
format in the next version. Thanks again for the feedback!

Sami

      reply	other threads:[~2024-08-01 19:39 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 17:58 [PATCH 00/15] Implement MODVERSIONS for Rust Sami Tolvanen
2024-06-17 17:58 ` [PATCH 01/15] tools: Add gendwarfksyms Sami Tolvanen
2024-06-17 17:58 ` [PATCH 02/15] gendwarfksyms: Add symbol list input handling Sami Tolvanen
2024-06-17 17:58 ` [PATCH 03/15] gendwarfksyms: Add CRC calculation Sami Tolvanen
2024-06-17 17:58 ` [PATCH 04/15] gendwarfksyms: Expand base_type Sami Tolvanen
2024-06-17 17:58 ` [PATCH 05/15] gendwarfksyms: Add a cache Sami Tolvanen
2024-06-17 17:58 ` [PATCH 06/15] gendwarfksyms: Expand type modifiers and typedefs Sami Tolvanen
2024-06-17 17:58 ` [PATCH 07/15] gendwarfksyms: Add pretty-printing Sami Tolvanen
2024-06-17 17:58 ` [PATCH 08/15] gendwarfksyms: Expand subroutine_type Sami Tolvanen
2024-06-17 17:58 ` [PATCH 09/15] gendwarfksyms: Expand array_type Sami Tolvanen
2024-06-17 17:58 ` [PATCH 10/15] gendwarfksyms: Expand structure types Sami Tolvanen
2024-06-17 17:58 ` [PATCH 11/15] gendwarfksyms: Limit structure expansion Sami Tolvanen
2024-06-17 17:58 ` [PATCH 12/15] gendwarfksyms: Add inline debugging Sami Tolvanen
2024-06-17 17:58 ` [PATCH 13/15] modpost: Add support for hashing long symbol names Sami Tolvanen
2024-06-18 16:47   ` Masahiro Yamada
2024-06-18 20:07     ` Sami Tolvanen
2024-06-17 17:58 ` [PATCH 14/15] module: Support hashed symbol names when checking modversions Sami Tolvanen
2024-06-17 17:58 ` [PATCH 15/15] kbuild: Use gendwarfksyms to generate Rust symbol versions Sami Tolvanen
2024-06-18 16:28 ` [PATCH 00/15] Implement MODVERSIONS for Rust Masahiro Yamada
2024-06-18 20:05   ` Sami Tolvanen
2024-06-18 16:44 ` Greg Kroah-Hartman
2024-06-18 16:50   ` Masahiro Yamada
2024-06-18 17:18     ` Greg Kroah-Hartman
2024-06-18 19:03       ` Masahiro Yamada
2024-06-18 20:19         ` Sami Tolvanen
2024-06-18 19:42 ` Luis Chamberlain
2024-06-18 21:19   ` Sami Tolvanen
2024-06-18 23:32     ` Luis Chamberlain
2024-07-10  7:30 ` Petr Pavlu
2024-07-15 20:39   ` Sami Tolvanen
2024-07-16  7:12     ` Greg Kroah-Hartman
2024-07-18 17:04       ` Sami Tolvanen
2024-07-22  8:20     ` Petr Pavlu
2024-07-26 21:05       ` Sami Tolvanen
2024-07-31 20:46         ` Neal Gompa
2024-08-01 11:22         ` Petr Pavlu
2024-08-01 19:38           ` Sami Tolvanen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABCJKueScjymx3TR-DCVynfgSg93-17cKC0s0b3A5KpiFEOiEA@mail.gmail.com \
    --to=samitolvanen@google.com \
    --cc=alex.gaynor@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mmaurer@google.com \
    --cc=ojeda@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).