[PATCH 14/14] segment_report_module: Inline consider_phdr() into only caller

Navin P navinp0304@gmail.com
Thu Nov 12 16:52:00 GMT 2020


Hi,
 I already have a patch that makes elfutils compile with clang. Since
you are working
this will be of use to you. I've attached the patch since it is big.

 Here are some of the changes
 1. All functions at file scope have static qualifier so that no
collison with other files.
 2. Non global variables declared in the outer function should be
passed as pointer variable
    in the new outer file scope function whenever they are assigned in
the nested  function. The
    argument  addition in new functions are at the end.

3.  With the applied patch above , gcc passes all 220 tests where
clang fails 3 tests which
   is due to llvm_addrsig (change in libelf/elf.h ) and other 2 tests
are error related to
.rela.eh_frame.

clang test suite
=========
# TOTAL: 222
# PASS:  217
# SKIP:  2
# XFAIL: 0
# FAIL:  3
# XPASS: 0
# ERROR: 0

On Thu, Nov 12, 2020 at 8:36 PM Timm Bäder via Elfutils-devel
<elfutils-devel@sourceware.org> wrote:
>
> Get rid of the nested function this way
>
> Signed-off-by: Timm Bäder <tbaeder@redhat.com>
> ---
>  libdwfl/dwfl_segment_report_module.c | 142 +++++++++++++--------------
>  1 file changed, 66 insertions(+), 76 deletions(-)
>
> diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
> index 0679453e..f93c60e2 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -475,7 +475,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
>        /* NOTE if the number of sections is > 0xff00 then e_shnum
>          is zero and the actual number would come from the section
>          zero sh_size field. We ignore this here because getting shdrs
> -        is just a nice bonus (see below in consider_phdr PT_LOAD
> +        is just a nice bonus (see below in the type == PT_LOAD case
>          where we trim the last segment).  */
>        shdrs_end = ehdr.e32.e_shoff + ehdr.e32.e_shnum * ehdr.e32.e_shentsize;
>        break;
> @@ -561,80 +561,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
>    build_id.len = 0;
>    build_id.vaddr =0;
>
> -  /* Consider each of the program headers we've read from the image.  */
> -  inline void consider_phdr (GElf_Word type,
> -                            GElf_Addr vaddr, GElf_Xword memsz,
> -                            GElf_Off offset, GElf_Xword filesz,
> -                            GElf_Xword align)
> -  {
> -    switch (type)
> -      {
> -      case PT_DYNAMIC:
> -       dyn_vaddr = vaddr;
> -       dyn_filesz = filesz;
> -       break;
> -
> -      case PT_NOTE:
> -          /* We calculate from the p_offset of the note segment,
> -           because we don't yet know the bias for its p_vaddr.  */
> -          consider_notes (dwfl, memory_callback, memory_callback_arg,
> -                          start + offset, filesz, align,
> -                          buffer, buffer_available, start, segment,
> -                          ei_data, &build_id,
> -                          &xlatefrom, &xlateto,
> -                          ehdr.e32.e_ident[EI_DATA]);
> -       break;
> -
> -      case PT_LOAD:
> -       align = dwfl->segment_align > 1 ? dwfl->segment_align : align ?: 1;
> -
> -       GElf_Addr vaddr_end = (vaddr + memsz + align - 1) & -align;
> -       GElf_Addr filesz_vaddr = filesz < memsz ? vaddr + filesz : vaddr_end;
> -       GElf_Off filesz_offset = filesz_vaddr - vaddr + offset;
> -
> -       if (file_trimmed_end < offset + filesz)
> -         {
> -           file_trimmed_end = offset + filesz;
> -
> -           /* Trim the last segment so we don't bother with zeros
> -              in the last page that are off the end of the file.
> -              However, if the extra bit in that page includes the
> -              section headers, keep them.  */
> -           if (shdrs_end <= filesz_offset && shdrs_end > file_trimmed_end)
> -             {
> -               filesz += shdrs_end - file_trimmed_end;
> -               file_trimmed_end = shdrs_end;
> -             }
> -         }
> -
> -       total_filesz += filesz;
> -
> -       if (file_end < filesz_offset)
> -         {
> -           file_end = filesz_offset;
> -           if (filesz_vaddr - start == filesz_offset)
> -             contiguous = file_end;
> -         }
> -
> -       if (!found_bias && (offset & -align) == 0
> -           && likely (filesz_offset >= phoff + phnum * phentsize))
> -         {
> -           bias = start - vaddr;
> -           found_bias = true;
> -         }
> -
> -       if ((vaddr & -align) < module_start)
> -         {
> -           module_start = vaddr & -align;
> -           module_address_sync = vaddr + memsz;
> -         }
> -
> -       if (module_end < vaddr_end)
> -         module_end = vaddr_end;
> -       break;
> -      }
> -  }
> -
>    Elf32_Phdr *p32 = phdrsp;
>    Elf64_Phdr *p64 = phdrsp;
>    if ((ei_class == ELFCLASS32 &&
> @@ -646,6 +572,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
>      }
>    else
>      {
> +      /* Consider each of the program headers we've read from the image.  */
>        for (uint_fast16_t i = 0; i < phnum; ++i)
>          {
>            bool is32 = (ei_class == ELFCLASS32);
> @@ -656,7 +583,70 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
>            GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
>            GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
>
> -          consider_phdr (type, vaddr, memsz, offset, filesz, align);
> +          if (type == PT_DYNAMIC)
> +            {
> +              dyn_vaddr = vaddr;
> +              dyn_filesz = filesz;
> +            }
> +          else if (type == PT_NOTE)
> +            {
> +              /* We calculate from the p_offset of the note segment,
> +               because we don't yet know the bias for its p_vaddr.  */
> +              consider_notes (dwfl, memory_callback, memory_callback_arg,
> +                              start + offset, filesz, align,
> +                              buffer, buffer_available, start, segment,
> +                              ei_data, &build_id,
> +                              &xlatefrom, &xlateto,
> +                              ehdr.e32.e_ident[EI_DATA]);
> +            }
> +          else if (type == PT_LOAD)
> +            {
> +              align = dwfl->segment_align > 1 ? dwfl->segment_align : align ?: 1;
> +
> +              GElf_Addr vaddr_end = (vaddr + memsz + align - 1) & -align;
> +              GElf_Addr filesz_vaddr = filesz < memsz ? vaddr + filesz : vaddr_end;
> +              GElf_Off filesz_offset = filesz_vaddr - vaddr + offset;
> +
> +              if (file_trimmed_end < offset + filesz)
> +                {
> +                  file_trimmed_end = offset + filesz;
> +
> +                  /* Trim the last segment so we don't bother with zeros
> +                     in the last page that are off the end of the file.
> +                     However, if the extra bit in that page includes the
> +                     section headers, keep them.  */
> +                  if (shdrs_end <= filesz_offset && shdrs_end > file_trimmed_end)
> +                    {
> +                      filesz += shdrs_end - file_trimmed_end;
> +                      file_trimmed_end = shdrs_end;
> +                    }
> +                }
> +
> +              total_filesz += filesz;
> +
> +              if (file_end < filesz_offset)
> +                {
> +                  file_end = filesz_offset;
> +                  if (filesz_vaddr - start == filesz_offset)
> +                    contiguous = file_end;
> +                }
> +
> +              if (!found_bias && (offset & -align) == 0
> +                  && likely (filesz_offset >= phoff + phnum * phentsize))
> +                {
> +                  bias = start - vaddr;
> +                  found_bias = true;
> +                }
> +
> +              if ((vaddr & -align) < module_start)
> +                {
> +                  module_start = vaddr & -align;
> +                  module_address_sync = vaddr + memsz;
> +                }
> +
> +              if (module_end < vaddr_end)
> +                module_end = vaddr_end;
> +            }
>          }
>      }
>
> --
> 2.26.2
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-elfutils-compile-with-clang.patch
Type: text/x-patch
Size: 141806 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/elfutils-devel/attachments/20201112/4d03b8c9/attachment-0001.bin>


More information about the Elfutils-devel mailing list