linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-kbuild@vger.kernel.org, bpf <bpf@vger.kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Zheng Yejian <zhengyejian1@huawei.com>,
	Martin  Kelly <martin.kelly@crowdstrike.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH 00/14] scripts/sorttable: Rewrite the accessing of the Elf data fields
Date: Sun, 05 Jan 2025 11:22:11 -0500	[thread overview]
Message-ID: <20250105162211.971039541@goodmis.org> (raw)


While looking at getting rid of the place holder __ftrace_invalid_address___
from the available_filter_functions file[1], I noticed that the sorttable.[ch]
code could use a major clean up!

This series is that clean up of the scripts/sorttable.c code. The sorttable.c
was a copy from recordmcount.c which is very hard to maintain. That's because
it uses macro helpers and places the code in a header file sorttable.h to
handle both the 64 bit and 32 bit version of  the Elf structures. It also uses
_r()/r()/r2() wrappers around accessing the data which will read the 64 bit or
32 bit version of the data as well as handle endianess. If the wrong wrapper is
used, an invalid value will result, and this has been a cause for bugs in the
past. In fact the new ORC code doesn't even use it. That's fine because ORC is
only for 64 bit x86 which is the default parsing.

Instead of having a bunch of macros defined and then include the code
twice from a header, the Elf structures are each wrapped in a union.
The union holds the 64 bit and 32 bit version of the needed structure.
To access the values, helper function pointers are used instead of
defining a function. For example, instead of having:

  In sorttable.h:

     #undef Elf_Ehdr
     #undef Elf_Shdr

     #ifdef SORTTABLE_64
     # define Elf_Ehdr		Elf64_Ehdr
     # define Elf_Shdr		Elf64_Shdr
     [..]
     # define _r			r8
     #else
     # define Elf_Ehdr		Elf32_Ehdr
     # define Elf_Shdr		Elf32_Shdr
     [..]
     # define _r			r
     #endif

     [..]
     Elf_Shdr *s, *shdr = (Elf_Shdr *)((char *)ehdr + _r(&ehdr->e_shoff));

  In sorttable.c:

     #include "sorttable.h"
     #define SORTTABLE_64
     #include "sorttable.h"

Using the Unions we have just sorttable.c:

     typedef union {
	    Elf32_Ehdr	e32;
	    Elf64_Ehdr	e64;
     } Elf_Ehdr;

     typedef union {
	    Elf32_Shdr	e32;
	    Elf64_Shdr	e64;
     } Elf_Shdr;

     [..]

     static uint64_t ehdr64_shoff(Elf_Ehdr *ehdr)
     {
	    return r8(&ehdr->e64.e_shoff);
     }

     static uint64_t ehdr32_shoff(Elf_Ehdr *ehdr)
     {
	    return r(&ehdr->e32.e_shoff);
     }

     [..]
     static uint64_t (*ehdr_shoff)(Elf_Ehdr *ehdr);
     [..]

	    switch (ehdr->e32.e_ident[EI_CLASS]) {
	    case ELFCLASS32:
		    [..]
		    ehdr_shoff	= ehdr32_shoff;
		    [..]
	    case ELFCLASS65:
		    [..]
		    ehdr_shoff	= ehdr64_shoff;
		    [..]

     shdr_start = (Elf_Shdr *)((char *)ehdr + ehdr_shoff(ehdr));

The code may be a little more verbose, but the meat of the code is easier to
read, and the conversion functions live in the helper functions to make
it easier to have the fields read the proper way, and not worry how to
read the fields within the code that accesses them.

This makes the code less error prone and easier to maintain. This also
makes it easier to extend and update the sorttable code.

[1] https://lore.kernel.org/all/20250102232609.529842248@goodmis.org/

Steven Rostedt (14):
      scripts/sorttable: Remove unused macro defines
      scripts/sorttable: Remove unused write functions
      scripts/sorttable: Remove unneeded Elf_Rel
      scripts/sorttable: Have the ORC code use the _r() functions to read
      scripts/sorttable: Make compare_extable() into two functions
      scripts/sorttable: Convert Elf_Ehdr to union
      scripts/sorttable: Replace Elf_Shdr Macro with a union
      scripts/sorttable: Convert Elf_Sym MACRO over to a union
      scripts/sorttable: Add helper functions for Elf_Ehdr
      scripts/sorttable: Add helper functions for Elf_Shdr
      scripts/sorttable: Add helper functions for Elf_Sym
      scripts/sorttable: Use uint64_t for mcount sorting
      scripts/sorttable: Move code from sorttable.h into sorttable.c
      scripts/sorttable: Get start/stop_mcount_loc from ELF file directly

----
 scripts/sorttable.c | 674 +++++++++++++++++++++++++++++++++++++++++++++++-----
 scripts/sorttable.h | 497 --------------------------------------
 2 files changed, 620 insertions(+), 551 deletions(-)
 delete mode 100644 scripts/sorttable.h

             reply	other threads:[~2025-01-05 16:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-05 16:22 Steven Rostedt [this message]
2025-01-05 16:22 ` [PATCH 01/14] scripts/sorttable: Remove unused macro defines Steven Rostedt
2025-01-05 16:22 ` [PATCH 02/14] scripts/sorttable: Remove unused write functions Steven Rostedt
2025-01-05 16:22 ` [PATCH 03/14] scripts/sorttable: Remove unneeded Elf_Rel Steven Rostedt
2025-01-05 16:22 ` [PATCH 04/14] scripts/sorttable: Have the ORC code use the _r() functions to read Steven Rostedt
2025-01-05 16:22 ` [PATCH 05/14] scripts/sorttable: Make compare_extable() into two functions Steven Rostedt
2025-01-05 16:22 ` [PATCH 06/14] scripts/sorttable: Convert Elf_Ehdr to union Steven Rostedt
2025-01-05 16:22 ` [PATCH 07/14] scripts/sorttable: Replace Elf_Shdr Macro with a union Steven Rostedt
2025-01-05 16:22 ` [PATCH 08/14] scripts/sorttable: Convert Elf_Sym MACRO over to " Steven Rostedt
2025-01-05 16:22 ` [PATCH 09/14] scripts/sorttable: Add helper functions for Elf_Ehdr Steven Rostedt
2025-01-05 16:22 ` [PATCH 10/14] scripts/sorttable: Add helper functions for Elf_Shdr Steven Rostedt
2025-01-05 16:22 ` [PATCH 11/14] scripts/sorttable: Add helper functions for Elf_Sym Steven Rostedt
2025-01-05 16:22 ` [PATCH 12/14] scripts/sorttable: Use uint64_t for mcount sorting Steven Rostedt
2025-01-05 16:22 ` [PATCH 13/14] scripts/sorttable: Move code from sorttable.h into sorttable.c Steven Rostedt
2025-01-05 16:22 ` [PATCH 14/14] scripts/sorttable: Get start/stop_mcount_loc from ELF file directly Steven Rostedt

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=20250105162211.971039541@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=bpf@vger.kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.kelly@crowdstrike.com \
    --cc=masahiroy@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nicolas@fjasle.eu \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=zhengyejian1@huawei.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).