Hi
On Thu, Jul 14, 2022 at 3:54 PM Janosch Frank <fran...@linux.ibm.com> wrote:
> On 7/13/22 17:58, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Jul 13, 2022 at 5:07 PM Janosch Frank <fran...@linux.ibm.com>
> wrote:
> >>
> >> Time to add a bit more descriptiveness to the dumps.
> >
> > Please add some more description & motivation to the patch (supposedly
> > necessary for next patches), and explain that it currently doesn't
> > change the dump (afaict).
>
> How about:
>
> 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.
>
lgtm, thanks
>
>
> >>
> >> - if (dump_is_64bit(s)) {
> >> - s->phdr_offset = sizeof(Elf64_Ehdr);
> >> - s->shdr_offset = s->phdr_offset + sizeof(Elf64_Phdr) *
> s->phdr_num;
> >> - s->note_offset = s->shdr_offset + sizeof(Elf64_Shdr) *
> s->shdr_num;
> >> - s->memory_offset = s->note_offset + s->note_size;
> >> - } else {
> >> -
> >> - s->phdr_offset = sizeof(Elf32_Ehdr);
> >> - s->shdr_offset = s->phdr_offset + sizeof(Elf32_Phdr) *
> s->phdr_num;
> >> - s->note_offset = s->shdr_offset + sizeof(Elf32_Shdr) *
> s->shdr_num;
> >> - s->memory_offset = s->note_offset + s->note_size;
> >> + /*
> >> + * calculate shdr_num and elf_section_data_size so we know the
> offsets and
> >> + * sizes of all parts.
> >> + *
> >> + * If phdr_num overflowed we have at least one section header
> >> + * More sections/hdrs can be added by the architectures
> >> + */
> >> + if (s->shdr_num > 1) {
> >> + /* Reserve the string table */
> >> + s->shdr_num += 1;
> >> }
> >>
> >> + tmp = (s->phdr_num == PN_XNUM) ? s->sh_info : s->phdr_num;
> >> + if (dump_is_64bit(s)) {
> >> + s->shdr_offset = sizeof(Elf64_Ehdr);
> >> + s->phdr_offset = s->shdr_offset + sizeof(Elf64_Shdr) *
> s->shdr_num;
> >> + s->note_offset = s->phdr_offset + sizeof(Elf64_Phdr) * tmp;
> >> + } else {
> >> + s->shdr_offset = sizeof(Elf32_Ehdr);
> >> + s->phdr_offset = s->shdr_offset + sizeof(Elf32_Shdr) *
> s->shdr_num;
> >> + s->note_offset = s->phdr_offset + sizeof(Elf32_Phdr) * tmp;
> >> + }
> >> + s->memory_offset = s->note_offset + s->note_size;
> >
> > I suggest to split this in a different patch. It's not obvious that
> > you can change phdr_offset / shdr_offset, it deserves a comment.
>
> Right, will do
>
> >
> >> + s->section_offset = s->memory_offset + s->total_size;
> >> +
> >> return;
> >>
> >> cleanup:
> >> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> >> index 8379e29ef6..2c25c7d309 100644
> >> --- a/include/sysemu/dump.h
> >> +++ b/include/sysemu/dump.h
> >> @@ -178,6 +178,7 @@ typedef struct DumpState {
> >> void *elf_section_hdrs;
> >> uint64_t elf_section_data_size;
> >> void *elf_section_data;
> >> + GArray *string_table_buf; /* String table section */
> >>
> >> uint8_t *note_buf; /* buffer for notes */
> >> size_t note_buf_offset; /* the writing place in note_buf */
> >> --
> >> 2.34.1
> >>
> >
>
>
--
Marc-André Lureau
