Re: [PATCH v5 12/18] dump/dump: Add section string table support

[Steffen Eiden]

Hi Janosch,

On 8/11/22 14:11, Janosch Frank wrote:
> As sections don't have a type like the notes do we need another way to
> determine their contents. The string table allows us to assign each
> section an identification string which architectures can then use to
> tag their sections with.
>
> There will be no string table if the architecture doesn't add custom
> sections which are introduced in a following patch.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   dump/dump.c           | 71 +++++++++++++++++++++++++++++++++++++++++++
>   include/sysemu/dump.h |  4 +++
>   2 files changed, 75 insertions(+)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 31eb20108c..0d6dbf453a 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
[ snip ]
>   }
>   
> +static void prepare_elf_section_hdr_string(DumpState *s, void *buff)
> +{
> +    Elf32_Shdr shdr32;
> +    Elf64_Shdr shdr64;
> +    int shdr_size;
> +    void *shdr;
> +
> +    if (dump_is_64bit(s)) {
> +        shdr_size = sizeof(Elf64_Shdr);
> +        memset(&shdr64, 0, shdr_size);
> +        shdr64.sh_type = SHT_STRTAB;
> +        shdr64.sh_offset = s->section_offset + s->elf_section_data_size;
> +        shdr64.sh_name = s->string_table_buf->len;
> +        g_array_append_vals(s->string_table_buf, ".strtab", sizeof(".strtab"));
I think you mixed up .strtab and .shstrtab here.
'.shstrtab' should be used here.

The ELF specs define bots as follows (from man 5 elf) :

       .shstrtab
              This section holds section names.  This section is of type
              SHT_STRTAB.  No attribute types are used.

       .strtab
              This section holds strings, most commonly the strings that
              represent the names associated with symbol table entries.
              If the file has a loadable segment that includes the
              symbol string table, the section's attributes will include
              the SHF_ALLOC bit.  Otherwise, the bit will be off.  This
              section is of type SHT_STRTAB.

However, the name lookup works, as you correctly specified that this 
section holds the section header names via the 'e_shstrndx' field in the 
elf header.

> +        shdr64.sh_size = s->string_table_buf->len;
> +        shdr = &shdr64;
> +    } else {
> +        shdr_size = sizeof(Elf32_Shdr);
> +        memset(&shdr32, 0, shdr_size);
> +        shdr32.sh_type = SHT_STRTAB;
> +        shdr32.sh_offset = s->section_offset + s->elf_section_data_size;
> +        shdr32.sh_name = s->string_table_buf->len;
> +        g_array_append_vals(s->string_table_buf, ".strtab", sizeof(".strtab"));
> +        shdr32.sh_size = s->string_table_buf->len;
> +        shdr = &shdr32;
> +    }
> +
> +    memcpy(buff, shdr, shdr_size);
> +
[snip]
Also, with your patches the dump output places the headers in this ordering:
[elf hdr]
[section hdrs]
[program hdrs]

**normally** program hdrs are placed before section hdrs,
but this is just a convention IIRC.


Steffen