Skip to content

scripts/sorttable: Rewrite the accessing of the Elf data fields

Steven Rostedt bpf linux-kbuild linux-trace-kernel lkml 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest14 patches 0 repliesNew

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/14scripts/sorttable: Remove unused macro definesscripts/sorttable.h+0 -270 replies
2/14scripts/sorttable: Remove unused write functionsscripts/sorttable.c+0 -260 replies
3/14scripts/sorttable: Remove unneeded Elf_Relscripts/sorttable.h+2 -210 replies
4/14scripts/sorttable: Have the ORC code use the _r() functions to readscripts/sorttable.h+4 -40 replies
5/14scripts/sorttable: Make compare_extable() into two functionsscripts/sorttable.c scripts/sorttable.h+20 -140 replies
6/14scripts/sorttable: Convert Elf_Ehdr to unionscripts/sorttable.c scripts/sorttable.h+26 -220 replies
7/14scripts/sorttable: Replace Elf_Shdr Macro with a unionscripts/sorttable.c scripts/sorttable.h+51 -330 replies
8/14scripts/sorttable: Convert Elf_Sym MACRO over to a unionscripts/sorttable.c scripts/sorttable.h+19 -110 replies
9/14scripts/sorttable: Add helper functions for Elf_Ehdrscripts/sorttable.c scripts/sorttable.h+41 -40 replies
10/14scripts/sorttable: Add helper functions for Elf_Shdrscripts/sorttable.c scripts/sorttable.h+85 -230 replies
11/14scripts/sorttable: Add helper functions for Elf_Symscripts/sorttable.c scripts/sorttable.h+63 -140 replies
12/14scripts/sorttable: Use uint64_t for mcount sortingscripts/sorttable.h+11 -110 replies
13/14scripts/sorttable: Move code from sorttable.h into sorttable.cscripts/sorttable.c scripts/sorttable.h+457 -4950 replies
14/14scripts/sorttable: Get start/stop_mcount_loc from ELF file directlyscripts/sorttable.c+45 -500 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 00/14] scripts/sorttable: Rewrite the accessing of the Elf data fields

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/[email protected]/

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